Re: [PATCH RESEND v2 3/5] sbitmap: rewrite sbitmap_find_bit_in_index to reduce repeat code

From: Jan Kara
Date: Thu Dec 22 2022 - 07:23:33 EST


On Thu 22-12-22 22:33:51, Kemeng Shi wrote:
> Rewrite sbitmap_find_bit_in_index as following:
> 1. Rename sbitmap_find_bit_in_index to sbitmap_find_bit_in_word
> 2. Accept "struct sbitmap_word *" directly instead of accepting
> "struct sbitmap *" and "int index" to get "struct sbitmap_word *".
> 3. Accept depth/shallow_depth and wrap for __sbitmap_get_word from caller
> to support need of both __sbitmap_get_shallow and __sbitmap_get.
>
> With helper function sbitmap_find_bit_in_word, we can remove repeat
> code in __sbitmap_get_shallow to find bit considring deferred clear.
>
> Signed-off-by: Kemeng Shi <shikemeng@xxxxxxxxxxxxxxx>
> ---
> lib/sbitmap.c | 29 ++++++++++++++---------------
> 1 file changed, 14 insertions(+), 15 deletions(-)

Just one style nit below. Please fix that up. Otherwise feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

> diff --git a/lib/sbitmap.c b/lib/sbitmap.c
> index 11e75f4040fb..3f7e276a427d 100644
> --- a/lib/sbitmap.c
> +++ b/lib/sbitmap.c
> @@ -167,15 +167,16 @@ static int __sbitmap_get_word(unsigned long *word, unsigned long depth,
> return nr;
> }
>
> -static int sbitmap_find_bit_in_index(struct sbitmap *sb, int index,
> - unsigned int alloc_hint)
> +static int sbitmap_find_bit_in_word(struct sbitmap_word *map,
> + unsigned int depth,
> + unsigned int alloc_hint,
> + bool wrap)
> {
> - struct sbitmap_word *map = &sb->map[index];
> int nr;
>
> do {
> - nr = __sbitmap_get_word(&map->word, __map_depth(sb, index),
> - alloc_hint, !sb->round_robin);
> + nr = __sbitmap_get_word(&map->word, depth,
> + alloc_hint, wrap);
> if (nr != -1)
> break;
> if (!sbitmap_deferred_clear(map))
> @@ -203,7 +204,8 @@ static int __sbitmap_get(struct sbitmap *sb, unsigned int alloc_hint)
> alloc_hint = 0;
>
> for (i = 0; i < sb->map_nr; i++) {
> - nr = sbitmap_find_bit_in_index(sb, index, alloc_hint);
> + nr = sbitmap_find_bit_in_word(&sb->map[index], __map_depth(sb, index),

Please avoid lines over 80 characters.

> + alloc_hint, !sb->round_robin);
> if (nr != -1) {
> nr += index << sb->shift;
> break;

Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR