block: mq-deadline: rename sort_list to sort_rb

Message ID 20230704040626.24899-1-lipeifeng@oppo.com
State New
Headers
Series block: mq-deadline: rename sort_list to sort_rb |

Commit Message

李培锋 July 4, 2023, 4:06 a.m. UTC
  From: lipeifeng <lipeifeng@oppo.com>

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.

Signed-off-by: lipeifeng <lipeifeng@oppo.com>
---
 block/mq-deadline.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)
  

Comments

Bart Van Assche July 4, 2023, 2:13 p.m. UTC | #1
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.
  
李培锋 July 5, 2023, 12:30 a.m. UTC | #2
>> 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.
  
李培锋 July 6, 2023, 9:27 a.m. UTC | #3
>>> 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.
  
Jens Axboe July 6, 2023, 2:05 p.m. UTC | #4
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.
  
李培锋 July 7, 2023, 12:26 a.m. UTC | #5
>>>>> 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
  

Patch

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));