[PATCH 0/4] random improvments and cleanups for elevator.c

[PATCH 0/4] random improvments and cleanups for elevator.c

[Jinlong Chen]

This series updates some function documents (patch 1 and 3), removes an
outdated comment (patch 4), and adds a warning message (patch 2) in
elevator.c.

Jinlong Chen (4):
  elevator: update the document of elevator_switch
  elevator: printk a warning if switching to a new io scheduler fails
  elevator: update the document of elevator_match
  elevator: remove an outdated comment in elevator_change

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

-- 
2.34.1

[PATCH 1/4] elevator: update the document of elevator_switch

[Jinlong Chen]

We no longer support falling back to the old io scheduler if switching to
the new one fails. Update the document to indicate that.

Fixes: a1ce35fa4985 ("block: remove dead elevator code")
Signed-off-by: Jinlong Chen <nickyc975@zju.edu.cn>
---
 block/elevator.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/elevator.c b/block/elevator.c
index a5bdc3b1e7e5..01aa9f38f22e 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -650,10 +650,10 @@ void elevator_init_mq(struct request_queue *q)
 }
 
 /*
- * switch to new_e io scheduler. be careful not to introduce deadlocks -
- * we don't free the old io scheduler, before we have allocated what we
- * need for the new one. this way we have a chance of going back to the old
- * one, if the new one fails init for some reason.
+ * Switch to new_e io scheduler.
+ *
+ * If switching fails, we are most likely running out of memory and not able
+ * to restore the old io scheduler, so leaving the io scheduler being none.
  */
 int elevator_switch(struct request_queue *q, struct elevator_type *new_e)
 {
-- 
2.34.1

Re: [PATCH 1/4] elevator: update the document of elevator_switch

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 2/4] elevator: printk a warning if switching to a new io scheduler fails

[Jinlong Chen]

printk a warning to indicate that the io scheduler has been set to none
if switching to a new io scheduler fails.

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

diff --git a/block/elevator.c b/block/elevator.c
index 01aa9f38f22e..1fa45717b1d6 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -683,6 +683,12 @@ int elevator_switch(struct request_queue *q, struct elevator_type *new_e)
 out_unfreeze:
 	blk_mq_unquiesce_queue(q);
 	blk_mq_unfreeze_queue(q);
+
+	if (ret) {
+		pr_warn("elv: switch to \"%s\" failed, falling back to \"none\"\n",
+			new_e->elevator_name);
+	}
+
 	return ret;
 }
 
-- 
2.34.1

Re: [PATCH 2/4] elevator: printk a warning if switching to a new io scheduler fails

[Jinlong Chen]

> 
> printk a warning to indicate that the io scheduler has been set to none
> if switching to a new io scheduler fails.
> 
> Signed-off-by: Jinlong Chen <nickyc975@zju.edu.cn>
> ---
>  block/elevator.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/block/elevator.c b/block/elevator.c
> index 01aa9f38f22e..1fa45717b1d6 100644
> --- a/block/elevator.c
> +++ b/block/elevator.c
> @@ -683,6 +683,12 @@ int elevator_switch(struct request_queue *q, struct elevator_type *new_e)
>  out_unfreeze:
>  	blk_mq_unquiesce_queue(q);
>  	blk_mq_unfreeze_queue(q);
> +
> +	if (ret) {
> +		pr_warn("elv: switch to \"%s\" failed, falling back to \"none\"\n",
> +			new_e->elevator_name);
> +	}
> +
>  	return ret;
>  }
>  

Hi, Jens!

This patch is suggested by Christoph, but I forgot to add the Suggested-by
tag. Would you please add "Suggested-by: Christoph Hellwig <hch@lst.de>"
if you apply the patch?

Thanks!
Jinlong Chen

Re: [PATCH 2/4] elevator: printk a warning if switching to a new io scheduler fails

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 3/4] elevator: update the document of elevator_match

[Jinlong Chen]

elevator_match does not care about elevator_features any more. Remove
related descriptions from its document.

Fixes: ffb86425ee2c ("block: don't check for required features in elevator_match")
Signed-off-by: Jinlong Chen <nickyc975@zju.edu.cn>
---
 block/elevator.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/block/elevator.c b/block/elevator.c
index 1fa45717b1d6..5287b39cd7a9 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -91,12 +91,11 @@ static inline bool elv_support_features(struct request_queue *q,
 }
 
 /**
- * elevator_match - Test an elevator name and features
+ * elevator_match - Check whether @e's name or alias matches @name
  * @e: Scheduler to test
  * @name: Elevator name to test
  *
- * Return true if the elevator @e name matches @name and if @e provides all
- * the features specified by @required_features.
+ * Return true if the elevator @e's name or alias matches @name.
  */
 static bool elevator_match(const struct elevator_type *e, const char *name)
 {
-- 
2.34.1

Re: [PATCH 3/4] elevator: update the document of elevator_match

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 4/4] elevator: remove an outdated comment in elevator_change

[Jinlong Chen]

mq is no longer a special case.

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

diff --git a/block/elevator.c b/block/elevator.c
index 5287b39cd7a9..599413620558 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -721,9 +721,6 @@ static int elevator_change(struct request_queue *q, const char *elevator_name)
 	if (!blk_queue_registered(q))
 		return -ENOENT;
 
-	/*
-	 * Special case for mq, turn off scheduling
-	 */
 	if (!strncmp(elevator_name, "none", 4)) {
 		if (q->elevator)
 			elevator_disable(q);
-- 
2.34.1

Re: [PATCH 4/4] elevator: remove an outdated comment in elevator_change

[Christoph Hellwig]

On Tue, Nov 22, 2022 at 10:21:26PM +0800, Jinlong Chen wrote:
> mq is no longer a special case.

But switching to none still is a special case.  Then again this should
be very obvious from the code, so I'm fine the comment removal:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 0/4] random improvments and cleanups for elevator.c

[Jens Axboe]

On Tue, 22 Nov 2022 22:21:22 +0800, Jinlong Chen wrote:
> This series updates some function documents (patch 1 and 3), removes an
> outdated comment (patch 4), and adds a warning message (patch 2) in
> elevator.c.
> 
> Jinlong Chen (4):
>   elevator: update the document of elevator_switch
>   elevator: printk a warning if switching to a new io scheduler fails
>   elevator: update the document of elevator_match
>   elevator: remove an outdated comment in elevator_change
> 
> [...]

Applied, thanks!

[1/4] elevator: update the document of elevator_switch
      commit: ac1171bd2c7a3a32dfbdd3c347919fee32b745a1
[2/4] elevator: printk a warning if switching to a new io scheduler fails
      commit: e0cca8bc9cd8d6176921cb3f5f466d3ccfbc6b99
[3/4] elevator: update the document of elevator_match
      commit: f69b5e8f356e4e57e94b806ca1dcb9771933bb9c
[4/4] elevator: remove an outdated comment in elevator_change
      commit: 4284354758d67cf77ab2a4494e28d4c05fb83074

Best regards,
-- 
Jens Axboe