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

[Michal Hocko]

On Thu 23-11-17 13:26:16, Jan Kara wrote:
> On Thu 23-11-17 12:52:47, Michal Hocko wrote:
[...]
> > @@ -489,6 +489,7 @@ struct super_block *sget_userns(struct file_system_type *type,
> >  				continue;
> >  			if (user_ns != old->s_user_ns) {
> >  				spin_unlock(&sb_lock);
> > +				unregister_shrinker(&s->s_shrink);
> 
> This is wrong as 's' can be NULL at this point.

Ohh, I've seen destroy_unused_super(s) and thought it operates on
non-NULL s. My bad.

> I think the right fix is to
> move unregister_shrinker() into destroy_unused_super(). But for that we
> need a reliable way to detect whether the shrinker has been already
> registered - possibly by initializing sb->shrinker.list in alloc_super()
> and then checking for list_empty() in destroy_unused_super().

Yeah, that makes sense.

> Also I'd note that early shrinker registration breaks assumption of
> destroy_unused_super() that nobody could have seen the superblock -
> shrinkers could have - but since shrinker code doesn't use RCU to access
> the superblock, we are fine. But still comment before
> destroy_unused_super() should be probably updated.

Right. Thanks a lot for the review Jan!

What about the following?
---