Re: [PATCH v3 4/8] scsi: call scsi_stop_queue() without state_mutex held

Martin Wilck
2023-06-07T19:38:10Z

On Wed, 2023-06-07 at 12:16 -0700, Bart Van Assche wrote:
> On 6/7/23 11:22, mwilck@suse.com wrote:
> > From: Martin Wilck <mwilck@suse.com>
> >
> > sdev->state_mutex protects only sdev->sdev_state. There's no reason
> > to keep it held while calling scsi_stop_queue().
> >
> > Signed-off-by: Martin Wilck <mwilck@suse.com>
> > ---
> >   drivers/scsi/scsi_lib.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> > index ce5788643011..26e7ce25fa05 100644
> > --- a/drivers/scsi/scsi_lib.c
> > +++ b/drivers/scsi/scsi_lib.c
> > @@ -2795,9 +2795,9 @@ static void scsi_device_block(struct
> > scsi_device *sdev, void *data)
> >
> >         mutex_lock(&sdev->state_mutex);
> >         err = __scsi_internal_device_block_nowait(sdev);
> > +       mutex_unlock(&sdev->state_mutex);
> >         if (err == 0)
> >                 scsi_stop_queue(sdev, false);
> > -       mutex_unlock(&sdev->state_mutex);
> >
> >         WARN_ONCE(err, "__scsi_internal_device_block_nowait(%s)
> > failed: err = %d\n",
> >                   dev_name(&sdev->sdev_gendev), err);
>
> There is a reason why scsi_stop_queue() is called with the sdev state
> mutex held: if this mutex is not held, unblocking of a SCSI device
> can
> start before the scsi_stop_queue() call has finished. It is not
> allowed
> to swap the order of the blk_mq_quiesce_queue() and
> blk_mq_unquiesce_queue() calls.

Thanks. This wasn't obvious to me from the current code. I'll add a
comment in the next version.

Regards
Martin