Re: [PATCH 1/3] super: use super_{lock,unlock}()

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

 



On Thu 17-08-23 12:47:42, Christian Brauner wrote:
> Replace the open-coded {down,up}_{read,write}() calls with simple
> wrappers. Follow-up patches will benefit from this as well.
> 
> Signed-off-by: Christian Brauner <brauner@xxxxxxxxxx>

Looks good. Except we already have a wrapper trylock_super() which is now
inconsistently named. So that should be renamed as well.

								Honza

> ---
>  fs/super.c | 126 ++++++++++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 78 insertions(+), 48 deletions(-)
> 
> diff --git a/fs/super.c b/fs/super.c
> index c878e7373f93..878675921bdc 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -50,6 +50,42 @@ static char *sb_writers_name[SB_FREEZE_LEVELS] = {
>  	"sb_internal",
>  };
>  
> +static inline void super_lock(struct super_block *sb, bool excl)
> +{
> +	if (excl)
> +		down_write(&sb->s_umount);
> +	else
> +		down_read(&sb->s_umount);
> +}
> +
> +static inline void super_unlock(struct super_block *sb, bool excl)
> +{
> +	if (excl)
> +		up_write(&sb->s_umount);
> +	else
> +		up_read(&sb->s_umount);
> +}
> +
> +static inline void super_lock_write(struct super_block *sb)
> +{
> +	super_lock(sb, true);
> +}
> +
> +static inline void super_lock_read(struct super_block *sb)
> +{
> +	super_lock(sb, false);
> +}
> +
> +static inline void super_unlock_write(struct super_block *sb)
> +{
> +	super_unlock(sb, true);
> +}
> +
> +static inline void super_unlock_read(struct super_block *sb)
> +{
> +	super_unlock(sb, false);
> +}
> +
>  /*
>   * One thing we have to be careful of with a per-sb shrinker is that we don't
>   * drop the last active reference to the superblock from within the shrinker.
> @@ -110,7 +146,7 @@ static unsigned long super_cache_scan(struct shrinker *shrink,
>  		freed += sb->s_op->free_cached_objects(sb, sc);
>  	}
>  
> -	up_read(&sb->s_umount);
> +	super_unlock_read(sb);
>  	return freed;
>  }
>  
> @@ -176,7 +212,7 @@ static void destroy_unused_super(struct super_block *s)
>  {
>  	if (!s)
>  		return;
> -	up_write(&s->s_umount);
> +	super_unlock_write(s);
>  	list_lru_destroy(&s->s_dentry_lru);
>  	list_lru_destroy(&s->s_inode_lru);
>  	security_sb_free(s);
> @@ -340,7 +376,7 @@ void deactivate_locked_super(struct super_block *s)
>  		put_filesystem(fs);
>  		put_super(s);
>  	} else {
> -		up_write(&s->s_umount);
> +		super_unlock_write(s);
>  	}
>  }
>  
> @@ -357,7 +393,7 @@ EXPORT_SYMBOL(deactivate_locked_super);
>  void deactivate_super(struct super_block *s)
>  {
>  	if (!atomic_add_unless(&s->s_active, -1, 1)) {
> -		down_write(&s->s_umount);
> +		super_lock_write(s);
>  		deactivate_locked_super(s);
>  	}
>  }
> @@ -381,12 +417,12 @@ static int grab_super(struct super_block *s) __releases(sb_lock)
>  {
>  	s->s_count++;
>  	spin_unlock(&sb_lock);
> -	down_write(&s->s_umount);
> +	super_lock_write(s);
>  	if ((s->s_flags & SB_BORN) && atomic_inc_not_zero(&s->s_active)) {
>  		put_super(s);
>  		return 1;
>  	}
> -	up_write(&s->s_umount);
> +	super_unlock_write(s);
>  	put_super(s);
>  	return 0;
>  }
> @@ -414,7 +450,7 @@ bool trylock_super(struct super_block *sb)
>  		if (!hlist_unhashed(&sb->s_instances) &&
>  		    sb->s_root && (sb->s_flags & SB_BORN))
>  			return true;
> -		up_read(&sb->s_umount);
> +		super_unlock_read(sb);
>  	}
>  
>  	return false;
> @@ -439,13 +475,13 @@ bool trylock_super(struct super_block *sb)
>  void retire_super(struct super_block *sb)
>  {
>  	WARN_ON(!sb->s_bdev);
> -	down_write(&sb->s_umount);
> +	super_lock_write(sb);
>  	if (sb->s_iflags & SB_I_PERSB_BDI) {
>  		bdi_unregister(sb->s_bdi);
>  		sb->s_iflags &= ~SB_I_PERSB_BDI;
>  	}
>  	sb->s_iflags |= SB_I_RETIRED;
> -	up_write(&sb->s_umount);
> +	super_unlock_write(sb);
>  }
>  EXPORT_SYMBOL(retire_super);
>  
> @@ -521,7 +557,7 @@ void generic_shutdown_super(struct super_block *sb)
>  	/* should be initialized for __put_super_and_need_restart() */
>  	hlist_del_init(&sb->s_instances);
>  	spin_unlock(&sb_lock);
> -	up_write(&sb->s_umount);
> +	super_unlock_write(sb);
>  	if (sb->s_bdi != &noop_backing_dev_info) {
>  		if (sb->s_iflags & SB_I_PERSB_BDI)
>  			bdi_unregister(sb->s_bdi);
> @@ -685,7 +721,7 @@ EXPORT_SYMBOL(sget);
>  
>  void drop_super(struct super_block *sb)
>  {
> -	up_read(&sb->s_umount);
> +	super_unlock_read(sb);
>  	put_super(sb);
>  }
>  
> @@ -693,7 +729,7 @@ EXPORT_SYMBOL(drop_super);
>  
>  void drop_super_exclusive(struct super_block *sb)
>  {
> -	up_write(&sb->s_umount);
> +	super_unlock_write(sb);
>  	put_super(sb);
>  }
>  EXPORT_SYMBOL(drop_super_exclusive);
> @@ -739,10 +775,10 @@ void iterate_supers(void (*f)(struct super_block *, void *), void *arg)
>  		sb->s_count++;
>  		spin_unlock(&sb_lock);
>  
> -		down_read(&sb->s_umount);
> +		super_lock_read(sb);
>  		if (sb->s_root && (sb->s_flags & SB_BORN))
>  			f(sb, arg);
> -		up_read(&sb->s_umount);
> +		super_unlock_read(sb);
>  
>  		spin_lock(&sb_lock);
>  		if (p)
> @@ -773,10 +809,10 @@ void iterate_supers_type(struct file_system_type *type,
>  		sb->s_count++;
>  		spin_unlock(&sb_lock);
>  
> -		down_read(&sb->s_umount);
> +		super_lock_read(sb);
>  		if (sb->s_root && (sb->s_flags & SB_BORN))
>  			f(sb, arg);
> -		up_read(&sb->s_umount);
> +		super_unlock_read(sb);
>  
>  		spin_lock(&sb_lock);
>  		if (p)
> @@ -813,7 +849,7 @@ struct super_block *get_active_super(struct block_device *bdev)
>  		if (sb->s_bdev == bdev) {
>  			if (!grab_super(sb))
>  				goto restart;
> -			up_write(&sb->s_umount);
> +			super_unlock_write(sb);
>  			return sb;
>  		}
>  	}
> @@ -833,17 +869,11 @@ struct super_block *user_get_super(dev_t dev, bool excl)
>  		if (sb->s_dev ==  dev) {
>  			sb->s_count++;
>  			spin_unlock(&sb_lock);
> -			if (excl)
> -				down_write(&sb->s_umount);
> -			else
> -				down_read(&sb->s_umount);
> +			super_lock(sb, excl);
>  			/* still alive? */
>  			if (sb->s_root && (sb->s_flags & SB_BORN))
>  				return sb;
> -			if (excl)
> -				up_write(&sb->s_umount);
> -			else
> -				up_read(&sb->s_umount);
> +			super_unlock(sb, excl);
>  			/* nope, got unmounted */
>  			spin_lock(&sb_lock);
>  			__put_super(sb);
> @@ -889,9 +919,9 @@ int reconfigure_super(struct fs_context *fc)
>  
>  	if (remount_ro) {
>  		if (!hlist_empty(&sb->s_pins)) {
> -			up_write(&sb->s_umount);
> +			super_unlock_write(sb);
>  			group_pin_kill(&sb->s_pins);
> -			down_write(&sb->s_umount);
> +			super_lock_write(sb);
>  			if (!sb->s_root)
>  				return 0;
>  			if (sb->s_writers.frozen != SB_UNFROZEN)
> @@ -954,7 +984,7 @@ int reconfigure_super(struct fs_context *fc)
>  
>  static void do_emergency_remount_callback(struct super_block *sb)
>  {
> -	down_write(&sb->s_umount);
> +	super_lock_write(sb);
>  	if (sb->s_root && sb->s_bdev && (sb->s_flags & SB_BORN) &&
>  	    !sb_rdonly(sb)) {
>  		struct fs_context *fc;
> @@ -967,7 +997,7 @@ static void do_emergency_remount_callback(struct super_block *sb)
>  			put_fs_context(fc);
>  		}
>  	}
> -	up_write(&sb->s_umount);
> +	super_unlock_write(sb);
>  }
>  
>  static void do_emergency_remount(struct work_struct *work)
> @@ -990,12 +1020,12 @@ void emergency_remount(void)
>  
>  static void do_thaw_all_callback(struct super_block *sb)
>  {
> -	down_write(&sb->s_umount);
> +	super_lock_write(sb);
>  	if (sb->s_root && sb->s_flags & SB_BORN) {
>  		emergency_thaw_bdev(sb);
>  		thaw_super_locked(sb);
>  	} else {
> -		up_write(&sb->s_umount);
> +		super_unlock_write(sb);
>  	}
>  }
>  
> @@ -1182,10 +1212,10 @@ EXPORT_SYMBOL(get_tree_keyed);
>   */
>  static bool lock_active_super(struct super_block *sb)
>  {
> -	down_read(&sb->s_umount);
> +	super_lock_read(sb);
>  	if (!sb->s_root ||
>  	    (sb->s_flags & (SB_ACTIVE | SB_BORN)) != (SB_ACTIVE | SB_BORN)) {
> -		up_read(&sb->s_umount);
> +		super_unlock_read(sb);
>  		return false;
>  	}
>  	return true;
> @@ -1208,7 +1238,7 @@ static void fs_bdev_mark_dead(struct block_device *bdev, bool surprise)
>  	if (sb->s_op->shutdown)
>  		sb->s_op->shutdown(sb);
>  
> -	up_read(&sb->s_umount);
> +	super_unlock_read(sb);
>  }
>  
>  static void fs_bdev_sync(struct block_device *bdev)
> @@ -1220,7 +1250,7 @@ static void fs_bdev_sync(struct block_device *bdev)
>  	if (!lock_active_super(sb))
>  		return;
>  	sync_filesystem(sb);
> -	up_read(&sb->s_umount);
> +	super_unlock_read(sb);
>  }
>  
>  const struct blk_holder_ops fs_holder_ops = {
> @@ -1342,9 +1372,9 @@ int get_tree_bdev(struct fs_context *fc,
>  		 * bdev_mark_dead()). It is safe because we have active sb
>  		 * reference and SB_BORN is not set yet.
>  		 */
> -		up_write(&s->s_umount);
> +		super_unlock_write(s);
>  		error = setup_bdev_super(s, fc->sb_flags, fc);
> -		down_write(&s->s_umount);
> +		super_lock_write(s);
>  		if (!error)
>  			error = fill_super(s, fc);
>  		if (error) {
> @@ -1394,9 +1424,9 @@ struct dentry *mount_bdev(struct file_system_type *fs_type,
>  		 * bdev_mark_dead()). It is safe because we have active sb
>  		 * reference and SB_BORN is not set yet.
>  		 */
> -		up_write(&s->s_umount);
> +		super_unlock_write(s);
>  		error = setup_bdev_super(s, flags, NULL);
> -		down_write(&s->s_umount);
> +		super_lock_write(s);
>  		if (!error)
>  			error = fill_super(s, data, flags & SB_SILENT ? 1 : 0);
>  		if (error) {
> @@ -1685,29 +1715,29 @@ int freeze_super(struct super_block *sb)
>  	int ret;
>  
>  	atomic_inc(&sb->s_active);
> -	down_write(&sb->s_umount);
> +	super_lock_write(sb);
>  	if (sb->s_writers.frozen != SB_UNFROZEN) {
>  		deactivate_locked_super(sb);
>  		return -EBUSY;
>  	}
>  
>  	if (!(sb->s_flags & SB_BORN)) {
> -		up_write(&sb->s_umount);
> +		super_unlock_write(sb);
>  		return 0;	/* sic - it's "nothing to do" */
>  	}
>  
>  	if (sb_rdonly(sb)) {
>  		/* Nothing to do really... */
>  		sb->s_writers.frozen = SB_FREEZE_COMPLETE;
> -		up_write(&sb->s_umount);
> +		super_unlock_write(sb);
>  		return 0;
>  	}
>  
>  	sb->s_writers.frozen = SB_FREEZE_WRITE;
>  	/* Release s_umount to preserve sb_start_write -> s_umount ordering */
> -	up_write(&sb->s_umount);
> +	super_unlock_write(sb);
>  	sb_wait_write(sb, SB_FREEZE_WRITE);
> -	down_write(&sb->s_umount);
> +	super_lock_write(sb);
>  
>  	/* Now we go and block page faults... */
>  	sb->s_writers.frozen = SB_FREEZE_PAGEFAULT;
> @@ -1743,7 +1773,7 @@ int freeze_super(struct super_block *sb)
>  	 */
>  	sb->s_writers.frozen = SB_FREEZE_COMPLETE;
>  	lockdep_sb_freeze_release(sb);
> -	up_write(&sb->s_umount);
> +	super_unlock_write(sb);
>  	return 0;
>  }
>  EXPORT_SYMBOL(freeze_super);
> @@ -1753,7 +1783,7 @@ static int thaw_super_locked(struct super_block *sb)
>  	int error;
>  
>  	if (sb->s_writers.frozen != SB_FREEZE_COMPLETE) {
> -		up_write(&sb->s_umount);
> +		super_unlock_write(sb);
>  		return -EINVAL;
>  	}
>  
> @@ -1770,7 +1800,7 @@ static int thaw_super_locked(struct super_block *sb)
>  			printk(KERN_ERR
>  				"VFS:Filesystem thaw failed\n");
>  			lockdep_sb_freeze_release(sb);
> -			up_write(&sb->s_umount);
> +			super_unlock_write(sb);
>  			return error;
>  		}
>  	}
> @@ -1790,7 +1820,7 @@ static int thaw_super_locked(struct super_block *sb)
>   */
>  int thaw_super(struct super_block *sb)
>  {
> -	down_write(&sb->s_umount);
> +	super_lock_write(sb);
>  	return thaw_super_locked(sb);
>  }
>  EXPORT_SYMBOL(thaw_super);
> 
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR



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

  Powered by Linux