Messages in this thread | | | Date | Tue, 20 Dec 2022 10:38:08 +0800 | Subject | Re: [External] Re: [PATCH] blk-throtl: Introduce sync and async queues for blk-throtl | From | hanjinke <> |
| |
在 2022/12/20 上午5:22, Tejun Heo 写道: > Hello, > > This looks generally fine to me. Some nits below. > >> +static inline struct bio *throtl_qnode_bio_peek(struct throtl_qnode *qn) >> +{ >> + struct bio *bio1, *bio2; >> + >> + /* qn for read ios */ >> + if (qn->dispatch_sync_cnt == UINT_MAX) >> + return bio_list_peek(&qn->bios[SYNC]); >> + >> + /* qn for write ios */ >> + bio1 = bio_list_peek(&qn->bios[SYNC]); >> + bio2 = bio_list_peek(&qn->bios[ASYNC]); >> + >> + if (bio1 && bio2) { >> + if (qn->dispatch_sync_cnt == THROTL_SYNC_FACTOR) >> + return bio2; >> + return bio1; >> + } >> + >> + return bio1 ?: bio2; >> +} > > Wouldn't it be simpler to write: > > if (qn->dispatch_sync_count < THROTL_SYNC_FACTOR) > return bio1 ?: bio2; > else > return bio2 ?: bio1; > >> +/** >> + * throtl_qnode_bio_pop: pop a bio from a qnode >> + * @qn: the qnode to pop a bio from >> + * >> + * For read io qn, just pop bio from sync queu and return. >> + * For write io qn, the target queue to pop was determined by the dispatch_sync_cnt. >> + * Try to pop bio from target queue, fetch the bio and return when it is not empty. >> + * If the target queue empty, pop bio from other queue instead. >> + */ >> +static inline struct bio *throtl_qnode_bio_pop(struct throtl_qnode *qn) >> +{ >> + struct bio *bio; >> + >> + /* qn for read ios */ >> + if (qn->dispatch_sync_cnt == UINT_MAX) >> + return bio_list_pop(&qn->bios[SYNC]); >> + >> + /* try to dispatch sync io */ >> + if (qn->dispatch_sync_cnt < THROTL_SYNC_FACTOR) { >> + bio = bio_list_pop(&qn->bios[SYNC]); >> + if (bio) { >> + qn->dispatch_sync_cnt++; >> + return bio; >> + } >> + bio = bio_list_pop(&qn->bios[ASYNC]); >> + qn->dispatch_sync_cnt = 0; >> + return bio; >> + } >> + >> + /* try to dispatch async io */ >> + bio = bio_list_pop(&qn->bios[ASYNC]); >> + if (bio) { >> + qn->dispatch_sync_cnt = 0; >> + return bio; >> + } >> + bio = bio_list_pop(&qn->bios[SYNC]); >> + >> + return bio; >> +} > > This also seems like it can be simplified a bit. > > Thanks. >
Indeed, I will make it more clear and send the v2.
Thanks.
| |