Re: [PATCH 5.10.y-cip 07/28] lib/bitmap: add bitmap_{read,write}()

Claudiu Beznea
2026-03-25T08:37:33Z

Hi, Pavel,

On 3/24/26 11:29, Pavel Machek wrote:
> Hi!
>
>> From: Syed Nayyar Waris <syednwaris@gmail.com>
>>
>> commit 63c15822b8dd02a2423cfd92232245ace3f7a11b upstream.
>>
>> The two new functions allow reading/writing values of length up to
>> BITS_PER_LONG bits at arbitrary position in the bitmap.
>>
>> The code was taken from "bitops: Introduce the for_each_set_clump macro"
>> by Syed Nayyar Waris with a number of changes and simplifications:
>> - instead of using roundup(), which adds an unnecessary dependency
>> on <linux/math.h>, we calculate space as BITS_PER_LONG-offset;
>> - indentation is reduced by not using else-clauses (suggested by
>> checkpatch for bitmap_get_value());
>> - bitmap_get_value()/bitmap_set_value() are renamed to bitmap_read()
>> and bitmap_write();
>> - some redundant computations are omitted.
>>
>> +++ b/include/linux/bitmap.h
>> @@ -601,6 +605,79 @@ static inline void bitmap_set_value8(unsigned long *map, unsigned long value,
>> map[index] |= value << offset;
>> }
>>
>> +/**
>> + * bitmap_read - read a value of n-bits from the memory region
>> + * @map: address to the bitmap memory region
>> + * @start: bit offset of the n-bit value
>> + * @nbits: size of value in bits, nonzero, up to BITS_PER_LONG
>> + *
>> + * Returns: value of @nbits bits located at the @start bit offset within the
>> + * @map memory region. For @nbits = 0 and @nbits > BITS_PER_LONG the return
>> + * value is undefined.
>> + */
>> +static inline unsigned long bitmap_read(const unsigned long *map,
>> + unsigned long start,
>> + unsigned long nbits)
>> +{
>> + size_t index = BIT_WORD(start);
>> + unsigned long offset = start % BITS_PER_LONG;
>> + unsigned long space = BITS_PER_LONG - offset;
>> + unsigned long value_low, value_high;
>> +
>> + if (unlikely(!nbits || nbits > BITS_PER_LONG))
>> + return 0;
>
> If someone attempts to read more than bits_per_long, we silently give
> him 0. That sounds like recipe for hard-to-debug problem when the code
> designed for 64-bit system is ran on 32-bit.

The documentation states it explicitly:

"For @nbits = 0 and @nbits > BITS_PER_LONG the return value is undefined."

>
> I'd expect WARN_ON here.

According to [1] the use of WARN()/WARN_ON() seems discouraged nowadays [1]

[1] https://lwn.net/Articles/969923/

>
>> +static inline void bitmap_write(unsigned long *map, unsigned long value,
>> + unsigned long start, unsigned long nbits)
>> +{
>> + size_t index;
>> + unsigned long offset;
>> + unsigned long space;
>> + unsigned long mask;
>> + bool fit;
>> +
>> + if (unlikely(!nbits || nbits > BITS_PER_LONG))
>> + return;
>
> And similary here.

Documentation also states "For @nbits == 0 and @nbits > BITS_PER_LONG no writes
are performed."

Please let me know how would you like me to proceed with it.

Thank you for your review,
Claudiu