diff mbox series

[V2,08/20] block: don't allow to switch elevator if updating nr_hw_queues is in-progress

Message ID 20250418163708.442085-9-ming.lei@redhat.com (mailing list archive)
State New
Headers show
Series block: unify elevator changing and fix lockdep warning | expand

Commit Message

Ming Lei April 18, 2025, 4:36 p.m. UTC
Elevator switch code is another `nr_hw_queue` reader in non-fast-IO code
path, so it can't be done if updating `nr_hw_queues` is in-progress.

Take same approach with not allowing add/del disk when updating
nr_hw_queues is in-progress, by holding srcu lock & check
set->updating_nr_hwq.

Reported-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Closes: https://lore.kernel.org/linux-block/mz4t4tlwiqjijw3zvqnjb7ovvvaegkqganegmmlc567tt5xj67@xal5ro544cnc/
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/elevator.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Nilay Shroff April 19, 2025, 11:22 a.m. UTC | #1
On 4/18/25 10:06 PM, Ming Lei wrote:
> Elevator switch code is another `nr_hw_queue` reader in non-fast-IO code
> path, so it can't be done if updating `nr_hw_queues` is in-progress.
> 
> Take same approach with not allowing add/del disk when updating
> nr_hw_queues is in-progress, by holding srcu lock & check
> set->updating_nr_hwq.
> 
> Reported-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> Closes: https://lore.kernel.org/linux-block/mz4t4tlwiqjijw3zvqnjb7ovvvaegkqganegmmlc567tt5xj67@xal5ro544cnc/
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  block/elevator.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/block/elevator.c b/block/elevator.c
> index d25b9cc6c509..c23912652f96 100644
> --- a/block/elevator.c
> +++ b/block/elevator.c
> @@ -720,9 +720,10 @@ ssize_t elv_iosched_store(struct gendisk *disk, const char *buf,
>  {
>  	char elevator_name[ELV_NAME_MAX];
>  	char *name;
> -	int ret;
> +	int ret, idx;
>  	unsigned int memflags;
>  	struct request_queue *q = disk->queue;
> +	struct blk_mq_tag_set *set = q->tag_set;
>  
>  	/*
>  	 * If the attribute needs to load a module, do it before freezing the
> @@ -734,6 +735,12 @@ ssize_t elv_iosched_store(struct gendisk *disk, const char *buf,
>  
>  	elv_iosched_load_module(name);
>  
> +	idx = srcu_read_lock(&set->update_nr_hwq_srcu);
> +	if (set->updating_nr_hwq) {
> +		ret = -EBUSY;
> +		goto exit;
> +	}
> +
>  	memflags = blk_mq_freeze_queue(q);
>  	mutex_lock(&q->elevator_lock);
>  	ret = elevator_change(q, name);
> @@ -741,6 +748,8 @@ ssize_t elv_iosched_store(struct gendisk *disk, const char *buf,
>  		ret = count;
>  	mutex_unlock(&q->elevator_lock);
>  	blk_mq_unfreeze_queue(q, memflags);
> +exit:
> +	srcu_read_unlock(&set->update_nr_hwq_srcu, idx);
>  	return ret;
>  }
>  

I have same comment here as well as I mentioned in the previous patch.
(i.e. about using reader-writer lock instead of srcu).

Thanks,
--Nilay
diff mbox series

Patch

diff --git a/block/elevator.c b/block/elevator.c
index d25b9cc6c509..c23912652f96 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -720,9 +720,10 @@  ssize_t elv_iosched_store(struct gendisk *disk, const char *buf,
 {
 	char elevator_name[ELV_NAME_MAX];
 	char *name;
-	int ret;
+	int ret, idx;
 	unsigned int memflags;
 	struct request_queue *q = disk->queue;
+	struct blk_mq_tag_set *set = q->tag_set;
 
 	/*
 	 * If the attribute needs to load a module, do it before freezing the
@@ -734,6 +735,12 @@  ssize_t elv_iosched_store(struct gendisk *disk, const char *buf,
 
 	elv_iosched_load_module(name);
 
+	idx = srcu_read_lock(&set->update_nr_hwq_srcu);
+	if (set->updating_nr_hwq) {
+		ret = -EBUSY;
+		goto exit;
+	}
+
 	memflags = blk_mq_freeze_queue(q);
 	mutex_lock(&q->elevator_lock);
 	ret = elevator_change(q, name);
@@ -741,6 +748,8 @@  ssize_t elv_iosched_store(struct gendisk *disk, const char *buf,
 		ret = count;
 	mutex_unlock(&q->elevator_lock);
 	blk_mq_unfreeze_queue(q, memflags);
+exit:
+	srcu_read_unlock(&set->update_nr_hwq_srcu, idx);
 	return ret;
 }