On Tue, 2023-01-24 at 10:01 -0800, Bart Van Assche wrote:
> On 1/24/23 07:49, mwilck@xxxxxxxx wrote:
> > From: Martin Wilck <mwilck@xxxxxxxx>
> >
> > Since the might_sleep() annotation was added in scsi_device_put()
> > and
> > alua_rtpg_queue(), we have seen repeated reports of "BUG: sleeping
> > function
> > called from invalid context" [1], [2]. alua_rtpg_queue() is always
> > called
> > from contexts where the caller must hold at least one reference to
> > the scsi
> > device in question. This means that the reference taken by
> > alua_rtpg_queue() itself can't be the last one, and thus can be
> > dropped
> > without entering the code path in which scsi_device_put() might
> > actually
> > sleep.
> >
> > Add a new helper function, scsi_device_put_nosleep() for cases like
> > this,
> > where a device reference is put from atomic context, and at the
> > same time
> > it is certain that this reference is not the last one, and use it
> > from
> > alua_rtpg_queue().
>
> Something I should have asked earlier, has this alternative been
> considered? This alternative has the advantage that no new functions
> are
> introduced.
Looks good to me, too. No, I didn't have this idea before.
Martin
>
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 1426b9b03612..9feb0323bc44 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -588,8 +588,6 @@ void scsi_device_put(struct scsi_device *sdev)
> {
> struct module *mod = sdev->host->hostt->module;
>
> - might_sleep();
> -
> put_device(&sdev->sdev_gendev);
> module_put(mod);
> }
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index 981d1bab2120..8ef9a5494340 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -452,5 +451,7 @@ static void scsi_device_dev_release(struct device
> unsigned long flags;
>
> + might_sleep();
> +
> scsi_dh_release_device(sdev);
>
> parent = sdev->sdev_gendev.parent;
>
> Thanks,
>
> Bart.
