Re: [PATCH V8 07/11] blk-mq: stop to handle IO and drain IO before hctx becomes inactive

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

On Fri, Apr 24, 2020 at 12:38:51PM +0200, Christoph Hellwig wrote:
> On Fri, Apr 24, 2020 at 06:23:47PM +0800, Ming Lei wrote:
> > Before one CPU becomes offline, check if it is the last online CPU of hctx.
> > If yes, mark this hctx as inactive, meantime wait for completion of all
> > in-flight IOs originated from this hctx. Meantime check if this hctx has
> > become inactive in blk_mq_get_driver_tag(), if yes, release the
> > allocated tag.
> > 
> > This way guarantees that there isn't any inflight IO before shutdowning
> > the managed IRQ line when all CPUs of this IRQ line is offline.
> 
> Can you take a look at all my comments on the previous version here
> (splitting blk_mq_get_driver_tag for direct_issue vs not, what is

I am not sure if it helps by adding two helper, given only two
parameters are needed, and the new parameter is just a constant.

> the point of barrier(), smp_mb__before_atomic and
> smp_mb__after_atomic), as none seems to be addressed and I also didn't
> see a reply.

I believe it has been documented:

+   /*
+    * Add one memory barrier in case that direct issue IO process is
+    * migrated to other CPU which may not belong to this hctx, so we can
+    * order driver tag assignment and checking BLK_MQ_S_INACTIVE.
+    * Otherwise, barrier() is enough given both setting BLK_MQ_S_INACTIVE
+    * and driver tag assignment are run on the same CPU in case that
+    * BLK_MQ_S_INACTIVE is set.
+    */

OK, I can add more:

In case of not direct issue, __blk_mq_delay_run_hw_queue() guarantees
that dispatch is done on CPUs of this hctx.

In case of direct issue, the direct issue IO process may be migrated to
other CPU which doesn't belong to hctx->cpumask even though the chance
is quite small, but still possible.

This patch sets hctx as inactive in the last CPU of hctx, so barrier()
is enough for not direct issue. Otherwise, one smp_mb() is added for
ordering tag assignment(include setting rq) and checking S_INACTIVE in
blk_mq_get_driver_tag().



Thanks,
Ming