On Tue 16-06-26 16:08:18, Christian Brauner wrote:
> The superblock carries two counters: s_active, the active reference
> count that keeps the filesystem usable, and s_count, the passive
> reference count that merely keeps the structure itself alive. Turn the
> passive count into a refcount_t and rename it to s_passive to make the
> pairing with s_active obvious.
> 
> Everything is still serialized by sb_lock, so there is no functional
> change; the conversion buys the usual refcount_t saturation and
> underflow checking. The following patches start dropping passive
> references without holding sb_lock and make the device-to-superblock
> table hold one passive reference per registered entry, which a plain
> integer cannot support.
> 
> Signed-off-by: Christian Brauner (Amutable) <[email protected]>

Yeah, looks like a reasonable cleanup. Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

                                                                Honza

> ---
>  fs/super.c                     | 18 +++++++++---------
>  include/linux/fs/super_types.h |  2 +-
>  2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/super.c b/fs/super.c
> index a8fd61136aaf..25dd72b550e0 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -102,7 +102,7 @@ static bool super_flags(const struct super_block *sb, 
> unsigned int flags)
>   * creation will succeed and SB_BORN is set by vfs_get_tree() or we're
>   * woken and we'll see SB_DYING.
>   *
> - * The caller must have acquired a temporary reference on @sb->s_count.
> + * The caller must have acquired a temporary reference on @sb->s_passive.
>   *
>   * Return: The function returns true if SB_BORN was set and with
>   *         s_umount held. The function returns false if SB_DYING was
> @@ -367,7 +367,7 @@ static struct super_block *alloc_super(struct 
> file_system_type *type, int flags,
>       spin_lock_init(&s->s_inode_wblist_lock);
>       fserror_mount(s);
>  
> -     s->s_count = 1;
> +     refcount_set(&s->s_passive, 1);
>       atomic_set(&s->s_active, 1);
>       mutex_init(&s->s_vfs_rename_mutex);
>       lockdep_set_class(&s->s_vfs_rename_mutex, &type->s_vfs_rename_key);
> @@ -407,7 +407,7 @@ static struct super_block *alloc_super(struct 
> file_system_type *type, int flags,
>   */
>  static void __put_super(struct super_block *s)
>  {
> -     if (!--s->s_count) {
> +     if (refcount_dec_and_test(&s->s_passive)) {
>               list_del_init(&s->s_list);
>               WARN_ON(s->s_dentry_lru.node);
>               WARN_ON(s->s_inode_lru.node);
> @@ -529,7 +529,7 @@ static bool grab_super(struct super_block *sb)
>  {
>       bool locked;
>  
> -     sb->s_count++;
> +     refcount_inc(&sb->s_passive);
>       spin_unlock(&sb_lock);
>       locked = super_lock_excl(sb);
>       if (locked) {
> @@ -556,7 +556,7 @@ static bool grab_super(struct super_block *sb)
>   *   lock held in read mode in case of success. On successful return,
>   *   the caller must drop the s_umount lock when done.
>   *
> - *   Note that unlike get_super() et.al. this one does *not* bump ->s_count.
> + *   Note that unlike get_super() et.al. this one does *not* bump 
> ->s_passive.
>   *   The reason why it's safe is that we are OK with doing trylock instead
>   *   of down_read().  There's a couple of places that are OK with that, but
>   *   it's very much not a general-purpose interface.
> @@ -858,7 +858,7 @@ static void __iterate_supers(void (*f)(struct super_block 
> *, void *), void *arg,
>            sb = next_super(sb, flags)) {
>               if (super_flags(sb, SB_DYING))
>                       continue;
> -             sb->s_count++;
> +             refcount_inc(&sb->s_passive);
>               spin_unlock(&sb_lock);
>  
>               if (flags & SUPER_ITER_UNLOCKED) {
> @@ -903,7 +903,7 @@ void iterate_supers_type(struct file_system_type *type,
>               if (super_flags(sb, SB_DYING))
>                       continue;
>  
> -             sb->s_count++;
> +             refcount_inc(&sb->s_passive);
>               spin_unlock(&sb_lock);
>  
>               locked = super_lock_shared(sb);
> @@ -935,7 +935,7 @@ struct super_block *user_get_super(dev_t dev, bool excl)
>               if (sb->s_dev != dev)
>                       continue;
>  
> -             sb->s_count++;
> +             refcount_inc(&sb->s_passive);
>               spin_unlock(&sb_lock);
>  
>               locked = super_lock(sb, excl);
> @@ -1369,7 +1369,7 @@ static struct super_block *bdev_super_lock(struct 
> block_device *bdev, bool excl)
>  
>       /* Make sure sb doesn't go away from under us */
>       spin_lock(&sb_lock);
> -     sb->s_count++;
> +     refcount_inc(&sb->s_passive);
>       spin_unlock(&sb_lock);
>  
>       mutex_unlock(&bdev->bd_holder_lock);
> diff --git a/include/linux/fs/super_types.h b/include/linux/fs/super_types.h
> index ef7941e9dc79..68747182abf9 100644
> --- a/include/linux/fs/super_types.h
> +++ b/include/linux/fs/super_types.h
> @@ -145,7 +145,7 @@ struct super_block {
>       unsigned long                           s_magic;
>       struct dentry                           *s_root;
>       struct rw_semaphore                     s_umount;
> -     int                                     s_count;
> +     refcount_t                              s_passive;
>       atomic_t                                s_active;
>  #ifdef CONFIG_SECURITY
>       void                                    *s_security;
> 
> -- 
> 2.47.3
> 
-- 
Jan Kara <[email protected]>
SUSE Labs, CR

Reply via email to