[PATCH] lib/sbitmap: define swap_lock as raw_spinlock_t

[PATCH] lib/sbitmap: define swap_lock as raw_spinlock_t

[Ming Lei]

When called from sbitmap_queue_get(), sbitmap_deferred_clear() may be run
with preempt disabled. In RT kernel, spin_lock() can sleep, then warning
of "BUG: sleeping function called from invalid context" can be triggered.

Fix it by replacing it with raw_spin_lock.

Cc: Yang Yang <yang.yang@vivo.com>
Fixes: 72d04bdcf3f7 ("sbitmap: fix io hung due to race on sbitmap_word::cleared")
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 include/linux/sbitmap.h | 2 +-
 lib/sbitmap.c           | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/sbitmap.h b/include/linux/sbitmap.h
index c09cdcc99471..189140bf11fc 100644
--- a/include/linux/sbitmap.h
+++ b/include/linux/sbitmap.h
@@ -40,7 +40,7 @@ struct sbitmap_word {
 	/**
 	 * @swap_lock: serializes simultaneous updates of ->word and ->cleared
 	 */
-	spinlock_t swap_lock;
+	raw_spinlock_t swap_lock;
 } ____cacheline_aligned_in_smp;
 
 /**
diff --git a/lib/sbitmap.c b/lib/sbitmap.c
index 5e2e93307f0d..d3412984170c 100644
--- a/lib/sbitmap.c
+++ b/lib/sbitmap.c
@@ -65,7 +65,7 @@ static inline bool sbitmap_deferred_clear(struct sbitmap_word *map,
 {
 	unsigned long mask, word_mask;
 
-	guard(spinlock_irqsave)(&map->swap_lock);
+	guard(raw_spinlock_irqsave)(&map->swap_lock);
 
 	if (!map->cleared) {
 		if (depth == 0)
@@ -136,7 +136,7 @@ int sbitmap_init_node(struct sbitmap *sb, unsigned int depth, int shift,
 	}
 
 	for (i = 0; i < sb->map_nr; i++)
-		spin_lock_init(&sb->map[i].swap_lock);
+		raw_spin_lock_init(&sb->map[i].swap_lock);
 
 	return 0;
 }
-- 
2.46.0

Re: [PATCH] lib/sbitmap: define swap_lock as raw_spinlock_t

[YangYang]

On 2024/9/19 10:17, Ming Lei wrote:
> When called from sbitmap_queue_get(), sbitmap_deferred_clear() may be run
> with preempt disabled. In RT kernel, spin_lock() can sleep, then warning
> of "BUG: sleeping function called from invalid context" can be triggered.
> 
> Fix it by replacing it with raw_spin_lock.
> 
> Cc: Yang Yang <yang.yang@vivo.com>
> Fixes: 72d04bdcf3f7 ("sbitmap: fix io hung due to race on sbitmap_word::cleared")
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>   include/linux/sbitmap.h | 2 +-
>   lib/sbitmap.c           | 4 ++--
>   2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/sbitmap.h b/include/linux/sbitmap.h
> index c09cdcc99471..189140bf11fc 100644
> --- a/include/linux/sbitmap.h
> +++ b/include/linux/sbitmap.h
> @@ -40,7 +40,7 @@ struct sbitmap_word {
>   	/**
>   	 * @swap_lock: serializes simultaneous updates of ->word and ->cleared
>   	 */
> -	spinlock_t swap_lock;
> +	raw_spinlock_t swap_lock;
>   } ____cacheline_aligned_in_smp;
>   
>   /**
> diff --git a/lib/sbitmap.c b/lib/sbitmap.c
> index 5e2e93307f0d..d3412984170c 100644
> --- a/lib/sbitmap.c
> +++ b/lib/sbitmap.c
> @@ -65,7 +65,7 @@ static inline bool sbitmap_deferred_clear(struct sbitmap_word *map,
>   {
>   	unsigned long mask, word_mask;
>   
> -	guard(spinlock_irqsave)(&map->swap_lock);
> +	guard(raw_spinlock_irqsave)(&map->swap_lock);
>   
>   	if (!map->cleared) {
>   		if (depth == 0)
> @@ -136,7 +136,7 @@ int sbitmap_init_node(struct sbitmap *sb, unsigned int depth, int shift,
>   	}
>   
>   	for (i = 0; i < sb->map_nr; i++)
> -		spin_lock_init(&sb->map[i].swap_lock);
> +		raw_spin_lock_init(&sb->map[i].swap_lock);
>   
>   	return 0;
>   }

Looks good. Feel free to add:

Reviewed-by: Yang Yang <yang.yang@vivo.com>

Thanks.

Re: [PATCH] lib/sbitmap: define swap_lock as raw_spinlock_t

[Jens Axboe]

On Thu, 19 Sep 2024 10:17:09 +0800, Ming Lei wrote:
> When called from sbitmap_queue_get(), sbitmap_deferred_clear() may be run
> with preempt disabled. In RT kernel, spin_lock() can sleep, then warning
> of "BUG: sleeping function called from invalid context" can be triggered.
> 
> Fix it by replacing it with raw_spin_lock.
> 
> 
> [...]

Applied, thanks!

[1/1] lib/sbitmap: define swap_lock as raw_spinlock_t
      commit: 65f666c6203600053478ce8e34a1db269a8701c9

Best regards,
-- 
Jens Axboe