Re: [PATCH v3 1/5] lib/bitmap: add bitmap_{set,get}_value()
From: Yury Norov <yury.norov@gmail.com>
Date: 2023-07-17 15:51:10
Also in:
lkml
On Mon, Jul 17, 2023 at 01:37:04PM +0200, Alexander Potapenko wrote:
The two new functions allow setting/getting 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 couple of minor changes: - 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())
Please preserve Syed's authorship ('From' field in git log).
quoted hunk ↗ jump to hunk
Cc: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Syed Nayyar Waris <redacted> Signed-off-by: William Breathitt Gray <redacted> Link: https://lore.kernel.org/lkml/fe12eedf3666f4af5138de0e70b67a07c7f40338.1592224129.git.syednwaris@gmail.com/ (local) Suggested-by: Yury Norov <yury.norov@gmail.com> Signed-off-by: Alexander Potapenko <glider@google.com> --- include/linux/bitmap.h | 57 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+)diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h index 03644237e1efb..4559366084988 100644 --- a/include/linux/bitmap.h +++ b/include/linux/bitmap.h@@ -76,7 +76,11 @@ struct device; * bitmap_to_arr32(buf, src, nbits) Copy nbits from buf to u32[] dst * bitmap_to_arr64(buf, src, nbits) Copy nbits from buf to u64[] dst * bitmap_get_value8(map, start) Get 8bit value from map at start + * bitmap_get_value(map, start, nbits) Get bit value of size 'nbits' + * from map at start * bitmap_set_value8(map, value, start) Set 8bit value to map at start + * bitmap_set_value(map, value, start, nbits) Set bit value of size 'nbits' + * of map at start
The 'bit value of size' sounds more confusing than it should. The size of bit is actually a bit... Can you rephrase? Moreover, 'set bits' has a meaning of actually setting them, i.e. switching to '1'. Maybe: "Copy 'nbits' to bitmap starting at 'start'"?
quoted hunk ↗ jump to hunk
* * Note, bitmap_zero() and bitmap_fill() operate over the region of * unsigned longs, that is, bits behind bitmap till the unsigned long@@ -583,6 +587,31 @@ static inline unsigned long bitmap_get_value8(const unsigned long *map, return (map[index] >> offset) & 0xFF; } +/** + * bitmap_get_value - get 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
* @nbits: size of value in bits, up to BITS_PER_LONG
+ *
+ * Returns value of nbits located at the @start bit offset within the @map
+ * memory region.
+ */
+static inline unsigned long bitmap_get_value(const unsigned long *map,
+ unsigned long start,
+ unsigned long nbits)
+{
+ const size_t index = BIT_WORD(start);
+ const unsigned long offset = start % BITS_PER_LONG;
+ const unsigned long space = BITS_PER_LONG - offset;
+ unsigned long value_low, value_high;
+
+ if (space >= nbits)
+ return (map[index] >> offset) & GENMASK(nbits - 1, 0);
+ value_low = map[index] & BITMAP_FIRST_WORD_MASK(start);
+ value_high = map[index + 1] & BITMAP_LAST_WORD_MASK(start + nbits);
+ return (value_low >> offset) | (value_high << space);
+}When nbits == 0, copy-like functions shouldn't touch any memory. See how other bitmap and find_bit functions hold it. Thanks, Yury _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel