> Il giorno 5 apr 2020, alle ore 16:00, Doug Anderson <dianders@xxxxxxxxxxxx> ha scritto: > > Hi, > > On Sun, Apr 5, 2020 at 2:15 AM Ming Lei <ming.lei@xxxxxxxxxx> wrote: >> >> OK, looks it isn't specific on BFQ any more. >> >> Follows another candidate approach for this issue, given it is so hard >> to trigger, we can make it more reliable by rerun queue when has_work() >> returns true after ops->dispath_request() returns NULL. >> >> diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c >> index 74cedea56034..4408e5d4fcd8 100644 >> --- a/block/blk-mq-sched.c >> +++ b/block/blk-mq-sched.c >> @@ -80,6 +80,7 @@ void blk_mq_sched_restart(struct blk_mq_hw_ctx *hctx) >> blk_mq_run_hw_queue(hctx, true); >> } >> >> +#define BLK_MQ_BUDGET_DELAY 3 /* ms units */ >> /* >> * Only SCSI implements .get_budget and .put_budget, and SCSI restarts >> * its queue by itself in its completion handler, so we don't need to >> @@ -103,6 +104,9 @@ static void blk_mq_do_dispatch_sched(struct blk_mq_hw_ctx *hctx) >> rq = e->type->ops.dispatch_request(hctx); >> if (!rq) { >> blk_mq_put_dispatch_budget(hctx); >> + >> + if (e->type->ops.has_work && e->type->ops.has_work(hctx)) >> + blk_mq_delay_run_hw_queue(hctx, BLK_MQ_BUDGET_DELAY); > > I agree that your patch should solve the race. With the current BFQ's > has_work() it's a bit of a disaster though. It will essentially put > blk-mq into a busy-wait loop (with a 3 ms delay between each poll) > while BFQ's has_work() says "true" but BFQ doesn't dispatch anything. > > ...so I guess the question that still needs to be answered: does > has_work() need to be exact? If so then we need the patch you propose > plus one to BFQ. If not, we should continue along the lines of my > patch. > Some more comments. BFQ's I/O plugging lasts 9 ms by default. So, with this last Ming's patch, BFQ may happen to be polled every 3ms, for at most three times. On the opposite end, making bfq_has_work plugging aware costs more complexity, and possibly one more lock. While avoiding the above occasional polling, this may imply a lot of overhead or CPU stalls on every dispatch. Paolo > -Doug