Re: [PATCH 14/17] vfs: leverage bd_super in get_super and get_active_super

[Jan Kara <jack@suse.cz>]

On Mon 07-01-13 20:41:04, Fernando Luis Vázquez Cao wrote:
> Using get_active_super will not work properly if the fs (like btrfs) does not
> save its s_bdev with the device it is on.  Also it does not provide the entire
> picture, since an filesystem can be contained on multiple disks (again like
> btrfs).
> 
> Fortunately we now have a bd_super field in struct block_device were a pointer
> to the superblock sitting on top of the block device can be stored.
> 
> Filesystems using the vfs helper mount_bdev (the ext filesystems, xfs, etc) and
> gfs2 already do this (for the former it is a freebie), so for these there is
> no need to iterate through the list of superblocks; we can get the superblock
> directly from ->bd_super which is more efficient and what this patch
> implements.
> 
> A multi-device filesystem (once again lile btrfs) can use that field to store
> a pointer to the superblock for each block device in its storage pool. By
> doing so it would get_active_super and, by extension, thaw_bdev initiated
> freezes working.
> 
> Thanks go to Josef Bacik and Christoph Hellwig for initiating this effort
> to fix btrfs and for suggesting the solution implemented here, respectively.
  Looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>

									Honza
> 
> Cc: linux-fsdevel@vger.kernel.org
> Cc: linux-btrfs@vger.kernel.org
> Cc: Josef Bacik <jbacik@fusionio.com>
> Cc: Eric Sandeen <sandeen@redhat.com>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Dave Chinner <dchinner@redhat.com>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Luiz Capitulino <lcapitulino@redhat.com>
> Cc: Chris Mason <chris.mason@fusionio.com>
> Signed-off-by: Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp>
> ---
> 
> diff -urNp linux-3.8-rc1-orig/fs/super.c linux-3.8-rc1/fs/super.c
> --- linux-3.8-rc1-orig/fs/super.c	2012-12-25 16:35:43.100018000 +0900
> +++ linux-3.8-rc1/fs/super.c	2012-12-25 16:36:22.312018000 +0900
> @@ -632,26 +632,29 @@ struct super_block *get_super(struct blo
>  		return NULL;
>  
>  	spin_lock(&sb_lock);
> +	if ((sb = bdev->bd_super) != NULL)
> +		goto out_found;
>  rescan:
>  	list_for_each_entry(sb, &super_blocks, s_list) {
>  		if (hlist_unhashed(&sb->s_instances))
>  			continue;
> -		if (sb->s_bdev == bdev) {
> -			sb->s_count++;
> -			spin_unlock(&sb_lock);
> -			down_read(&sb->s_umount);
> -			/* still alive? */
> -			if (sb->s_root && (sb->s_flags & MS_BORN))
> -				return sb;
> -			up_read(&sb->s_umount);
> -			/* nope, got unmounted */
> -			spin_lock(&sb_lock);
> -			__put_super(sb);
> -			goto rescan;
> -		}
> +		if (sb->s_bdev == bdev)
> +			goto out_found;
>  	}
>  	spin_unlock(&sb_lock);
>  	return NULL;
> +out_found:
> +	sb->s_count++;
> +	spin_unlock(&sb_lock);
> +	down_read(&sb->s_umount);
> +	/* still alive? */
> +	if (sb->s_root && (sb->s_flags & MS_BORN))
> +		return sb;
> +	up_read(&sb->s_umount);
> +	/* nope, got unmounted */
> +	spin_lock(&sb_lock);
> +	__put_super(sb);
> +	goto rescan;
>  }
>  
>  EXPORT_SYMBOL(get_super);
> @@ -696,18 +699,22 @@ struct super_block *get_active_super(str
>  
>  restart:
>  	spin_lock(&sb_lock);
> +	if ((sb = bdev->bd_super) != NULL)
> +		goto out_grabsuper;
>  	list_for_each_entry(sb, &super_blocks, s_list) {
>  		if (hlist_unhashed(&sb->s_instances))
>  			continue;
> -		if (sb->s_bdev == bdev) {
> -			if (grab_super(sb)) /* drops sb_lock */
> -				return sb;
> -			else
> -				goto restart;
> -		}
> +		if (sb->s_bdev == bdev)
> +			goto out_grabsuper;
>  	}
>  	spin_unlock(&sb_lock);
>  	return NULL;
> +
> +out_grabsuper:
> +	if (grab_super(sb)) /* drops sb_lock */
> +		return sb;
> +	else
> +		goto restart;
>  }
>   
>  struct super_block *user_get_super(dev_t dev)
> 
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html