Commit bc86256d authored Jun 06, 2012 by Artem Bityutskiy Committed by Al Viro Jul 14, 2012

affs: stop setting bm_flags

AFFS stores values '1' and '2' in 'bm_flags', and I fail to see any logic when
it prefers one or another. AFFS writes '1' only from '->put_super()', while
'->sync_fs()' and '->write_super()' store value '2'. So on the first glance,
it looks like we want to have '1' if we unmount. However, this does not really
happen in these cases:
1. superblock is written via 'write_super()' then we unmount;
2. we re-mount R/O, then unmount.
which are quite typical.

I could not find good documentation describing this field, except of one random
piece of documentation in the internet which says that -1 means that the root
block is valid, which is not consistent with what we have in the Linux AFFS
driver.

Jan Kara commented on this: "I have some vague recollection that on Amiga
boolean was usually encoded as: 0 == false, ~0 == -1 == true. But it has been
ages..."

Thus, my conclusion is that value of '1' is as good as value of '2' and we can
just always use '2'. An Jan Kara suggested to go further: "generally bm_flags
handling looks strange. If they are 0, we mount fs read only and thus cannot
change them. If they are != 0, we write 2 there. So IMHO if you just removed
bm_flags setting, nothing will really happen."

So this patch removes the bm_flags setting completely. This makes the "clean"
argument of the 'affs_commit_super()' function unneeded, so it is also removed.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

parent fdb1335a

fs/affs/super.c

+4 −5

Original line number	Diff line number	Diff line
		@@ -25,13 +25,12 @@ static int affs_statfs(struct dentry dentry, struct kstatfs buf);
		static int affs_remount (struct super_block sb, int flags, char *data);

		static void
		affs_commit_super(struct super_block *sb, int wait, int clean)
		affs_commit_super(struct super_block *sb, int wait)
		{
		struct affs_sb_info *sbi = AFFS_SB(sb);
		struct buffer_head *bh = sbi->s_root_bh;
		struct affs_root_tail *tail = AFFS_ROOT_TAIL(sb, bh);

		tail->bm_flag = cpu_to_be32(clean);
		secs_to_datestamp(get_seconds(), &tail->disk_change);
		affs_fix_checksum(sb, bh);
		mark_buffer_dirty(bh);
		@@ -46,7 +45,7 @@ affs_put_super(struct super_block *sb)
		pr_debug("AFFS: put_super()\n");

		if (!(sb->s_flags & MS_RDONLY) && sb->s_dirt)
		affs_commit_super(sb, 1, 1);
		affs_commit_super(sb, 1);

		kfree(sbi->s_prefix);
		affs_free_bitmap(sb);
		@@ -60,7 +59,7 @@ affs_write_super(struct super_block *sb)
		{
		lock_super(sb);
		if (!(sb->s_flags & MS_RDONLY))
		affs_commit_super(sb, 1, 2);
		affs_commit_super(sb, 1);
		sb->s_dirt = 0;
		unlock_super(sb);

		@@ -71,7 +70,7 @@ static int
		affs_sync_fs(struct super_block *sb, int wait)
		{
		lock_super(sb);
		affs_commit_super(sb, wait, 2);
		affs_commit_super(sb, wait);
		sb->s_dirt = 0;
		unlock_super(sb);
		return 0;

Admin message