On Fri, Jun 15, 2001 at 01:10:00AM -0400, Alexander Viro wrote:
> +static inline void write_super(struct super_block *sb)
> +{
> +     lock_super(sb);
> +     if (sb->s_root && sb->s_dirt)
                                ^^^^
When I first looked at this, I thought it was a typo.  I don't think we
should have s_dirty and s_dirt as fields of the superblock.  how about
s_dirty_inodes and s_isdirty, respectively?

> +restart:
> +     spin_lock(&sb_lock);
> +     sb = sb_entry(super_blocks.next);
> +     while (sb != sb_entry(&super_blocks))
> +             if (sb->s_dirt) {
> +                     sb->s_count++;
> +                     spin_unlock(&sb_lock);
> +                     down_read(&sb->s_umount);
> +                     write_super(sb);
> +                     drop_super(sb);
> +                     goto restart;
> +             } else
> +                     sb = sb_entry(sb->s_list.next);
> +     spin_unlock(&sb_lock);

I think this could be clearer.

        struct list_head *tmp;
restart:
        spin_lock(&sb_lock);
        list_for_each(tmp, super_blocks) {
                struct super_block *sb = sb_entry(tmp);
                if (!sb->s_dirt)
                        continue;
                spin_unlock(&sb_lock);
                down_read(&sb->s_umount);
                write_super(sb);
                drop_super(sb);
                goto restart;
        }
        spin_unlock(&sb_lock);

> @@ -773,16 +810,16 @@
>                                      void *data, int silent)
>  {
>       struct super_block * s;
> -     s = get_empty_super();
> +     s = alloc_super();
>       if (!s)
>               goto out;
>       s->s_dev = dev;
>       s->s_bdev = bdev;
>       s->s_flags = flags;
> -     s->s_dirt = 0;
>       s->s_type = type;
> -     s->s_dquot.flags = 0;
> -     s->s_maxbytes = MAX_NON_LFS;
> +     spin_lock(&sb_lock);
> +     list_add (&s->s_list, super_blocks.prev);

I'd use list_add_tail(&s->s_list, super_blocks);

> -     if (mnt->mnt_instances.next != mnt->mnt_instances.prev) {
> +     if (atomic_read(&sb->s_active) > 1) {

I'm happy to see that line disappear.  It was mightily confusing.

-- 
Revolutions do not require corporate support.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to