On Mon, Jul 17, 2017 at 12:20:56PM +0100, Roger Pau Monné wrote:
> On Sat, Jul 15, 2017 at 07:15:56AM +0800, Ming Lei wrote:
> > stopping queue may cause race and may not stop the queue really
> > after the API returns, and we have improved quiescing
> > interface and it really can block dispatching once it returns.
> >
> > So switch to quiesce/unquiece like what we did on other drivers
> > (NVMe, NBD, mtip32xx, ...)
> >
> > The blk_mq_stop_hw_queues() and blk_mq_start_stopped_hw_queues()
> > used in blkif_queue_rq() and blkif_interrupt() are for congestion
> > control, we leave it as it is since it is safe for this usage.
>
> Again I yet don't understand the difference between those two, neither
> why start/stop is not fixed instead of introducing quiesce/unquiece.
There are two usages covered by start/stop now:
- congestion control for handling queue busy(BLK_STS_RESOURCE), now
only xen-blkfront and virtio-blk use that
- other usages, such as in xlvbd_release_gendisk(), for blocking
IO to driver/device
For the 1st case, it is usually fine to use stop/start
For the 2nd case, stop queue isn't enough, and we can't guarantee
no IO is dispatched to device/driver after returning from stop queue,
for details. Most of this usage have been fixed by Sagi Grimberg:
http://marc.info/?t=149927415900006&r=1&w=2
start/stop is a bad name for 2nd usage too, what we really want
is to block IO to driver/devices, so we should use quiesce/unquiesce.
xen-blkfront is missed in Sagi's patchset which has been merged to
linus tree already, so this patch just fixes xen-blkfront simply like
other patches.
We can't use quiesce/unquiesce for replacing stop/start in the
case of BLK_STS_RESOURCE, because quiesce may sleep, and we needn't
block IO for this usage actually.
> Not to mention that start/stop is not documented, which makes all this
> even more fun.
Did you read comment of blk_mq_stop_hw_queue() and
blk_mq_stop_hw_queues() in linus tree?
>
> Anyway I would like to ask, is the way to re-start a stopped queue the
> same way to unquiece?
I don't know what your exact question, but it is definitely that
unquiesce is counter part of quiesce, and quiesce/unquiesce doesn't
depend on 'stopped' state any more if you take a look at the code.
>
> If not I would rather prefer that start/stop or quiece/unquiece is
> used exclusively, in order to not make the code even more complex. It
I do not think the code becomes more complex, please see the line change
of this patch:
1 file changed, 8 insertions(+), 14 deletions(-)
then see the change of the whole patchset:
8 files changed, 54 insertions(+), 129 deletions(-)
It is really a cleanup and simplifying.
> seems fairly easy to mess up and call "start" on a "quiesced" queue
> (or the other way around).
Definitely it shouldn't be worried because start/stop is removed
in this patchset.
--
Ming
