[PATCH v2 0/5] random improvements and cleanups for elevator.c

[PATCH v2 0/5] random improvements and cleanups for elevator.c

[Jinlong Chen]

The series slightly improves the readability of elevator.c.

Changes in v2:
  - add patch 3 to further improve the readability (suggested by Christoph)
  - add Reviewed-by tags from Christoph

Jinlong Chen (5):
  elevator: print none at first in elv_iosched_show even if the queue
    has a scheduler
  elevator: replace continue with else-if in elv_iosched_show
  elevator: print e->elevator_name instead of cur->elevator_name in
    elv_iosched_show
  elevator: repalce "len+name" with "name+len" in elv_iosched_show
  elevator: use bool instead of int as the return type of
    elv_iosched_allow_bio_merge

 block/elevator.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

-- 
2.34.1

[PATCH v2 1/5] elevator: print none at first in elv_iosched_show even if the queue has a scheduler

[Jinlong Chen]

This makes the printing order of the io schedulers consistent, and removes
a redundant q->elevator check.

Signed-off-by: Jinlong Chen <nickyc975@zju.edu.cn>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 block/elevator.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/block/elevator.c b/block/elevator.c
index 599413620558..308bee253564 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -767,10 +767,12 @@ ssize_t elv_iosched_show(struct request_queue *q, char *name)
 	if (!elv_support_iosched(q))
 		return sprintf(name, "none\n");
 
-	if (!q->elevator)
+	if (!q->elevator) {
 		len += sprintf(name+len, "[none] ");
-	else
+	} else {
+		len += sprintf(name+len, "none ");
 		cur = eq->type;
+	}
 
 	spin_lock(&elv_list_lock);
 	list_for_each_entry(e, &elv_list, list) {
@@ -783,9 +785,6 @@ ssize_t elv_iosched_show(struct request_queue *q, char *name)
 	}
 	spin_unlock(&elv_list_lock);
 
-	if (q->elevator)
-		len += sprintf(name+len, "none");
-
 	len += sprintf(len+name, "\n");
 	return len;
 }
-- 
2.34.1

[PATCH v2 2/5] elevator: replace continue with else-if in elv_iosched_show

[Jinlong Chen]

else-if is more readable than continue here.

Signed-off-by: Jinlong Chen <nickyc975@zju.edu.cn>
---
 block/elevator.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/block/elevator.c b/block/elevator.c
index 308bee253564..ffa750976d25 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -776,11 +776,9 @@ ssize_t elv_iosched_show(struct request_queue *q, char *name)
 
 	spin_lock(&elv_list_lock);
 	list_for_each_entry(e, &elv_list, list) {
-		if (e == cur) {
+		if (e == cur)
 			len += sprintf(name+len, "[%s] ", cur->elevator_name);
-			continue;
-		}
-		if (elv_support_features(q, e))
+		else if (elv_support_features(q, e))
 			len += sprintf(name+len, "%s ", e->elevator_name);
 	}
 	spin_unlock(&elv_list_lock);
-- 
2.34.1

[PATCH v2 3/5] elevator: print e->elevator_name instead of cur->elevator_name in elv_iosched_show

[Jinlong Chen]

Printing e->elevator_name in all cases improves the readability.

Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jinlong Chen <nickyc975@zju.edu.cn>
---
 block/elevator.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/elevator.c b/block/elevator.c
index ffa750976d25..b2f2252f29e7 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -777,7 +777,7 @@ ssize_t elv_iosched_show(struct request_queue *q, char *name)
 	spin_lock(&elv_list_lock);
 	list_for_each_entry(e, &elv_list, list) {
 		if (e == cur)
-			len += sprintf(name+len, "[%s] ", cur->elevator_name);
+			len += sprintf(name+len, "[%s] ", e->elevator_name);
 		else if (elv_support_features(q, e))
 			len += sprintf(name+len, "%s ", e->elevator_name);
 	}
-- 
2.34.1

[PATCH v2 4/5] elevator: repalce "len+name" with "name+len" in elv_iosched_show

[Jinlong Chen]

The "pointer + offset" pattern is more resonable.

