Hello James,
On Mon, Nov 08, 2021 at 11:42:01AM -0500, James Bottomley wrote:
> On Wed, 2021-11-03 at 11:43 +0800, Ming Lei wrote:
> [...]
> > +void scsi_start_queue(struct scsi_device *sdev)
> > +{
> > + if (cmpxchg(&sdev->queue_stopped, 1, 0))
> > + blk_mq_unquiesce_queue(sdev->request_queue);
> > +}
> > +
> > +static void scsi_stop_queue(struct scsi_device *sdev, bool nowait)
> > +{
> > + if (!cmpxchg(&sdev->queue_stopped, 0, 1)) {
> > + if (nowait)
> > + blk_mq_quiesce_queue_nowait(sdev-
> > >request_queue);
> > + else
> > + blk_mq_quiesce_queue(sdev->request_queue);
> > + } else {
> > + if (!nowait)
> > + blk_mq_wait_quiesce_done(sdev->request_queue);
> > + }
> > +}
>
> This looks counter intuitive. I assume it's done so that if we call
> scsi_stop_queue when the queue has already been stopped, it waits until
The motivation is to balance blk_mq_quiesce_queue_nowait()/blk_mq_quiesce_queue()
and blk_mq_unquiesce_queue().
That needs one extra mutex to cover the quiesce action and update
the flag, but we can't hold the mutex in scsi_internal_device_block_nowait(),
so take this way with the atomic flag.
> the queue is actually quiesced before returning so the behaviour is the
> same in the !nowait case? Some sort of comment explaining that would
> be useful.
I will add comment on the current usage.
Thanks,
Ming
