Messages in this thread | | | Subject | Re: [PATCH] block: fix io hung by block throttle | From | Junxiao Bi <> | Date | Wed, 21 Apr 2021 14:28:53 -0700 |
| |
Ping? This can cause io hung.
Thanks,
Junxiao.
On 4/14/21 9:11 PM, Hillf Danton wrote: > On Wed, 14 Apr 2021 14:18:30 Junxiao Bi wrote: >> There is a race bug which can cause io hung when multiple processes >> run parallel in rq_qos_wait(). >> Let assume there were 4 processes P1/P2/P3/P4, P1/P2 were at the entry >> of rq_qos_wait, and P3/P4 were waiting for io done, 2 io were inflight, >> the inflight io limit was 2. See race below. >> >> void rq_qos_wait() >> { >> ... >> bool has_sleeper; >> >> >>>> P3/P4 were in sleeper list, has_sleeper was true for both P1 and P2. >> has_sleeper = wq_has_sleeper(&rqw->wait); >> if (!has_sleeper && acquire_inflight_cb(rqw, private_data)) >> return; >> >> >>>> 2 inflight io done, P3/P4 were waken up to issue 2 new io. >> >>>> 2 new io done, no inflight io. >> >> >>>> P1/P2 were added to the sleeper list, 2 entry in the list >> prepare_to_wait_exclusive(&rqw->wait, &data.wq, TASK_UNINTERRUPTIBLE); >> >> >>>> P1/P2 were in the sleeper list, has_sleeper was true for P1/P2. >> has_sleeper = !wq_has_single_sleeper(&rqw->wait); >> do { >> /* The memory barrier in set_task_state saves us here. */ >> if (data.got_token) >> break; >> if (!has_sleeper && acquire_inflight_cb(rqw, private_data)) { >> finish_wait(&rqw->wait, &data.wq); >> >> /* >> * We raced with wbt_wake_function() getting a token, >> * which means we now have two. Put our local token >> * and wake anyone else potentially waiting for one. >> */ >> smp_rmb(); >> if (data.got_token) >> cleanup_cb(rqw, private_data); >> break; >> } >> >> >>>> P1/P2 hung here forever. New io requests will also hung here. >> io_schedule(); >> has_sleeper = true; >> set_current_state(TASK_UNINTERRUPTIBLE); >> } while (1); >> finish_wait(&rqw->wait, &data.wq); >> } >> >> Cc: stable@vger.kernel.org >> Signed-off-by: Junxiao Bi <junxiao.bi@oracle.com> >> --- >> block/blk-rq-qos.c | 9 +++------ >> 1 file changed, 3 insertions(+), 6 deletions(-) >> >> diff --git a/block/blk-rq-qos.c b/block/blk-rq-qos.c >> index 656460636ad3..04d888c99bc0 100644 >> --- a/block/blk-rq-qos.c >> +++ b/block/blk-rq-qos.c >> @@ -260,19 +260,17 @@ void rq_qos_wait(struct rq_wait *rqw, void *private_data, >> .cb = acquire_inflight_cb, >> .private_data = private_data, >> }; >> - bool has_sleeper; >> >> - has_sleeper = wq_has_sleeper(&rqw->wait); >> - if (!has_sleeper && acquire_inflight_cb(rqw, private_data)) >> + if (!wq_has_sleeper(&rqw->wait) >> + && acquire_inflight_cb(rqw, private_data)) >> return; >> >> prepare_to_wait_exclusive(&rqw->wait, &data.wq, TASK_UNINTERRUPTIBLE); >> - has_sleeper = !wq_has_single_sleeper(&rqw->wait); >> do { >> /* The memory barrier in set_task_state saves us here. */ >> if (data.got_token) >> break; >> - if (!has_sleeper && acquire_inflight_cb(rqw, private_data)) { >> + if (acquire_inflight_cb(rqw, private_data)) { >> finish_wait(&rqw->wait, &data.wq); >> >> /* >> @@ -286,7 +284,6 @@ void rq_qos_wait(struct rq_wait *rqw, void *private_data, >> break; >> } >> io_schedule(); >> - has_sleeper = true; >> set_current_state(TASK_UNINTERRUPTIBLE); >> } while (1); >> finish_wait(&rqw->wait, &data.wq); >> -- >> 2.24.3 (Apple Git-128) >> > No wakeup may cause the hang. > > --- a/block/blk-rq-qos.c > +++ b/block/blk-rq-qos.c > @@ -287,7 +287,8 @@ void rq_qos_wait(struct rq_wait *rqw, vo > } > io_schedule(); > has_sleeper = true; > - set_current_state(TASK_UNINTERRUPTIBLE); > + prepare_to_wait_exclusive(&rqw->wait, &data.wq, > + TASK_UNINTERRUPTIBLE); > } while (1); > finish_wait(&rqw->wait, &data.wq); > }
| |