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

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

On Fri, 2016-11-04 at 12:17 -0600, Bart Van Assche wrote:
> On 11/04/2016 07:47 AM, James Bottomley wrote:
> > You know after
> > 
> > if (device_remove_file_self(dev, attr))
> > 
> > returns true that s_active is held and also that KERNFS_SUICIDAL is 
> > set on the node, so the non-self remove paths can simply check for 
> > this flag and return without descending into __kernfs_remove(), 
> > which would mean they never take s_active.  That means we never get 
> > the inversion.
> 
> Hello James,
> 
> The lock inversion is not triggered by the non-self remove paths but 
> by the self-remove path.

I think we should agree first what the problem is.  The inversion
occurs between the sysfs delete path and the device node delete caused
by a remove host.  When both are happening the inversion is that when

	if (device_remove_file_self(dev, attr))
		scsi_remove_device(to_scsi_device(dev));

Happens, after the if, the s_active lock is held then
scsi_remove_device goes on to take the scan_mutex.

Conversely in scsi_remove_host, the mutex is taken first then
scsi_forget_host iterates removing the devices, but sysfs file removal
eventually takes s_active in kernfs_drain, which is called from
kernfs_remove via kernfs_remove_by_name_ns, hence the inversion.

This is therefore a conflict between the self and non-self remove
paths.

> Anyway, can you have a look at the two attached 
> patches?

Well, they're still overly complex, but perhaps due to the
misunderstanding above?  If you look through that trigger case, you'll
see that the fix is simply to check KERNFS_SUICIDAL in
kernfs_remove_by_name_ns and not descend into __kernfs_remove() if it's
set.  I think kernfs_mutex mediates this, but probably only if it's
moved lower down in kernfs_drain.

James