Re: [ata_scsi_offline_dev] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:238

[Hannes Reinecke]

On 11/07/2017 12:12 AM, Linus Torvalds wrote:
> On Mon, Nov 6, 2017 at 2:53 PM, Fengguang Wu <fengguang.wu@xxxxxxxxx> wrote:
>>
>> The same dmesg happen to contain another libata related bug. Attached again.
>> It's rare and in the error handling path, so unlikely a new regression.
>>
>> [   49.608280] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:238
>> [   49.647821]  mutex_lock+0x20/0x50
>> [   49.651443]  kernfs_find_and_get_ns+0x23/0x60
>> [   49.656104]  sysfs_notify+0x77/0x90
>> [   49.659900]  scsi_device_set_state+0x63/0x150
>> [   49.664559]  ata_scsi_offline_dev+0x1c/0x30 [libata]
>> [   49.669817]  ata_eh_detach_dev+0x3b/0xb0 [libata]
> 
> ata_eh_detach_dev() does
> 
>         spin_lock_irqsave(ap->lock, flags);
> 
> and then does
> 
>         if (ata_scsi_offline_dev(dev)) {
>                 dev->flags |= ATA_DFLAG_DETACHED;
>                 ap->pflags |= ATA_PFLAG_SCSI_HOTPLUG;
>         }
> 
> inside that spinlock. And this code is not new - it has done it since
> 2006 or so.
> 
> But it does seem to be a new regression in 4.14, caused by commit
> 8a97712e5314 ("scsi: make 'state' device attribute pollable"), because
> that's what added the sysfs_notify() call to scsi_device_set_state(),
> which made that spinlock be a problem.
> 
> That commit came in through the SCSI merge this merge window, and it
> seems to still revert cleanly.
> 
> So I do suspect that by now we should just revert that commit. It's
> not clear why that state attribute should be pollable, and the new
> code is clearly very much buggy.
> 
> Hannes, Martin?
> 
Seeing the complexity involved, yes, please revert that.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@xxxxxxx			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 NÃrnberg
GF: F. ImendÃrffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG NÃrnberg)