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

[Linus Torvalds]

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?

                Linus