Re: [Final call for testers][PATCH] superblock handling changes (2.4.6-pre3)

[Matthew Wilcox <matthew@wil.cx>]

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.