| Message ID | 20230704040626.24899-1-lipeifeng@oppo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: mq-deadline: rename sort_list to sort_rb | expand |
On 7/3/23 21:06, lipeifeng@oppo.com wrote: > Mq-deadline would store request in list:fifo_list and > rb_tree:sort_list, and sort_list should be renamed to > sort_rb which is beneficial for understanding. Huh? I think this patch makes the code less readable instead of more readable ... Bart.
>> Mq-deadline would store request in list:fifo_list and >> rb_tree:sort_list, and sort_list should be renamed to sort_rb which is >> beneficial for understanding. >Huh? I think this patch makes the code less readable instead of more readable ... Huh? Maybe we had different opinions about it, I thinks the essence of this word is 'sort' So that reader can get the meaning of it easily. And in my mind, *_rb is more reasonable for rb_root ratherthan *_list for reader. -----邮件原件----- 发件人: Bart Van Assche <bvanassche@acm.org> 发送时间: 2023年7月4日 22:13 收件人: 李培锋(wink) <lipeifeng@oppo.com>; axboe@kernel.dk 抄送: linux-block@vger.kernel.org; linux-kernel@vger.kernel.org; 张诗明(Simon Zhang) <zhangshiming@oppo.com>; 郭健 <guojian@oppo.com> 主题: Re: [PATCH] block: mq-deadline: rename sort_list to sort_rb On 7/3/23 21:06, lipeifeng@oppo.com wrote: > Mq-deadline would store request in list:fifo_list and > rb_tree:sort_list, and sort_list should be renamed to sort_rb which is > beneficial for understanding. Huh? I think this patch makes the code less readable instead of more readable ... Bart.
>>> Mq-deadline would store request in list:fifo_list and >>> rb_tree:sort_list, and sort_list should be renamed to sort_rb which >>> is beneficial for understanding. >>Huh? I think this patch makes the code less readable instead of more readable ... >Huh? Maybe we had different opinions about it, I thinks the essence of this word is 'sort' >So that reader can get the meaning of it easily. And in my mind, *_rb is more reasonable for rb_root ratherthan *_list for reader. Hi Sir： Should it be merged for the above reason? Hope for your reply, thanks. -----邮件原件----- 发件人: 李培锋(wink) 发送时间: 2023年7月5日 8:31 收件人: Bart Van Assche <bvanassche@acm.org>; axboe@kernel.dk 抄送: linux-block@vger.kernel.org; linux-kernel@vger.kernel.org; 张诗明(Simon Zhang) <zhangshiming@oppo.com>; 郭健 <guojian@oppo.com> 主题: 回复: [PATCH] block: mq-deadline: rename sort_list to sort_rb >> Mq-deadline would store request in list:fifo_list and >> rb_tree:sort_list, and sort_list should be renamed to sort_rb which >> is beneficial for understanding. >Huh? I think this patch makes the code less readable instead of more readable ... Huh? Maybe we had different opinions about it, I thinks the essence of this word is 'sort' So that reader can get the meaning of it easily. And in my mind, *_rb is more reasonable for rb_root ratherthan *_list for reader. -----邮件原件----- 发件人: Bart Van Assche <bvanassche@acm.org> 发送时间: 2023年7月4日 22:13 收件人: 李培锋(wink) <lipeifeng@oppo.com>; axboe@kernel.dk 抄送: linux-block@vger.kernel.org; linux-kernel@vger.kernel.org; 张诗明(Simon Zhang) <zhangshiming@oppo.com>; 郭健 <guojian@oppo.com> 主题: Re: [PATCH] block: mq-deadline: rename sort_list to sort_rb On 7/3/23 21:06, lipeifeng@oppo.com wrote: > Mq-deadline would store request in list:fifo_list and > rb_tree:sort_list, and sort_list should be renamed to sort_rb which is > beneficial for understanding. Huh? I think this patch makes the code less readable instead of more readable ... Bart.
On 7/6/23 3:27?AM, ???(wink) wrote: >>>> Mq-deadline would store request in list:fifo_list and >>>> rb_tree:sort_list, and sort_list should be renamed to sort_rb which >>>> is beneficial for understanding. > >>> Huh? I think this patch makes the code less readable instead of more readable ... > >> Huh? Maybe we had different opinions about it, I thinks the essence of this word is 'sort' >> So that reader can get the meaning of it easily. And in my mind, *_rb is more reasonable for rb_root ratherthan *_list for reader. > > Hi Sir? > Should it be merged for the above reason? Hope for your reply, thanks. No, the patch makes no sense. I agree with Bart that it doesn't make it any more readable, in fact it's worse. We have a sort and fifo list, the backing data structure isn't that exciting by itself.
>>>>> Mq-deadline would store request in list:fifo_list and >>>>> rb_tree:sort_list, and sort_list should be renamed to sort_rb which >>>>> is beneficial for understanding. >> >>>> Huh? I think this patch makes the code less readable instead of more readable ... >> >>> Huh? Maybe we had different opinions about it, I thinks the essence of this word is 'sort' >>> So that reader can get the meaning of it easily. And in my mind, *_rb is more reasonable for rb_root ratherthan *_list for reader. >> >> Hi Sir? >> Should it be merged for the above reason? Hope for your reply, thanks. >No, the patch makes no sense. I agree with Bart that it doesn't make it any more readable, in fact it's worse. We have a sort and fifo list, the backing data structure isn't that exciting by itself. That is okay, thank you for your reply and respect both of you. -----邮件原件----- 发件人: Jens Axboe <axboe@kernel.dk> 发送时间: 2023年7月6日 22:05 收件人: 李培锋(wink) <lipeifeng@oppo.com> 抄送: linux-block@vger.kernel.org; linux-kernel@vger.kernel.org; 张诗明(Simon Zhang) <zhangshiming@oppo.com>; 郭健 <guojian@oppo.com>; Bart Van Assche <bvanassche@acm.org> 主题: Re: 回复: [PATCH] block: mq-deadline: rename sort_list to sort_rb On 7/6/23 3:27?AM, ???(wink) wrote: >>>> Mq-deadline would store request in list:fifo_list and >>>> rb_tree:sort_list, and sort_list should be renamed to sort_rb which >>>> is beneficial for understanding. > >>> Huh? I think this patch makes the code less readable instead of more readable ... > >> Huh? Maybe we had different opinions about it, I thinks the essence of this word is 'sort' >> So that reader can get the meaning of it easily. And in my mind, *_rb is more reasonable for rb_root ratherthan *_list for reader. > > Hi Sir? > Should it be merged for the above reason? Hope for your reply, thanks. No, the patch makes no sense. I agree with Bart that it doesn't make it any more readable, in fact it's worse. We have a sort and fifo list, the backing data structure isn't that exciting by itself. -- Jens Axboe
diff --git a/block/mq-deadline.c b/block/mq-deadline.c index 6aa5daf7ae32..b3757b7a6780 100644 --- a/block/mq-deadline.c +++ b/block/mq-deadline.c @@ -68,11 +68,11 @@ struct io_stats_per_prio { /* * Deadline scheduler data per I/O priority (enum dd_prio). Requests are - * present on both sort_list[] and fifo_list[]. + * present on both sort_rb[] and fifo_list[]. */ struct dd_per_prio { struct list_head dispatch; - struct rb_root sort_list[DD_DIR_COUNT]; + struct rb_root sort_rb[DD_DIR_COUNT]; struct list_head fifo_list[DD_DIR_COUNT]; /* Position of the most recently dispatched request. */ sector_t latest_pos[DD_DIR_COUNT]; @@ -116,7 +116,7 @@ static const enum dd_prio ioprio_class_to_prio[] = { static inline struct rb_root * deadline_rb_root(struct dd_per_prio *per_prio, struct request *rq) { - return &per_prio->sort_list[rq_data_dir(rq)]; + return &per_prio->sort_rb[rq_data_dir(rq)]; } /* @@ -163,7 +163,7 @@ deadline_latter_request(struct request *rq) static inline struct request *deadline_from_pos(struct dd_per_prio *per_prio, enum dd_data_dir data_dir, sector_t pos) { - struct rb_node *node = per_prio->sort_list[data_dir].rb_node; + struct rb_node *node = per_prio->sort_rb[data_dir].rb_node; struct request *rq, *res = NULL; if (!node) @@ -477,7 +477,7 @@ static struct request *__dd_dispatch_request(struct deadline_data *dd, */ if (!list_empty(&per_prio->fifo_list[DD_READ])) { - BUG_ON(RB_EMPTY_ROOT(&per_prio->sort_list[DD_READ])); + BUG_ON(RB_EMPTY_ROOT(&per_prio->sort_rb[DD_READ])); if (deadline_fifo_request(dd, per_prio, DD_WRITE) && (dd->starved++ >= dd->writes_starved)) @@ -494,7 +494,7 @@ static struct request *__dd_dispatch_request(struct deadline_data *dd, if (!list_empty(&per_prio->fifo_list[DD_WRITE])) { dispatch_writes: - BUG_ON(RB_EMPTY_ROOT(&per_prio->sort_list[DD_WRITE])); + BUG_ON(RB_EMPTY_ROOT(&per_prio->sort_rb[DD_WRITE])); dd->starved = 0; @@ -711,8 +711,8 @@ static int dd_init_sched(struct request_queue *q, struct elevator_type *e) INIT_LIST_HEAD(&per_prio->dispatch); INIT_LIST_HEAD(&per_prio->fifo_list[DD_READ]); INIT_LIST_HEAD(&per_prio->fifo_list[DD_WRITE]); - per_prio->sort_list[DD_READ] = RB_ROOT; - per_prio->sort_list[DD_WRITE] = RB_ROOT; + per_prio->sort_rb[DD_READ] = RB_ROOT; + per_prio->sort_rb[DD_WRITE] = RB_ROOT; } dd->fifo_expire[DD_READ] = read_expire; dd->fifo_expire[DD_WRITE] = write_expire; @@ -752,7 +752,7 @@ static int dd_request_merge(struct request_queue *q, struct request **rq, if (!dd->front_merges) return ELEVATOR_NO_MERGE; - __rq = elv_rb_find(&per_prio->sort_list[bio_data_dir(bio)], sector); + __rq = elv_rb_find(&per_prio->sort_rb[bio_data_dir(bio)], sector); if (__rq) { BUG_ON(sector != blk_rq_pos(__rq));