Re: [PATCH 1/9] vfs: add __iterate_supers() and helpers around it

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri 05-10-12 14:31:46, Fernando Luis Vázquez Cao wrote:
> iterate_supers() calls a function provided by the caller with the s_umount
> semaphore taken in read mode. However, there may be cases where write mode
> is preferable, so we add __iterate_supers(), which lets one
> specify the mode of the lock, and replace iterate_supers with two helpers
> around __iterate_supers(), iterate_supers_read() and iterate_supers_write().
> 
> This will be used to fix the emergency thaw (filesystem unfreeze) code, which
> iterates over the list of superblocks but needs to hold the s_umount semaphore
> in _write_ mode bebore carrying out the actual thaw operation.
> 
> This patch introduces no semantic changes since current iterate_supers()
> callers are just updated to use iterate_supers_read() instead.
  The patch looks good. You can add:
Reviewed-by: Jan Kara <jack@xxxxxxx>

									Honza
> 
> Cc: Josef Bacik <jbacik@xxxxxxxxxxxx>
> Cc: Eric Sandeen <sandeen@xxxxxxxxxx>
> Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>
> Cc: Jan Kara <jack@xxxxxxx>
> Cc: Dave Chinner <dchinner@xxxxxxxxxx>
> Signed-off-by: Fernando Luis Vazquez Cao <fernando@xxxxxxxxxxxxx>
> ---
> 
> diff -urNp linux-3.6.0-rc7-orig/fs/buffer.c linux-3.6.0-rc7/fs/buffer.c
> --- linux-3.6.0-rc7-orig/fs/buffer.c	2012-09-24 13:45:05.569520902 +0900
> +++ linux-3.6.0-rc7/fs/buffer.c	2012-09-26 13:06:06.578342989 +0900
> @@ -521,7 +521,7 @@ static void do_thaw_one(struct super_blo
>  
>  static void do_thaw_all(struct work_struct *work)
>  {
> -	iterate_supers(do_thaw_one, NULL);
> +	iterate_supers_read(do_thaw_one, NULL);
>  	kfree(work);
>  	printk(KERN_WARNING "Emergency Thaw complete\n");
>  }
> diff -urNp linux-3.6.0-rc7-orig/fs/drop_caches.c linux-3.6.0-rc7/fs/drop_caches.c
> --- linux-3.6.0-rc7-orig/fs/drop_caches.c	2012-07-22 05:58:29.000000000 +0900
> +++ linux-3.6.0-rc7/fs/drop_caches.c	2012-09-26 13:06:06.582342990 +0900
> @@ -59,7 +59,7 @@ int drop_caches_sysctl_handler(ctl_table
>  		return ret;
>  	if (write) {
>  		if (sysctl_drop_caches & 1)
> -			iterate_supers(drop_pagecache_sb, NULL);
> +			iterate_supers_read(drop_pagecache_sb, NULL);
>  		if (sysctl_drop_caches & 2)
>  			drop_slab();
>  	}
> diff -urNp linux-3.6.0-rc7-orig/fs/quota/quota.c linux-3.6.0-rc7/fs/quota/quota.c
> --- linux-3.6.0-rc7-orig/fs/quota/quota.c	2012-09-24 13:45:06.105520990 +0900
> +++ linux-3.6.0-rc7/fs/quota/quota.c	2012-09-26 13:06:06.594342992 +0900
> @@ -58,7 +58,7 @@ static int quota_sync_all(int type)
>  		return -EINVAL;
>  	ret = security_quotactl(Q_SYNC, type, 0, NULL);
>  	if (!ret)
> -		iterate_supers(quota_sync_one, &type);
> +		iterate_supers_read(quota_sync_one, &type);
>  	return ret;
>  }
>  
> diff -urNp linux-3.6.0-rc7-orig/fs/super.c linux-3.6.0-rc7/fs/super.c
> --- linux-3.6.0-rc7-orig/fs/super.c	2012-09-24 13:45:06.129520994 +0900
> +++ linux-3.6.0-rc7/fs/super.c	2012-09-26 13:14:39.462346665 +0900
> @@ -537,14 +537,16 @@ void drop_super(struct super_block *sb)
>  EXPORT_SYMBOL(drop_super);
>  
>  /**
> - *	iterate_supers - call function for all active superblocks
> + *	__iterate_supers - call function for all active superblocks
>   *	@f: function to call
>   *	@arg: argument to pass to it
> + *	@wlock: mode of superblock lock (false->read lock, true->write lock)
>   *
>   *	Scans the superblock list and calls given function, passing it
>   *	locked superblock and given argument.
>   */
> -void iterate_supers(void (*f)(struct super_block *, void *), void *arg)
> +static void __iterate_supers(void (*f)(struct super_block *, void *), void *arg,
> +			     bool wlock)
>  {
>  	struct super_block *sb, *p = NULL;
>  
> @@ -555,10 +557,18 @@ void iterate_supers(void (*f)(struct sup
>  		sb->s_count++;
>  		spin_unlock(&sb_lock);
>  
> -		down_read(&sb->s_umount);
> +		if (wlock)
> +			down_write(&sb->s_umount);
> +		else
> +			down_read(&sb->s_umount);
> +
>  		if (sb->s_root && (sb->s_flags & MS_BORN))
>  			f(sb, arg);
> -		up_read(&sb->s_umount);
> +
> +		if (wlock)
> +			up_write(&sb->s_umount);
> +		else
> +			up_read(&sb->s_umount);
>  
>  		spin_lock(&sb_lock);
>  		if (p)
> @@ -571,6 +581,34 @@ void iterate_supers(void (*f)(struct sup
>  }
>  
>  /**
> + *	iterate_supers_read - call function for all active superblocks
> + *	@f: function to call
> + *	@arg: argument to pass to it
> + *
> + *	Scans the superblock list and calls given function, passing it
> + *	a superblock locked in _read_ mode and given argument. The lock
> + *	is automatically relased after the function returns.
> + */
> +void iterate_supers_read(void (*f)(struct super_block *, void *), void *arg)
> +{
> +	__iterate_supers(f, arg , false);
> +}
> +
> +/**
> + *	iterate_supers_write - call function for all active superblocks
> + *	@f: function to call
> + *	@arg: argument to pass to it
> + *
> + *	Scans the superblock list and calls given function, passing it
> + *	a superblock locked in _write_ mode and given argument. The lock
> + *	is automatically relased after the function returns.
> + */
> +void iterate_supers_write(void (*f)(struct super_block *, void *), void *arg)
> +{
> +	__iterate_supers(f, arg , true);
> +}
> +
> +/**
>   *	iterate_supers_type - call function for superblocks of given type
>   *	@type: fs type
>   *	@f: function to call
> diff -urNp linux-3.6.0-rc7-orig/fs/sync.c linux-3.6.0-rc7/fs/sync.c
> --- linux-3.6.0-rc7-orig/fs/sync.c	2012-09-24 13:45:06.129520994 +0900
> +++ linux-3.6.0-rc7/fs/sync.c	2012-09-26 13:06:06.594342992 +0900
> @@ -104,9 +104,9 @@ SYSCALL_DEFINE0(sync)
>  	int nowait = 0, wait = 1;
>  
>  	wakeup_flusher_threads(0, WB_REASON_SYNC);
> -	iterate_supers(sync_inodes_one_sb, NULL);
> -	iterate_supers(sync_fs_one_sb, &nowait);
> -	iterate_supers(sync_fs_one_sb, &wait);
> +	iterate_supers_read(sync_inodes_one_sb, NULL);
> +	iterate_supers_read(sync_fs_one_sb, &nowait);
> +	iterate_supers_read(sync_fs_one_sb, &wait);
>  	iterate_bdevs(fdatawrite_one_bdev, NULL);
>  	iterate_bdevs(fdatawait_one_bdev, NULL);
>  	if (unlikely(laptop_mode))
> @@ -122,11 +122,11 @@ static void do_sync_work(struct work_str
>  	 * Sync twice to reduce the possibility we skipped some inodes / pages
>  	 * because they were temporarily locked
>  	 */
> -	iterate_supers(sync_inodes_one_sb, &nowait);
> -	iterate_supers(sync_fs_one_sb, &nowait);
> +	iterate_supers_read(sync_inodes_one_sb, &nowait);
> +	iterate_supers_read(sync_fs_one_sb, &nowait);
>  	iterate_bdevs(fdatawrite_one_bdev, NULL);
> -	iterate_supers(sync_inodes_one_sb, &nowait);
> -	iterate_supers(sync_fs_one_sb, &nowait);
> +	iterate_supers_read(sync_inodes_one_sb, &nowait);
> +	iterate_supers_read(sync_fs_one_sb, &nowait);
>  	iterate_bdevs(fdatawrite_one_bdev, NULL);
>  	printk("Emergency Sync complete\n");
>  	kfree(work);
> diff -urNp linux-3.6.0-rc7-orig/include/linux/fs.h linux-3.6.0-rc7/include/linux/fs.h
> --- linux-3.6.0-rc7-orig/include/linux/fs.h	2012-09-24 13:45:06.301521033 +0900
> +++ linux-3.6.0-rc7/include/linux/fs.h	2012-09-26 13:06:06.598342993 +0900
> @@ -2684,7 +2684,8 @@ extern struct super_block *get_super(str
>  extern struct super_block *get_super_thawed(struct block_device *);
>  extern struct super_block *get_active_super(struct block_device *bdev);
>  extern void drop_super(struct super_block *sb);
> -extern void iterate_supers(void (*)(struct super_block *, void *), void *);
> +extern void iterate_supers_read(void (*)(struct super_block *, void *), void *);
> +extern void iterate_supers_write(void (*)(struct super_block *, void *), void *);
>  extern void iterate_supers_type(struct file_system_type *,
>  			        void (*)(struct super_block *, void *), void *);
>  
> diff -urNp linux-3.6.0-rc7-orig/security/selinux/hooks.c linux-3.6.0-rc7/security/selinux/hooks.c
> --- linux-3.6.0-rc7-orig/security/selinux/hooks.c	2012-09-24 13:45:08.121521275 +0900
> +++ linux-3.6.0-rc7/security/selinux/hooks.c	2012-09-26 13:06:06.610342995 +0900
> @@ -5755,7 +5755,7 @@ void selinux_complete_init(void)
>  
>  	/* Set up any superblocks initialized prior to the policy load. */
>  	printk(KERN_DEBUG "SELinux:  Setting up existing superblocks.\n");
> -	iterate_supers(delayed_superblock_init, NULL);
> +	iterate_supers_read(delayed_superblock_init, NULL);
>  }
>  
>  /* SELinux requires early initialization in order to label
> 
> 
-- 
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux