Re: [PATCH 3/4] scsi: make sure that request queue queiesce and unquiesce balanced

[Ming Lei <ming.lei@redhat.com>]

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