Thread (123 messages) 123 messages, 6 authors, 2020-08-18

Re: [PATCH RFC v7 10/12] megaraid_sas: switch fusion adapters to MQ

From: Ming Lei <hidden>
Date: 2020-08-06 16:48:31
Also in: linux-scsi

On Thu, Aug 06, 2020 at 08:07:38PM +0530, Kashyap Desai wrote:
quoted
quoted
Hi Ming -

There is still some race which is not handled.  Take a case of IO is
not able to get budget and it has already marked <restarts> flag.
<restarts> flag will be seen non-zero in completion path and
completion path will attempt h/w queue run. (But this particular IO is
still not in s/w queue.).
Attempt of running h/w queue from completion path will not flush any
IO since there is no IO in s/w queue.
Then where is the IO to be submitted in case of running out of budget?
Typical race in your latest patch is - (Lets consider command A,B and C)
Command A did not receive budget. Command B completed  (which was already
Command A doesn't get budget, and A is still in sw/scheduler queue
because we try to acquire budget before dequeuing request from sw/scheduler queue,
see __blk_mq_do_dispatch_sched() and blk_mq_do_dispatch_ctx().

Not consider direct issue, because the hw queue will be run explicitly
when not getting budget, see __blk_mq_try_issue_directly.

Not consider command A being added to hctx->dispatch too, because blk-mq will
re-run the queue, see blk_mq_dispatch_rq_list().

submitted earlier) at the same time and it make sdev->device_busy = 0 from
" scsi_finish_command".
Command B has still not called "scsi_end_request". Command C get the
budget and it will make sdev->device_busy = 1. Now, Command A set  set
sdev->restarts flags but will not run h/w queue since sdev->device_busy =
1.
Right.
Command B run h/w queue (make sdev->restart = 0) from completion path, but
command -A is still not in the s/w queue.
Then you didn't answer my question about where A is, did you?
Command-A is in now in s/w queue. Command-C completed but it will not run h/w queue because
sdev->restarts = 0.
Why does command-A become in sw/queue now?
quoted hunk ↗ jump to hunk
quoted
Any IO request which is going to be added to hctx->dispatch, the queue
will be
quoted
re-run via blk-mq core.

Any IO request being issued directly when running out of budget will be
insert
quoted
to hctx->dispatch or sw/scheduler queue, will be run in the submission
path.

I have *not* included below changes we discussed in my testing - If I
include below patch, it is correct that queue will be run in submission
path (at least the path which is impacted in my testing). You have already
mentioned that most of the submission path has fix now in latest kernel
w.r.t running h/w queue from submission path.  Below path is missing for
running h/w queue from submission path.
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c index
54f9015..bcfd33a 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -173,8 +173,10 @@ static int blk_mq_do_dispatch_ctx(struct
blk_mq_hw_ctx *hctx)
                if (!sbitmap_any_bit_set(&hctx->ctx_map))
                        break;

-               if (!blk_mq_get_dispatch_budget(hctx))
+               if (!blk_mq_get_dispatch_budget(hctx)) {
+                       blk_mq_delay_run_hw_queue(hctx,
+ BLK_MQ_BUDGET_DELAY);
                        break;
+               }

                rq = blk_mq_dequeue_from_ctx(hctx, ctx);
                if (!rq) {

Are you saying above fix should be included along with your latest patch ?
No.


Thanks,
Ming
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help