Re: [PATCH] fs: handle shrinker registration failure in sget_userns

Tetsuo Handa
2017-11-23T13:35:52Z

Jan Kara wrote:
> Looks good to me now. You can add:
>
> Reviewed-by: Jan Kara <jack@suse.cz>
>

It does not look good to me, for "goto fail" can call
destroy_unused_super() before s->s_shrink.list is initialized.
Also, the comment block saying "this object isn't exposed yet"
wants to be updated?

---
fs/super.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/fs/super.c b/fs/super.c
index 80b118c..44f0c6b 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -197,6 +197,7 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags,
if (!s)
return NULL;

+ INIT_LIST_HEAD(&s->s_shrink.list);
INIT_LIST_HEAD(&s->s_mounts);
s->s_user_ns = get_user_ns(user_ns);

@@ -260,9 +261,8 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags,
s->s_shrink.count_objects = super_cache_count;
s->s_shrink.batch = 1024;
s->s_shrink.flags = SHRINKER_NUMA_AWARE | SHRINKER_MEMCG_AWARE;
- INIT_LIST_HEAD(&s->s_shrink.list);
- return s;
-
+ if (register_shrinker(&s->s_shrink) == 0)
+ return s;
fail:
destroy_unused_super(s);
return NULL;
@@ -512,10 +512,6 @@ struct super_block *sget_userns(struct file_system_type *type,
s = alloc_super(type, (flags & ~SB_SUBMOUNT), user_ns);
if (!s)
return ERR_PTR(-ENOMEM);
- if (register_shrinker(&s->s_shrink)) {
- destroy_unused_super(s);
- return ERR_PTR(-ENOMEM);
- }
goto retry;
}

--
1.8.3.1