Signed-off-by: Jinlong Chen <nickyc975@zju.edu.cn>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 block/elevator.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/elevator.c b/block/elevator.c
index b2f2252f29e7..8a5c171306f1 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -783,7 +783,7 @@ ssize_t elv_iosched_show(struct request_queue *q, char *name)
 	}
 	spin_unlock(&elv_list_lock);
 
-	len += sprintf(len+name, "\n");
+	len += sprintf(name+len, "\n");
 	return len;
 }
 
-- 
2.34.1

[PATCH v2 5/5] elevator: use bool instead of int as the return type of elv_iosched_allow_bio_merge

[Jinlong Chen]

We have bool type now, update the old signature.

Signed-off-by: Jinlong Chen <nickyc975@zju.edu.cn>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 block/elevator.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/elevator.c b/block/elevator.c
index 8a5c171306f1..14e03632b5b5 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -57,7 +57,7 @@ static LIST_HEAD(elv_list);
  * Query io scheduler to see if the current process issuing bio may be
  * merged with rq.
  */
-static int elv_iosched_allow_bio_merge(struct request *rq, struct bio *bio)
+static bool elv_iosched_allow_bio_merge(struct request *rq, struct bio *bio)
 {
 	struct request_queue *q = rq->q;
 	struct elevator_queue *e = q->elevator;
@@ -65,7 +65,7 @@ static int elv_iosched_allow_bio_merge(struct request *rq, struct bio *bio)
 	if (e->type->ops.allow_merge)
 		return e->type->ops.allow_merge(q, rq, bio);
 
-	return 1;
+	return true;
 }
 
 /*
-- 
2.34.1

Re: [PATCH v2 0/5] random improvements and cleanups for elevator.c

[Jens Axboe]

On 11/29/22 8:46 AM, Jinlong Chen wrote:
> The series slightly improves the readability of elevator.c.
> 
> Changes in v2:
>   - add patch 3 to further improve the readability (suggested by Christoph)
>   - add Reviewed-by tags from Christoph
> 
> Jinlong Chen (5):
>   elevator: print none at first in elv_iosched_show even if the queue
>     has a scheduler
>   elevator: replace continue with else-if in elv_iosched_show
>   elevator: print e->elevator_name instead of cur->elevator_name in
>     elv_iosched_show
>   elevator: repalce "len+name" with "name+len" in elv_iosched_show
>   elevator: use bool instead of int as the return type of
>     elv_iosched_allow_bio_merge
> 
>  block/elevator.c | 23 ++++++++++-------------
>  1 file changed, 10 insertions(+), 13 deletions(-)

Applied, with some commit message re-formatting. Please keep lines
<= 74 chars.

-- 
Jens Axboe

Re: [PATCH v2 0/5] random improvements and cleanups for elevator.c

[Jens Axboe]

On Tue, 29 Nov 2022 23:46:33 +0800, Jinlong Chen wrote:
> The series slightly improves the readability of elevator.c.
> 
> Changes in v2:
>   - add patch 3 to further improve the readability (suggested by Christoph)
>   - add Reviewed-by tags from Christoph
> 
> Jinlong Chen (5):
>   elevator: print none at first in elv_iosched_show even if the queue
>     has a scheduler
>   elevator: replace continue with else-if in elv_iosched_show
>   elevator: print e->elevator_name instead of cur->elevator_name in
>     elv_iosched_show
>   elevator: repalce "len+name" with "name+len" in elv_iosched_show
>   elevator: use bool instead of int as the return type of
>     elv_iosched_allow_bio_merge
> 
> [...]

Applied, thanks!

[1/5] elevator: print none at first in elv_iosched_show even if the queue has a scheduler
      commit: 7919d679ae09c0dc30dfecb7cbc02306cf95cdd7
[2/5] elevator: replace continue with else-if in elv_iosched_show
      commit: 5998249e3238428156b09911f1606b41113443c5
[3/5] elevator: print e->elevator_name instead of cur->elevator_name in elv_iosched_show
      commit: 7a3b3660fd30c028e7ae1cd82697933789962406
[4/5] elevator: repalce "len+name" with "name+len" in elv_iosched_show
      commit: c6451ede406b9f57fcd61d48433a6b8b2be862e3
[5/5] elevator: use bool instead of int as the return type of elv_iosched_allow_bio_merge
      commit: 8d283ee62b077968e218531b24260e1cc51bd484

Best regards,
-- 
Jens Axboe