Re: [PATCH] Avoid that SCSI device removal through sysfs triggers a deadlock

[James Bottomley <jejb@linux.vnet.ibm.com>]

On Tue, 2016-11-08 at 08:52 -0800, Bart Van Assche wrote:
> On 11/08/2016 07:28 AM, James Bottomley wrote:
> > On Mon, 2016-11-07 at 16:32 -0800, Bart Van Assche wrote:
> > > diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
> > > index cf4c636..44ec536 100644
> > > --- a/fs/kernfs/dir.c
> > > +++ b/fs/kernfs/dir.c
> > > @@ -1410,7 +1410,7 @@ int kernfs_remove_by_name_ns(struct
> > > kernfs_node
> > > *parent, const char *name,
> > >  	mutex_lock(&kernfs_mutex);
> > > 
> > >  	kn = kernfs_find_ns(parent, name, ns);
> > > -	if (kn)
> > > +	if (kn && !(kn->flags & KERNFS_SUICIDED))
> > 
> > Actually, wrong flag, you need KERNFS_SUICIDAL.  The reason is that
> > kernfs_mutex is actually dropped half way through __kernfs_remove, 
> > so KERNFS_SUICIDED is not set atomically with this mutex.
> 
> Hello James,
> 
> Sorry but what you wrote is not correct.

I think you agree it is dropped.  I don't need to add the bit about the
reacquisition because the race is mediated by the first acquisition not
the second one, if you mediate on KERNFS_SUICIDAL, you only need to
worry about this because the mediation is in the first acquisition.  If
you mediate on KERNFS_SUICIDED, you need to explain that the final
thing that means the race can't happen is the unbreak in the sysfs
delete path re-acquiring s_active ... the explanation of what's going
on and why gets about 2x more complex.

James

>  __kernfs_remove() calls kernfs_drain(). That last function not only 
> drops but also reacquires kernfs_mutex. So both KERNFS_SUICIDAL and 
> KERNFS_SUICIDED are set while holding kernfs_mutex.