drivers/virtio/virtio_mem.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-)
Encapsulate find_next_bit and find_next_zero_bit calls into helpers
to verify if a range of subblocks is entirely plugged or unplugged.
This improves readability as requested by the TODO comments in the
source code.
Validation confirms the change is binary-neutral on ARM64, with the
generated instructions remaining identical.
Suggested-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Luan Haickel Araujo <luanharaujo@gmail.com>
---
Hi David,
This is my first patch contribution to the Linux kernel, following up on
your TODO notes in virtio_mem.c.
As part of my learning process (LFX 103), I verified the change in a
QEMU (KVM) environment with test_bitmap. I also compared the generated
assembly on ARM64 and it remains identical, confirming the helpers are
binary-neutral as expected.
I noticed these helpers could also be useful in include/linux/bitmap.h in
the future. If you think it makes sense to move them there, I'd be happy
to work on that in a next step.
Looking forward to your feedback.
Luan.
drivers/virtio/virtio_mem.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
index 48051e9e98ab..e94e30af3a5d 100644
--- a/drivers/virtio/virtio_mem.c
+++ b/drivers/virtio/virtio_mem.c
@@ -538,6 +538,22 @@ static void virtio_mem_sbm_set_sb_unplugged(struct virtio_mem *vm,
__bitmap_clear(vm->sbm.sb_states, bit, count);
}
+static inline bool bitmap_test_range_all_set(const unsigned long *map,
+ unsigned int start, unsigned int nbits)
+{
+ unsigned int next_zero_bit = find_next_zero_bit(map, start + nbits, start);
+
+ return next_zero_bit >= start + nbits;
+}
+
+static inline bool bitmap_test_range_all_zero(const unsigned long *map,
+ unsigned int start, unsigned int nbits)
+{
+ unsigned int next_set_bit = find_next_bit(map, start + nbits, start);
+
+ return next_set_bit >= start + nbits;
+}
+
/*
* Test if all selected subblocks are plugged.
*/
@@ -550,9 +566,7 @@ static bool virtio_mem_sbm_test_sb_plugged(struct virtio_mem *vm,
if (count == 1)
return test_bit(bit, vm->sbm.sb_states);
- /* TODO: Helper similar to bitmap_set() */
- return find_next_zero_bit(vm->sbm.sb_states, bit + count, bit) >=
- bit + count;
+ return bitmap_test_range_all_set(vm->sbm.sb_states, bit, count);
}
/*
@@ -564,9 +578,7 @@ static bool virtio_mem_sbm_test_sb_unplugged(struct virtio_mem *vm,
{
const int bit = virtio_mem_sbm_sb_state_bit_nr(vm, mb_id, sb_id);
- /* TODO: Helper similar to bitmap_set() */
- return find_next_bit(vm->sbm.sb_states, bit + count, bit) >=
- bit + count;
+ return bitmap_test_range_all_zero(vm->sbm.sb_states, bit, count);
}
/*
--
2.53.0
On 4/5/26 23:32, Luan Haickel Araujo wrote:
> Encapsulate find_next_bit and find_next_zero_bit calls into helpers
> to verify if a range of subblocks is entirely plugged or unplugged.
>
> This improves readability as requested by the TODO comments in the
> source code.
>
> Validation confirms the change is binary-neutral on ARM64, with the
> generated instructions remaining identical.
>
> Suggested-by: David Hildenbrand <david@redhat.com>
>
> Signed-off-by: Luan Haickel Araujo <luanharaujo@gmail.com>
> ---
> Hi David,
>
Hi!
> This is my first patch contribution to the Linux kernel, following up on
> your TODO notes in virtio_mem.c.
>
> As part of my learning process (LFX 103), I verified the change in a
> QEMU (KVM) environment with test_bitmap. I also compared the generated
> assembly on ARM64 and it remains identical, confirming the helpers are
> binary-neutral as expected.
>
> I noticed these helpers could also be useful in include/linux/bitmap.h in
> the future. If you think it makes sense to move them there, I'd be happy
> to work on that in a next step.
Right, the TODO really indicated that core code should provide such
helpers, not virtio-mem itself. There is no real benefit in having these
local helpers.
>
> Looking forward to your feedback.
>
> Luan.
> drivers/virtio/virtio_mem.c | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
> index 48051e9e98ab..e94e30af3a5d 100644
> --- a/drivers/virtio/virtio_mem.c
> +++ b/drivers/virtio/virtio_mem.c
> @@ -538,6 +538,22 @@ static void virtio_mem_sbm_set_sb_unplugged(struct virtio_mem *vm,
> __bitmap_clear(vm->sbm.sb_states, bit, count);
> }
>
> +static inline bool bitmap_test_range_all_set(const unsigned long *map,
> + unsigned int start, unsigned int nbits)
> +{
> + unsigned int next_zero_bit = find_next_zero_bit(map, start + nbits, start);
> +
> + return next_zero_bit >= start + nbits;
> +}
> +
> +static inline bool bitmap_test_range_all_zero(const unsigned long *map,
> + unsigned int start, unsigned int nbits)
> +{
> + unsigned int next_set_bit = find_next_bit(map, start + nbits, start);
> +
> + return next_set_bit >= start + nbits;
> +}
If you look closely, fs/btrfs/misc.h provides these functions. So they
should probably be factored out into common code.
Then, virtio-mem could just naturally reuse them.
But, I wonder whether we want to call them
bitmap_test_set()
bitmap_test_clear()
Given that they have the exact same interface as
bitmap_set/bitmap_clear, consuming a range.
--
Cheers,
David
© 2016 - 2026 Red Hat, Inc.