lkml.org 
[lkml]   [2012]   [Jun]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/7] affs: remove strange argument of affs_commit_super
    On Tue 05-06-12 14:28:09, Artem Bityutskiy wrote:
    > From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
    >
    > The 'affs_commit_super()' function takes a 'clean' argument which it stores in
    > the 'bm_flags' field of the AFFS root block. This patch removes it to make the
    > code simpler and cleaner and enable few other clean-ups.
    >
    > Currently 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.
    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'. This is exactly what this patch does.
    Yeah, I believe so as well. But 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.

    Honza

    > Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
    > ---
    > fs/affs/super.c | 10 +++++-----
    > 1 files changed, 5 insertions(+), 5 deletions(-)
    >
    > diff --git a/fs/affs/super.c b/fs/affs/super.c
    > index 0782653..698282a 100644
    > --- a/fs/affs/super.c
    > +++ b/fs/affs/super.c
    > @@ -25,13 +25,13 @@ 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);
    > + tail->bm_flag = cpu_to_be32(2);
    > secs_to_datestamp(get_seconds(), &tail->disk_change);
    > affs_fix_checksum(sb, bh);
    > mark_buffer_dirty(bh);
    > @@ -46,7 +46,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 +60,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 +71,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;
    > --
    > 1.7.7.6
    >
    > --
    > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    > the body of a message to majordomo@vger.kernel.org
    > More majordomo info at http://vger.kernel.org/majordomo-info.html
    > Please read the FAQ at http://www.tux.org/lkml/
    --
    Jan Kara <jack@suse.cz>
    SUSE Labs, CR


    \
     
     \ /
      Last update: 2012-06-06 13:21    [W:0.073 / U:0.084 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site