> On Oct 22, 2024, at 15:53, Chengming Zhou <chengming.zhou@xxxxxxxxx> wrote: > > On 2024/10/21 16:52, Muchun Song wrote: >> The memory barriers in list_del_init_careful() and list_empty_careful() >> in pairs already handle the proper ordering between data.got_token >> and data.wq.entry. So remove the redundant explicit barriers. And also >> change a "break" statement to "return" to avoid redundant calling of >> finish_wait(). >> Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx> > > Good catch! Just a small nit below, feel free to add: > > Reviewed-by: Chengming Zhou <chengming.zhou@xxxxxxxxx> > >> --- >> block/blk-rq-qos.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> diff --git a/block/blk-rq-qos.c b/block/blk-rq-qos.c >> index dc510f493ba57..9b0aa7dd6779f 100644 >> --- a/block/blk-rq-qos.c >> +++ b/block/blk-rq-qos.c >> @@ -218,7 +218,6 @@ static int rq_qos_wake_function(struct wait_queue_entry *curr, >> return -1; >> data->got_token = true; >> - smp_wmb(); >> wake_up_process(data->task); >> list_del_init_careful(&curr->entry); >> return 1; >> @@ -274,10 +273,9 @@ void rq_qos_wait(struct rq_wait *rqw, void *private_data, >> * 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; >> + return; >> } > > Would it be better to move this acquire_inflight_cb() above out of > the do-while(1) since we rely on the waker to get inflight counter > for us? I also noticed about this and I am working on this. Will send a separate patch for this refactoring later. Thanks. > > Thanks. > >> io_schedule(); >> has_sleeper = true;
