On 1/26/19 11:37 PM, Florian Stecker wrote: > > > On 1/26/19 2:25 AM, jianchao.wang wrote: ... >>>> >>>> dmesg: https://urldefense.proofpoint.com/v2/url?u=https-3A__florianstecker.de_dmesg-2D2019-2D01-2D25.txt&d=DwIDaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=7WdAxUBeiTUTCy8v-7zXyr4qk7sx26ATvfo6QSTvZyQ&m=t_udGGsH8ivtjXHin4oQ4LxELuPoXbl_ARP7oSxiQ6k&s=DC2fhRw2g4haGhhHxOYyX4zD6FWQQ5YoERdfptVRHM4&e= >>>> Note that no I/O scheduler is in use here, I deactivated them in a udev rule because I still want to be able to use my laptop. When I test this stuff I just reactivate mq-deadline manually. >>>> >>>> Config is the default in Arch Linux: https://urldefense.proofpoint.com/v2/url?u=https-3A__git.archlinux.org_svntogit_packages.git_tree_trunk_config-3Fh-3Dpackages_linux&d=DwIDaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=7WdAxUBeiTUTCy8v-7zXyr4qk7sx26ATvfo6QSTvZyQ&m=t_udGGsH8ivtjXHin4oQ4LxELuPoXbl_ARP7oSxiQ6k&s=GjV_fyXaKZ8za8zdB_gqmKW7yBlbla2OHRaAT0iGZkc&e= >>>> >>>> The problem also appears with BFQ. >>>> >>>> And rq->end_io is set to mq_flush_data_end_io when this happens. The only point I see where this function could invoke blk_mq_end_request is via blk_flush_complete_seq, but it does so only if seq == REQ_FSEQ_DONE. However, seq is REQ_FSEQ_POSTFLUSH for me (i.e. it just transitioned from REQ_FSEQ_DATA to REQ_FSEQ_POSTFLUSH). >>> >>> >>> Could it be this scenario ? >>> >>> data + post flush >>> >>> blk_flush_complete_seq a flush >>> case REQ_FSEQ_DATA >>> blk_flush_queue_rq >>> finally issued to driver blk_mq_dispatch_rq_list >>> try to issue a flush req >>> failed due to NON-NCQ command >>> due to RESTART is set, do nothing >>> req complete >>> req->end_io（） // doesn't check RESTART >>> mq_flush_data_end_io >>> blk_flush_complete_seq >>> case REQ_FSEQ_POSTFLUSH >>> blk_kick_flush >>> do nothing because previous flush >>> has not been completed, so >>> flush_pending_idx != flush_running_idx >>> This should be corrected as following, data + post flush blk_flush_complete_seq a flush case REQ_FSEQ_DATA blk_flush_queue_rq finally issued to driver blk_mq_dispatch_rq_list try to issue a flush req failed due to NON-NCQ command return BLK_STS_DEV_RESOURCE req complete req->end_io（） // doesn't check RESTART mq_flush_data_end_io case REQ_FSEQ_POSTFLUSH blk_kick_flush do nothing because previous flush has not been completed, so flush_pending_idx != flush_running_idx blk_mq_mq_run_hw_queue insert rq to hctx->dispatch_list due to RESTART is set, do nothing ... > I'm not sure if I understand every detail, but your scenario sounds roughly the same as what I imagine. The patch solves the problem for me (except for the fact that hctx is not defined in that function). > > Actually, it doesn't seem to me as if any of the other users of blk_mq_free_request require or even expect that it calls blk_mq_sched_restart (I could be wrong though). If that is true, how about we just call it from __blk_mq_end_request, like this (also tested successfully on my laptop)? > > diff --git a/block/blk-mq.c b/block/blk-mq.c > index 6a7566244de3..9f3d3456c4af 100644 > --- a/block/blk-mq.c > +++ b/block/blk-mq.c > @@ -481,7 +481,6 @@ static void __blk_mq_free_request(struct request *rq) > blk_mq_put_tag(hctx, hctx->tags, ctx, rq->tag); > if (sched_tag != -1) > blk_mq_put_tag(hctx, hctx->sched_tags, ctx, sched_tag); > - blk_mq_sched_restart(hctx); > blk_queue_exit(q); > } > > @@ -522,6 +521,7 @@ EXPORT_SYMBOL_GPL(blk_mq_free_request); > inline void __blk_mq_end_request(struct request *rq, blk_status_t error) > { > u64 now = ktime_get_ns(); > + struct blk_mq_hw_ctx *hctx = blk_mq_map_queue(rq->q, rq->mq_ctx->cpu); > > if (rq->rq_flags & RQF_STATS) { > blk_mq_poll_stats_start(rq->q); > @@ -541,6 +541,8 @@ inline void __blk_mq_end_request(struct request *rq, blk_status_t error) > blk_mq_free_request(rq->next_rq); > blk_mq_free_request(rq); > } > + > + blk_mq_sched_restart(hctx); > } > EXPORT_SYMBOL(__blk_mq_end_request); We should invoke blk_mq_sched_restart before blk_queue_exit is invoked. How about this one ? diff --git a/block/blk-flush.c b/block/blk-flush.c index a3fc7191..6e0f2d9 100644 --- a/block/blk-flush.c +++ b/block/blk-flush.c @@ -335,7 +335,7 @@ static void mq_flush_data_end_io(struct request *rq, blk_status_t error) blk_flush_complete_seq(rq, fq, REQ_FSEQ_DATA, error); spin_unlock_irqrestore(&fq->mq_flush_lock, flags); - blk_mq_run_hw_queue(hctx, true); + blk_mq_sched_restart(hctx); } Thanks Jianchao
