* [PATCH v8 0/4] Introduce the for_each_set_clump macro
@ 2020-06-15 12:50 Syed Nayyar Waris
2020-06-15 12:50 ` Syed Nayyar Waris
2020-06-15 12:51 ` [PATCH v8 1/4] bitops: " Syed Nayyar Waris
0 siblings, 2 replies; 10+ messages in thread
From: Syed Nayyar Waris @ 2020-06-15 12:50 UTC (permalink / raw)
To: akpm
Cc: linux-arch, amit.kucheria, arnd, yamada.masahiro, linux-kernel,
linus.walleij, daniel.lezcano, vilhelm.gray, michal.simek,
bgolaszewski, rrichter, linux-gpio, linux-pm, rui.zhang,
andriy.shevchenko, linux-arm-kernel
Hello Linus,
Since this patchset primarily affects GPIO drivers, would you like
to pick it up through your GPIO tree?
This patchset introduces a new generic version of for_each_set_clump.
The previous version of for_each_set_clump8 used a fixed size 8-bit
clump, but the new generic version can work with clump of any size but
less than or equal to BITS_PER_LONG. The patchset utilizes the new macro
in several GPIO drivers.
The earlier 8-bit for_each_set_clump8 facilitated a
for-loop syntax that iterates over a memory region entire groups of set
bits at a time.
For example, suppose you would like to iterate over a 32-bit integer 8
bits at a time, skipping over 8-bit groups with no set bit, where
XXXXXXXX represents the current 8-bit group:
Example: 10111110 00000000 11111111 00110011
First loop: 10111110 00000000 11111111 XXXXXXXX
Second loop: 10111110 00000000 XXXXXXXX 00110011
Third loop: XXXXXXXX 00000000 11111111 00110011
Each iteration of the loop returns the next 8-bit group that has at
least one set bit.
But with the new for_each_set_clump the clump size can be different from 8 bits.
Moreover, the clump can be split at word boundary in situations where word
size is not multiple of clump size. Following are examples showing the working
of new macro for clump sizes of 24 bits and 6 bits.
Example 1:
clump size: 24 bits, Number of clumps (or ports): 10
bitmap stores the bit information from where successive clumps are retrieved.
/* bitmap memory region */
0x00aa0000ff000000; /* Most significant bits */
0xaaaaaa0000ff0000;
0x000000aa000000aa;
0xbbbbabcdeffedcba; /* Least significant bits */
Different iterations of for_each_set_clump:-
'offset' is the bit position and 'clump' is the 24 bit clump from the
above bitmap.
Iteration first: offset: 0 clump: 0xfedcba
Iteration second: offset: 24 clump: 0xabcdef
Iteration third: offset: 48 clump: 0xaabbbb
Iteration fourth: offset: 96 clump: 0xaa
Iteration fifth: offset: 144 clump: 0xff
Iteration sixth: offset: 168 clump: 0xaaaaaa
Iteration seventh: offset: 216 clump: 0xff
Loop breaks because in the end the remaining bits (0x00aa) size was less
than clump size of 24 bits.
In above example it can be seen that in iteration third, the 24 bit clump
that was retrieved was split between bitmap[0] and bitmap[1]. This example
also shows that 24 bit zeroes if present in between, were skipped (preserving
the previous for_each_set_macro8 behaviour).
Example 2:
clump size = 6 bits, Number of clumps (or ports) = 3.
/* bitmap memory region */
0x00aa0000ff000000; /* Most significant bits */
0xaaaaaa0000ff0000;
0x0f00000000000000;
0x0000000000000ac0; /* Least significant bits */
Different iterations of for_each_set_clump:
'offset' is the bit position and 'clump' is the 6 bit clump from the
above bitmap.
Iteration first: offset: 6 clump: 0x2b
Loop breaks because 6 * 3 = 18 bits traversed in bitmap.
Here 6 * 3 is clump size * no. of clumps.
Changes in v8:
- [Patch 2/4]: Minor change: Use '__initdata' for correct section mismatch
in 'clump_test_data' array.
Changes in v7:
- [Patch 2/4]: Minor changes: Use macro 'DECLARE_BITMAP()' and split 'struct'
definition and test data.
Changes in v6:
- [Patch 2/4]: Make 'for loop' inside test_for_each_set_clump more
succinct.
Changes in v5:
- [Patch 4/4]: Minor change: Hardcode value for better code readability.
Changes in v4:
- [Patch 2/4]: Use 'for' loop in test function of for_each_set_clump.
- [Patch 3/4]: Minor change: Inline value for better code readability.
- [Patch 4/4]: Minor change: Inline value for better code readability.
Changes in v3:
- [Patch 3/4]: Change datatype of some variables from u64 to unsigned long
in function thunderx_gpio_set_multiple.
CHanges in v2:
- [Patch 2/4]: Unify different tests for 'for_each_set_clump'. Pass test data as
function parameters.
- [Patch 2/4]: Remove unnecessary bitmap_zero calls.
Syed Nayyar Waris (4):
bitops: Introduce the for_each_set_clump macro
lib/test_bitmap.c: Add for_each_set_clump test cases
gpio: thunderx: Utilize for_each_set_clump macro
gpio: xilinx: Utilize for_each_set_clump macro
drivers/gpio/gpio-thunderx.c | 11 ++-
drivers/gpio/gpio-xilinx.c | 62 ++++++-------
include/asm-generic/bitops/find.h | 19 ++++
include/linux/bitmap.h | 61 +++++++++++++
include/linux/bitops.h | 13 +++
lib/find_bit.c | 14 +++
lib/test_bitmap.c | 145 ++++++++++++++++++++++++++++++
7 files changed, 291 insertions(+), 34 deletions(-)
base-commit: 444fc5cde64330661bf59944c43844e7d4c2ccd8
--
2.26.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v8 0/4] Introduce the for_each_set_clump macro
2020-06-15 12:50 [PATCH v8 0/4] Introduce the for_each_set_clump macro Syed Nayyar Waris
@ 2020-06-15 12:50 ` Syed Nayyar Waris
2020-06-15 12:51 ` [PATCH v8 1/4] bitops: " Syed Nayyar Waris
1 sibling, 0 replies; 10+ messages in thread
From: Syed Nayyar Waris @ 2020-06-15 12:50 UTC (permalink / raw)
To: linus.walleij, akpm
Cc: andriy.shevchenko, vilhelm.gray, michal.simek, arnd, rrichter,
bgolaszewski, yamada.masahiro, rui.zhang, daniel.lezcano,
amit.kucheria, linux-arch, linux-gpio, linux-kernel,
linux-arm-kernel, linux-pm
Hello Linus,
Since this patchset primarily affects GPIO drivers, would you like
to pick it up through your GPIO tree?
This patchset introduces a new generic version of for_each_set_clump.
The previous version of for_each_set_clump8 used a fixed size 8-bit
clump, but the new generic version can work with clump of any size but
less than or equal to BITS_PER_LONG. The patchset utilizes the new macro
in several GPIO drivers.
The earlier 8-bit for_each_set_clump8 facilitated a
for-loop syntax that iterates over a memory region entire groups of set
bits at a time.
For example, suppose you would like to iterate over a 32-bit integer 8
bits at a time, skipping over 8-bit groups with no set bit, where
XXXXXXXX represents the current 8-bit group:
Example: 10111110 00000000 11111111 00110011
First loop: 10111110 00000000 11111111 XXXXXXXX
Second loop: 10111110 00000000 XXXXXXXX 00110011
Third loop: XXXXXXXX 00000000 11111111 00110011
Each iteration of the loop returns the next 8-bit group that has at
least one set bit.
But with the new for_each_set_clump the clump size can be different from 8 bits.
Moreover, the clump can be split at word boundary in situations where word
size is not multiple of clump size. Following are examples showing the working
of new macro for clump sizes of 24 bits and 6 bits.
Example 1:
clump size: 24 bits, Number of clumps (or ports): 10
bitmap stores the bit information from where successive clumps are retrieved.
/* bitmap memory region */
0x00aa0000ff000000; /* Most significant bits */
0xaaaaaa0000ff0000;
0x000000aa000000aa;
0xbbbbabcdeffedcba; /* Least significant bits */
Different iterations of for_each_set_clump:-
'offset' is the bit position and 'clump' is the 24 bit clump from the
above bitmap.
Iteration first: offset: 0 clump: 0xfedcba
Iteration second: offset: 24 clump: 0xabcdef
Iteration third: offset: 48 clump: 0xaabbbb
Iteration fourth: offset: 96 clump: 0xaa
Iteration fifth: offset: 144 clump: 0xff
Iteration sixth: offset: 168 clump: 0xaaaaaa
Iteration seventh: offset: 216 clump: 0xff
Loop breaks because in the end the remaining bits (0x00aa) size was less
than clump size of 24 bits.
In above example it can be seen that in iteration third, the 24 bit clump
that was retrieved was split between bitmap[0] and bitmap[1]. This example
also shows that 24 bit zeroes if present in between, were skipped (preserving
the previous for_each_set_macro8 behaviour).
Example 2:
clump size = 6 bits, Number of clumps (or ports) = 3.
/* bitmap memory region */
0x00aa0000ff000000; /* Most significant bits */
0xaaaaaa0000ff0000;
0x0f00000000000000;
0x0000000000000ac0; /* Least significant bits */
Different iterations of for_each_set_clump:
'offset' is the bit position and 'clump' is the 6 bit clump from the
above bitmap.
Iteration first: offset: 6 clump: 0x2b
Loop breaks because 6 * 3 = 18 bits traversed in bitmap.
Here 6 * 3 is clump size * no. of clumps.
Changes in v8:
- [Patch 2/4]: Minor change: Use '__initdata' for correct section mismatch
in 'clump_test_data' array.
Changes in v7:
- [Patch 2/4]: Minor changes: Use macro 'DECLARE_BITMAP()' and split 'struct'
definition and test data.
Changes in v6:
- [Patch 2/4]: Make 'for loop' inside test_for_each_set_clump more
succinct.
Changes in v5:
- [Patch 4/4]: Minor change: Hardcode value for better code readability.
Changes in v4:
- [Patch 2/4]: Use 'for' loop in test function of for_each_set_clump.
- [Patch 3/4]: Minor change: Inline value for better code readability.
- [Patch 4/4]: Minor change: Inline value for better code readability.
Changes in v3:
- [Patch 3/4]: Change datatype of some variables from u64 to unsigned long
in function thunderx_gpio_set_multiple.
CHanges in v2:
- [Patch 2/4]: Unify different tests for 'for_each_set_clump'. Pass test data as
function parameters.
- [Patch 2/4]: Remove unnecessary bitmap_zero calls.
Syed Nayyar Waris (4):
bitops: Introduce the for_each_set_clump macro
lib/test_bitmap.c: Add for_each_set_clump test cases
gpio: thunderx: Utilize for_each_set_clump macro
gpio: xilinx: Utilize for_each_set_clump macro
drivers/gpio/gpio-thunderx.c | 11 ++-
drivers/gpio/gpio-xilinx.c | 62 ++++++-------
include/asm-generic/bitops/find.h | 19 ++++
include/linux/bitmap.h | 61 +++++++++++++
include/linux/bitops.h | 13 +++
lib/find_bit.c | 14 +++
lib/test_bitmap.c | 145 ++++++++++++++++++++++++++++++
7 files changed, 291 insertions(+), 34 deletions(-)
base-commit: 444fc5cde64330661bf59944c43844e7d4c2ccd8
--
2.26.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v8 1/4] bitops: Introduce the for_each_set_clump macro
2020-06-15 12:50 [PATCH v8 0/4] Introduce the for_each_set_clump macro Syed Nayyar Waris
2020-06-15 12:50 ` Syed Nayyar Waris
@ 2020-06-15 12:51 ` Syed Nayyar Waris
2020-06-15 13:56 ` Andy Shevchenko
` (3 more replies)
1 sibling, 4 replies; 10+ messages in thread
From: Syed Nayyar Waris @ 2020-06-15 12:51 UTC (permalink / raw)
To: linus.walleij, akpm
Cc: andriy.shevchenko, vilhelm.gray, arnd, linux-arch, linux-kernel
This macro iterates for each group of bits (clump) with set bits,
within a bitmap memory region. For each iteration, "start" is set to
the bit offset of the found clump, while the respective clump value is
stored to the location pointed by "clump". Additionally, the
bitmap_get_value and bitmap_set_value functions are introduced to
respectively get and set a value of n-bits in a bitmap memory region.
The n-bits can have any size less than or equal to BITS_PER_LONG.
Moreover, during setting value of n-bit in bitmap, if a situation arise
that the width of next n-bit is exceeding the word boundary, then it
will divide itself such that some portion of it is stored in that word,
while the remaining portion is stored in the next higher word. Similar
situation occurs while retrieving value of n-bits from bitmap.
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Syed Nayyar Waris <syednwaris@gmail.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
---
Changes in v8:
- No change.
Changes in v7:
- No change.
Changes in v6:
- No change.
Changes in v5:
- No change.
Changes in v4:
- No change.
Changes in v3:
- No change.
Changes in v2:
- No change.
include/asm-generic/bitops/find.h | 19 ++++++++++
include/linux/bitmap.h | 61 +++++++++++++++++++++++++++++++
include/linux/bitops.h | 13 +++++++
lib/find_bit.c | 14 +++++++
4 files changed, 107 insertions(+)
diff --git a/include/asm-generic/bitops/find.h b/include/asm-generic/bitops/find.h
index 9fdf21302fdf..4e6600759455 100644
--- a/include/asm-generic/bitops/find.h
+++ b/include/asm-generic/bitops/find.h
@@ -97,4 +97,23 @@ extern unsigned long find_next_clump8(unsigned long *clump,
#define find_first_clump8(clump, bits, size) \
find_next_clump8((clump), (bits), (size), 0)
+/**
+ * find_next_clump - find next clump with set bits in a memory region
+ * @clump: location to store copy of found clump
+ * @addr: address to base the search on
+ * @size: bitmap size in number of bits
+ * @offset: bit offset at which to start searching
+ * @clump_size: clump size in bits
+ *
+ * Returns the bit offset for the next set clump; the found clump value is
+ * copied to the location pointed by @clump. If no bits are set, returns @size.
+ */
+extern unsigned long find_next_clump(unsigned long *clump,
+ const unsigned long *addr,
+ unsigned long size, unsigned long offset,
+ unsigned long clump_size);
+
+#define find_first_clump(clump, bits, size, clump_size) \
+ find_next_clump((clump), (bits), (size), 0, (clump_size))
+
#endif /*_ASM_GENERIC_BITOPS_FIND_H_ */
diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 99058eb81042..7ab2c65fc964 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -75,7 +75,11 @@
* bitmap_from_arr32(dst, buf, nbits) Copy nbits from u32[] buf to dst
* bitmap_to_arr32(buf, src, nbits) Copy nbits from buf to u32[] 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
*
* Note, bitmap_zero() and bitmap_fill() operate over the region of
* unsigned longs, that is, bits behind bitmap till the unsigned long
@@ -563,6 +567,34 @@ 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
+ *
+ * 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 ceiling = roundup(start + 1, BITS_PER_LONG);
+ const unsigned long space = ceiling - start;
+ unsigned long value_low, value_high;
+
+ if (space >= nbits)
+ return (map[index] >> offset) & GENMASK(nbits - 1, 0);
+ else {
+ 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);
+ }
+}
+
/**
* bitmap_set_value8 - set an 8-bit value within a memory region
* @map: address to the bitmap memory region
@@ -579,6 +611,35 @@ static inline void bitmap_set_value8(unsigned long *map, unsigned long value,
map[index] |= value << offset;
}
+/**
+ * bitmap_set_value - set n-bit value within a memory region
+ * @map: address to the bitmap memory region
+ * @value: value of nbits
+ * @start: bit offset of the n-bit value
+ * @nbits: size of value in bits
+ */
+static inline void bitmap_set_value(unsigned long *map,
+ unsigned long value,
+ unsigned long start, unsigned long nbits)
+{
+ const size_t index = BIT_WORD(start);
+ const unsigned long offset = start % BITS_PER_LONG;
+ const unsigned long ceiling = roundup(start + 1, BITS_PER_LONG);
+ const unsigned long space = ceiling - start;
+
+ value &= GENMASK(nbits - 1, 0);
+
+ if (space >= nbits) {
+ map[index] &= ~(GENMASK(nbits + offset - 1, offset));
+ map[index] |= value << offset;
+ } else {
+ map[index] &= ~BITMAP_FIRST_WORD_MASK(start);
+ map[index] |= value << offset;
+ map[index + 1] &= ~BITMAP_LAST_WORD_MASK(start + nbits);
+ map[index + 1] |= (value >> space);
+ }
+}
+
#endif /* __ASSEMBLY__ */
#endif /* __LINUX_BITMAP_H */
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index 9acf654f0b19..41c2d9ce63e7 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -62,6 +62,19 @@ extern unsigned long __sw_hweight64(__u64 w);
(start) < (size); \
(start) = find_next_clump8(&(clump), (bits), (size), (start) + 8))
+/**
+ * for_each_set_clump - iterate over bitmap for each clump with set bits
+ * @start: bit offset to start search and to store the current iteration offset
+ * @clump: location to store copy of current 8-bit clump
+ * @bits: bitmap address to base the search on
+ * @size: bitmap size in number of bits
+ * @clump_size: clump size in bits
+ */
+#define for_each_set_clump(start, clump, bits, size, clump_size) \
+ for ((start) = find_first_clump(&(clump), (bits), (size), (clump_size)); \
+ (start) < (size); \
+ (start) = find_next_clump(&(clump), (bits), (size), (start) + (clump_size), (clump_size)))
+
static inline int get_bitmask_order(unsigned int count)
{
int order;
diff --git a/lib/find_bit.c b/lib/find_bit.c
index 49f875f1baf7..1341bd39b32a 100644
--- a/lib/find_bit.c
+++ b/lib/find_bit.c
@@ -190,3 +190,17 @@ unsigned long find_next_clump8(unsigned long *clump, const unsigned long *addr,
return offset;
}
EXPORT_SYMBOL(find_next_clump8);
+
+unsigned long find_next_clump(unsigned long *clump, const unsigned long *addr,
+ unsigned long size, unsigned long offset,
+ unsigned long clump_size)
+{
+ offset = find_next_bit(addr, size, offset);
+ if (offset == size)
+ return size;
+
+ offset = rounddown(offset, clump_size);
+ *clump = bitmap_get_value(addr, offset, clump_size);
+ return offset;
+}
+EXPORT_SYMBOL(find_next_clump);
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v8 1/4] bitops: Introduce the for_each_set_clump macro
2020-06-15 12:51 ` [PATCH v8 1/4] bitops: " Syed Nayyar Waris
@ 2020-06-15 13:56 ` Andy Shevchenko
2020-06-15 13:56 ` Andy Shevchenko
2020-06-15 16:47 ` kernel test robot
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2020-06-15 13:56 UTC (permalink / raw)
To: Syed Nayyar Waris
Cc: linus.walleij, akpm, vilhelm.gray, arnd, linux-arch, linux-kernel
On Mon, Jun 15, 2020 at 06:21:18PM +0530, Syed Nayyar Waris wrote:
> This macro iterates for each group of bits (clump) with set bits,
> within a bitmap memory region. For each iteration, "start" is set to
> the bit offset of the found clump, while the respective clump value is
> stored to the location pointed by "clump". Additionally, the
> bitmap_get_value and bitmap_set_value functions are introduced to
> respectively get and set a value of n-bits in a bitmap memory region.
> The n-bits can have any size less than or equal to BITS_PER_LONG.
> Moreover, during setting value of n-bit in bitmap, if a situation arise
> that the width of next n-bit is exceeding the word boundary, then it
> will divide itself such that some portion of it is stored in that word,
> while the remaining portion is stored in the next higher word. Similar
> situation occurs while retrieving value of n-bits from bitmap.
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Syed Nayyar Waris <syednwaris@gmail.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
> ---
> Changes in v8:
> - No change.
>
> Changes in v7:
> - No change.
>
> Changes in v6:
> - No change.
>
> Changes in v5:
> - No change.
>
> Changes in v4:
> - No change.
>
> Changes in v3:
> - No change.
>
> Changes in v2:
> - No change.
>
> include/asm-generic/bitops/find.h | 19 ++++++++++
> include/linux/bitmap.h | 61 +++++++++++++++++++++++++++++++
> include/linux/bitops.h | 13 +++++++
> lib/find_bit.c | 14 +++++++
> 4 files changed, 107 insertions(+)
>
> diff --git a/include/asm-generic/bitops/find.h b/include/asm-generic/bitops/find.h
> index 9fdf21302fdf..4e6600759455 100644
> --- a/include/asm-generic/bitops/find.h
> +++ b/include/asm-generic/bitops/find.h
> @@ -97,4 +97,23 @@ extern unsigned long find_next_clump8(unsigned long *clump,
> #define find_first_clump8(clump, bits, size) \
> find_next_clump8((clump), (bits), (size), 0)
>
> +/**
> + * find_next_clump - find next clump with set bits in a memory region
> + * @clump: location to store copy of found clump
> + * @addr: address to base the search on
> + * @size: bitmap size in number of bits
> + * @offset: bit offset at which to start searching
> + * @clump_size: clump size in bits
> + *
> + * Returns the bit offset for the next set clump; the found clump value is
> + * copied to the location pointed by @clump. If no bits are set, returns @size.
> + */
> +extern unsigned long find_next_clump(unsigned long *clump,
> + const unsigned long *addr,
> + unsigned long size, unsigned long offset,
> + unsigned long clump_size);
> +
> +#define find_first_clump(clump, bits, size, clump_size) \
> + find_next_clump((clump), (bits), (size), 0, (clump_size))
> +
> #endif /*_ASM_GENERIC_BITOPS_FIND_H_ */
> diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
> index 99058eb81042..7ab2c65fc964 100644
> --- a/include/linux/bitmap.h
> +++ b/include/linux/bitmap.h
> @@ -75,7 +75,11 @@
> * bitmap_from_arr32(dst, buf, nbits) Copy nbits from u32[] buf to dst
> * bitmap_to_arr32(buf, src, nbits) Copy nbits from buf to u32[] 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
> *
> * Note, bitmap_zero() and bitmap_fill() operate over the region of
> * unsigned longs, that is, bits behind bitmap till the unsigned long
> @@ -563,6 +567,34 @@ 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
> + *
> + * 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 ceiling = roundup(start + 1, BITS_PER_LONG);
> + const unsigned long space = ceiling - start;
> + unsigned long value_low, value_high;
> +
> + if (space >= nbits)
> + return (map[index] >> offset) & GENMASK(nbits - 1, 0);
Andrew, note that this requires to have GENMASK() fix [1] applied.
[1]: https://lore.kernel.org/lkml/20200608221823.35799-1-rikard.falkeborn@gmail.com/
> + else {
> + 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);
> + }
> +}
> +
> /**
> * bitmap_set_value8 - set an 8-bit value within a memory region
> * @map: address to the bitmap memory region
> @@ -579,6 +611,35 @@ static inline void bitmap_set_value8(unsigned long *map, unsigned long value,
> map[index] |= value << offset;
> }
>
> +/**
> + * bitmap_set_value - set n-bit value within a memory region
> + * @map: address to the bitmap memory region
> + * @value: value of nbits
> + * @start: bit offset of the n-bit value
> + * @nbits: size of value in bits
> + */
> +static inline void bitmap_set_value(unsigned long *map,
> + unsigned long value,
> + unsigned long start, unsigned long nbits)
> +{
> + const size_t index = BIT_WORD(start);
> + const unsigned long offset = start % BITS_PER_LONG;
> + const unsigned long ceiling = roundup(start + 1, BITS_PER_LONG);
> + const unsigned long space = ceiling - start;
> +
> + value &= GENMASK(nbits - 1, 0);
> +
> + if (space >= nbits) {
> + map[index] &= ~(GENMASK(nbits + offset - 1, offset));
> + map[index] |= value << offset;
> + } else {
> + map[index] &= ~BITMAP_FIRST_WORD_MASK(start);
> + map[index] |= value << offset;
> + map[index + 1] &= ~BITMAP_LAST_WORD_MASK(start + nbits);
> + map[index + 1] |= (value >> space);
> + }
> +}
> +
> #endif /* __ASSEMBLY__ */
>
> #endif /* __LINUX_BITMAP_H */
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> index 9acf654f0b19..41c2d9ce63e7 100644
> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -62,6 +62,19 @@ extern unsigned long __sw_hweight64(__u64 w);
> (start) < (size); \
> (start) = find_next_clump8(&(clump), (bits), (size), (start) + 8))
>
> +/**
> + * for_each_set_clump - iterate over bitmap for each clump with set bits
> + * @start: bit offset to start search and to store the current iteration offset
> + * @clump: location to store copy of current 8-bit clump
> + * @bits: bitmap address to base the search on
> + * @size: bitmap size in number of bits
> + * @clump_size: clump size in bits
> + */
> +#define for_each_set_clump(start, clump, bits, size, clump_size) \
> + for ((start) = find_first_clump(&(clump), (bits), (size), (clump_size)); \
> + (start) < (size); \
> + (start) = find_next_clump(&(clump), (bits), (size), (start) + (clump_size), (clump_size)))
> +
> static inline int get_bitmask_order(unsigned int count)
> {
> int order;
> diff --git a/lib/find_bit.c b/lib/find_bit.c
> index 49f875f1baf7..1341bd39b32a 100644
> --- a/lib/find_bit.c
> +++ b/lib/find_bit.c
> @@ -190,3 +190,17 @@ unsigned long find_next_clump8(unsigned long *clump, const unsigned long *addr,
> return offset;
> }
> EXPORT_SYMBOL(find_next_clump8);
> +
> +unsigned long find_next_clump(unsigned long *clump, const unsigned long *addr,
> + unsigned long size, unsigned long offset,
> + unsigned long clump_size)
> +{
> + offset = find_next_bit(addr, size, offset);
> + if (offset == size)
> + return size;
> +
> + offset = rounddown(offset, clump_size);
> + *clump = bitmap_get_value(addr, offset, clump_size);
> + return offset;
> +}
> +EXPORT_SYMBOL(find_next_clump);
> --
> 2.26.2
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v8 1/4] bitops: Introduce the for_each_set_clump macro
2020-06-15 13:56 ` Andy Shevchenko
@ 2020-06-15 13:56 ` Andy Shevchenko
0 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2020-06-15 13:56 UTC (permalink / raw)
To: Syed Nayyar Waris
Cc: linus.walleij, akpm, vilhelm.gray, arnd, linux-arch, linux-kernel
On Mon, Jun 15, 2020 at 06:21:18PM +0530, Syed Nayyar Waris wrote:
> This macro iterates for each group of bits (clump) with set bits,
> within a bitmap memory region. For each iteration, "start" is set to
> the bit offset of the found clump, while the respective clump value is
> stored to the location pointed by "clump". Additionally, the
> bitmap_get_value and bitmap_set_value functions are introduced to
> respectively get and set a value of n-bits in a bitmap memory region.
> The n-bits can have any size less than or equal to BITS_PER_LONG.
> Moreover, during setting value of n-bit in bitmap, if a situation arise
> that the width of next n-bit is exceeding the word boundary, then it
> will divide itself such that some portion of it is stored in that word,
> while the remaining portion is stored in the next higher word. Similar
> situation occurs while retrieving value of n-bits from bitmap.
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Syed Nayyar Waris <syednwaris@gmail.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
> ---
> Changes in v8:
> - No change.
>
> Changes in v7:
> - No change.
>
> Changes in v6:
> - No change.
>
> Changes in v5:
> - No change.
>
> Changes in v4:
> - No change.
>
> Changes in v3:
> - No change.
>
> Changes in v2:
> - No change.
>
> include/asm-generic/bitops/find.h | 19 ++++++++++
> include/linux/bitmap.h | 61 +++++++++++++++++++++++++++++++
> include/linux/bitops.h | 13 +++++++
> lib/find_bit.c | 14 +++++++
> 4 files changed, 107 insertions(+)
>
> diff --git a/include/asm-generic/bitops/find.h b/include/asm-generic/bitops/find.h
> index 9fdf21302fdf..4e6600759455 100644
> --- a/include/asm-generic/bitops/find.h
> +++ b/include/asm-generic/bitops/find.h
> @@ -97,4 +97,23 @@ extern unsigned long find_next_clump8(unsigned long *clump,
> #define find_first_clump8(clump, bits, size) \
> find_next_clump8((clump), (bits), (size), 0)
>
> +/**
> + * find_next_clump - find next clump with set bits in a memory region
> + * @clump: location to store copy of found clump
> + * @addr: address to base the search on
> + * @size: bitmap size in number of bits
> + * @offset: bit offset at which to start searching
> + * @clump_size: clump size in bits
> + *
> + * Returns the bit offset for the next set clump; the found clump value is
> + * copied to the location pointed by @clump. If no bits are set, returns @size.
> + */
> +extern unsigned long find_next_clump(unsigned long *clump,
> + const unsigned long *addr,
> + unsigned long size, unsigned long offset,
> + unsigned long clump_size);
> +
> +#define find_first_clump(clump, bits, size, clump_size) \
> + find_next_clump((clump), (bits), (size), 0, (clump_size))
> +
> #endif /*_ASM_GENERIC_BITOPS_FIND_H_ */
> diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
> index 99058eb81042..7ab2c65fc964 100644
> --- a/include/linux/bitmap.h
> +++ b/include/linux/bitmap.h
> @@ -75,7 +75,11 @@
> * bitmap_from_arr32(dst, buf, nbits) Copy nbits from u32[] buf to dst
> * bitmap_to_arr32(buf, src, nbits) Copy nbits from buf to u32[] 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
> *
> * Note, bitmap_zero() and bitmap_fill() operate over the region of
> * unsigned longs, that is, bits behind bitmap till the unsigned long
> @@ -563,6 +567,34 @@ 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
> + *
> + * 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 ceiling = roundup(start + 1, BITS_PER_LONG);
> + const unsigned long space = ceiling - start;
> + unsigned long value_low, value_high;
> +
> + if (space >= nbits)
> + return (map[index] >> offset) & GENMASK(nbits - 1, 0);
Andrew, note that this requires to have GENMASK() fix [1] applied.
[1]: https://lore.kernel.org/lkml/20200608221823.35799-1-rikard.falkeborn@gmail.com/
> + else {
> + 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);
> + }
> +}
> +
> /**
> * bitmap_set_value8 - set an 8-bit value within a memory region
> * @map: address to the bitmap memory region
> @@ -579,6 +611,35 @@ static inline void bitmap_set_value8(unsigned long *map, unsigned long value,
> map[index] |= value << offset;
> }
>
> +/**
> + * bitmap_set_value - set n-bit value within a memory region
> + * @map: address to the bitmap memory region
> + * @value: value of nbits
> + * @start: bit offset of the n-bit value
> + * @nbits: size of value in bits
> + */
> +static inline void bitmap_set_value(unsigned long *map,
> + unsigned long value,
> + unsigned long start, unsigned long nbits)
> +{
> + const size_t index = BIT_WORD(start);
> + const unsigned long offset = start % BITS_PER_LONG;
> + const unsigned long ceiling = roundup(start + 1, BITS_PER_LONG);
> + const unsigned long space = ceiling - start;
> +
> + value &= GENMASK(nbits - 1, 0);
> +
> + if (space >= nbits) {
> + map[index] &= ~(GENMASK(nbits + offset - 1, offset));
> + map[index] |= value << offset;
> + } else {
> + map[index] &= ~BITMAP_FIRST_WORD_MASK(start);
> + map[index] |= value << offset;
> + map[index + 1] &= ~BITMAP_LAST_WORD_MASK(start + nbits);
> + map[index + 1] |= (value >> space);
> + }
> +}
> +
> #endif /* __ASSEMBLY__ */
>
> #endif /* __LINUX_BITMAP_H */
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> index 9acf654f0b19..41c2d9ce63e7 100644
> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -62,6 +62,19 @@ extern unsigned long __sw_hweight64(__u64 w);
> (start) < (size); \
> (start) = find_next_clump8(&(clump), (bits), (size), (start) + 8))
>
> +/**
> + * for_each_set_clump - iterate over bitmap for each clump with set bits
> + * @start: bit offset to start search and to store the current iteration offset
> + * @clump: location to store copy of current 8-bit clump
> + * @bits: bitmap address to base the search on
> + * @size: bitmap size in number of bits
> + * @clump_size: clump size in bits
> + */
> +#define for_each_set_clump(start, clump, bits, size, clump_size) \
> + for ((start) = find_first_clump(&(clump), (bits), (size), (clump_size)); \
> + (start) < (size); \
> + (start) = find_next_clump(&(clump), (bits), (size), (start) + (clump_size), (clump_size)))
> +
> static inline int get_bitmask_order(unsigned int count)
> {
> int order;
> diff --git a/lib/find_bit.c b/lib/find_bit.c
> index 49f875f1baf7..1341bd39b32a 100644
> --- a/lib/find_bit.c
> +++ b/lib/find_bit.c
> @@ -190,3 +190,17 @@ unsigned long find_next_clump8(unsigned long *clump, const unsigned long *addr,
> return offset;
> }
> EXPORT_SYMBOL(find_next_clump8);
> +
> +unsigned long find_next_clump(unsigned long *clump, const unsigned long *addr,
> + unsigned long size, unsigned long offset,
> + unsigned long clump_size)
> +{
> + offset = find_next_bit(addr, size, offset);
> + if (offset == size)
> + return size;
> +
> + offset = rounddown(offset, clump_size);
> + *clump = bitmap_get_value(addr, offset, clump_size);
> + return offset;
> +}
> +EXPORT_SYMBOL(find_next_clump);
> --
> 2.26.2
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v8 1/4] bitops: Introduce the for_each_set_clump macro
2020-06-15 12:51 ` [PATCH v8 1/4] bitops: " Syed Nayyar Waris
2020-06-15 13:56 ` Andy Shevchenko
@ 2020-06-15 16:47 ` kernel test robot
2020-06-16 8:14 ` Andy Shevchenko
2020-06-17 8:00 ` kernel test robot
3 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2020-06-15 16:47 UTC (permalink / raw)
To: Syed Nayyar Waris, linus.walleij, akpm
Cc: kbuild-all, andriy.shevchenko, vilhelm.gray, arnd, linux-arch,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 6544 bytes --]
Hi Syed,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on 444fc5cde64330661bf59944c43844e7d4c2ccd8]
url: https://github.com/0day-ci/linux/commits/Syed-Nayyar-Waris/Introduce-the-for_each_set_clump-macro/20200615-205729
base: 444fc5cde64330661bf59944c43844e7d4c2ccd8
config: ia64-randconfig-r003-20200615 (attached as .config)
compiler: ia64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=ia64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>, old ones prefixed by <<):
scripts/Makefile.build:59: 'arch/ia64/kernel/palinfo.ko' 'arch/ia64/kernel/mca_recovery.ko' 'arch/ia64/kernel/err_inject.ko' will not be built even though obj-m is specified.
scripts/Makefile.build:60: You cannot use subdir-y/m to visit a module Makefile. Use obj-y/m instead.
In file included from include/linux/bits.h:23,
from include/linux/bitops.h:5,
from include/linux/kernel.h:12,
from include/linux/list.h:9,
from include/linux/rculist.h:10,
from include/linux/sched/signal.h:5,
from arch/ia64/kernel/asm-offsets.c:10:
include/linux/bitmap.h: In function 'bitmap_get_value':
include/linux/bits.h:26:28: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
>> include/linux/bitmap.h:590:35: note: in expansion of macro 'GENMASK'
590 | return (map[index] >> offset) & GENMASK(nbits - 1, 0);
| ^~~~~~~
include/linux/bits.h:26:40: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
>> include/linux/bitmap.h:590:35: note: in expansion of macro 'GENMASK'
590 | return (map[index] >> offset) & GENMASK(nbits - 1, 0);
| ^~~~~~~
include/linux/bitmap.h: In function 'bitmap_set_value':
include/linux/bits.h:26:28: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
include/linux/bitmap.h:630:11: note: in expansion of macro 'GENMASK'
630 | value &= GENMASK(nbits - 1, 0);
| ^~~~~~~
include/linux/bits.h:26:40: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
include/linux/bitmap.h:630:11: note: in expansion of macro 'GENMASK'
630 | value &= GENMASK(nbits - 1, 0);
| ^~~~~~~
In file included from arch/ia64/include/asm/pgtable.h:154,
from arch/ia64/include/asm/uaccess.h:40,
from include/linux/uaccess.h:11,
from include/linux/sched/task.h:11,
from include/linux/sched/signal.h:9,
from arch/ia64/kernel/asm-offsets.c:10:
arch/ia64/include/asm/mmu_context.h: In function 'reload_context':
arch/ia64/include/asm/mmu_context.h:137:41: warning: variable 'old_rr4' set but not used [-Wunused-but-set-variable]
137 | unsigned long rr0, rr1, rr2, rr3, rr4, old_rr4;
| ^~~~~~~
arch/ia64/kernel/asm-offsets.c: At top level:
arch/ia64/kernel/asm-offsets.c:23:6: warning: no previous prototype for 'foo' [-Wmissing-prototypes]
23 | void foo(void)
| ^~~
<stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
vim +/GENMASK +590 include/linux/bitmap.h
569
570 /**
571 * bitmap_get_value - get a value of n-bits from the memory region
572 * @map: address to the bitmap memory region
573 * @start: bit offset of the n-bit value
574 * @nbits: size of value in bits
575 *
576 * Returns value of nbits located at the @start bit offset within the @map
577 * memory region.
578 */
579 static inline unsigned long bitmap_get_value(const unsigned long *map,
580 unsigned long start,
581 unsigned long nbits)
582 {
583 const size_t index = BIT_WORD(start);
584 const unsigned long offset = start % BITS_PER_LONG;
585 const unsigned long ceiling = roundup(start + 1, BITS_PER_LONG);
586 const unsigned long space = ceiling - start;
587 unsigned long value_low, value_high;
588
589 if (space >= nbits)
> 590 return (map[index] >> offset) & GENMASK(nbits - 1, 0);
591 else {
592 value_low = map[index] & BITMAP_FIRST_WORD_MASK(start);
593 value_high = map[index + 1] & BITMAP_LAST_WORD_MASK(start + nbits);
594 return (value_low >> offset) | (value_high << space);
595 }
596 }
597
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 41132 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v8 1/4] bitops: Introduce the for_each_set_clump macro
2020-06-15 12:51 ` [PATCH v8 1/4] bitops: " Syed Nayyar Waris
2020-06-15 13:56 ` Andy Shevchenko
2020-06-15 16:47 ` kernel test robot
@ 2020-06-16 8:14 ` Andy Shevchenko
2020-06-20 13:45 ` Syed Nayyar Waris
2020-06-20 23:41 ` Syed Nayyar Waris
2020-06-17 8:00 ` kernel test robot
3 siblings, 2 replies; 10+ messages in thread
From: Andy Shevchenko @ 2020-06-16 8:14 UTC (permalink / raw)
To: Syed Nayyar Waris
Cc: linus.walleij, akpm, vilhelm.gray, arnd, linux-arch, linux-kernel
On Mon, Jun 15, 2020 at 06:21:18PM +0530, Syed Nayyar Waris wrote:
> This macro iterates for each group of bits (clump) with set bits,
> within a bitmap memory region. For each iteration, "start" is set to
> the bit offset of the found clump, while the respective clump value is
> stored to the location pointed by "clump". Additionally, the
> bitmap_get_value and bitmap_set_value functions are introduced to
> respectively get and set a value of n-bits in a bitmap memory region.
> The n-bits can have any size less than or equal to BITS_PER_LONG.
> Moreover, during setting value of n-bit in bitmap, if a situation arise
> that the width of next n-bit is exceeding the word boundary, then it
> will divide itself such that some portion of it is stored in that word,
> while the remaining portion is stored in the next higher word. Similar
> situation occurs while retrieving value of n-bits from bitmap.
On the second view...
> +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 ceiling = roundup(start + 1, BITS_PER_LONG);
This perhaps should use round_up()
> + const unsigned long space = ceiling - start;
And I think I see a scenario to complain.
If start == 0, then ceiling will be 64.
space == 64. Not good.
> + unsigned long value_low, value_high;
> +
> + if (space >= nbits)
> + return (map[index] >> offset) & GENMASK(nbits - 1, 0);
> + else {
> + 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);
> + }
> +}
...
> +/**
> + * bitmap_set_value - set n-bit value within a memory region
> + * @map: address to the bitmap memory region
> + * @value: value of nbits
> + * @start: bit offset of the n-bit value
> + * @nbits: size of value in bits
> + */
> +static inline void bitmap_set_value(unsigned long *map,
> + unsigned long value,
> + unsigned long start, unsigned long nbits)
> +{
> + const size_t index = BIT_WORD(start);
> + const unsigned long offset = start % BITS_PER_LONG;
> + const unsigned long ceiling = roundup(start + 1, BITS_PER_LONG);
> + const unsigned long space = ceiling - start;
Ditto for both lines.
> + value &= GENMASK(nbits - 1, 0);
> +
> + if (space >= nbits) {
> + map[index] &= ~(GENMASK(nbits + offset - 1, offset));
> + map[index] |= value << offset;
> + } else {
> + map[index] &= ~BITMAP_FIRST_WORD_MASK(start);
> + map[index] |= value << offset;
> + map[index + 1] &= ~BITMAP_LAST_WORD_MASK(start + nbits);
> + map[index + 1] |= (value >> space);
> + }
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v8 1/4] bitops: Introduce the for_each_set_clump macro
2020-06-15 12:51 ` [PATCH v8 1/4] bitops: " Syed Nayyar Waris
` (2 preceding siblings ...)
2020-06-16 8:14 ` Andy Shevchenko
@ 2020-06-17 8:00 ` kernel test robot
3 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2020-06-17 8:00 UTC (permalink / raw)
To: Syed Nayyar Waris, linus.walleij, akpm
Cc: kbuild-all, andriy.shevchenko, vilhelm.gray, arnd, linux-arch,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 20402 bytes --]
Hi Syed,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on 444fc5cde64330661bf59944c43844e7d4c2ccd8]
url: https://github.com/0day-ci/linux/commits/Syed-Nayyar-Waris/Introduce-the-for_each_set_clump-macro/20200615-205729
base: 444fc5cde64330661bf59944c43844e7d4c2ccd8
config: x86_64-rhel (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
reproduce (this is a W=1 build):
# save the attached .config to linux build tree
make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>, old ones prefixed by <<):
In file included from include/linux/bits.h:23,
from include/linux/bitops.h:5,
from include/linux/kernel.h:12,
from include/linux/list.h:9,
from include/linux/preempt.h:11,
from include/linux/hardirq.h:5,
from include/linux/kvm_host.h:7,
from arch/x86/kvm/../../../virt/kvm/kvm_main.c:18:
include/linux/bitmap.h: In function 'bitmap_get_value':
>> include/linux/bits.h:26:28: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
include/linux/bitmap.h:590:35: note: in expansion of macro 'GENMASK'
590 | return (map[index] >> offset) & GENMASK(nbits - 1, 0);
| ^~~~~~~
include/linux/bits.h:26:40: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
include/linux/bitmap.h:590:35: note: in expansion of macro 'GENMASK'
590 | return (map[index] >> offset) & GENMASK(nbits - 1, 0);
| ^~~~~~~
include/linux/bitmap.h: In function 'bitmap_set_value':
>> include/linux/bits.h:26:28: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
include/linux/bitmap.h:630:11: note: in expansion of macro 'GENMASK'
630 | value &= GENMASK(nbits - 1, 0);
| ^~~~~~~
include/linux/bits.h:26:40: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
include/linux/bitmap.h:630:11: note: in expansion of macro 'GENMASK'
630 | value &= GENMASK(nbits - 1, 0);
| ^~~~~~~
cc1: all warnings being treated as errors
--
In file included from include/linux/bits.h:23,
from include/linux/bitops.h:5,
from include/linux/kernel.h:12,
from include/linux/list.h:9,
from include/linux/preempt.h:11,
from include/linux/hardirq.h:5,
from include/linux/kvm_host.h:7,
from arch/x86/kvm/../../../virt/kvm/irqchip.c:15:
include/linux/bitmap.h: In function 'bitmap_get_value':
>> include/linux/bits.h:26:28: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
include/linux/bitmap.h:590:35: note: in expansion of macro 'GENMASK'
590 | return (map[index] >> offset) & GENMASK(nbits - 1, 0);
| ^~~~~~~
include/linux/bits.h:26:40: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
include/linux/bitmap.h:590:35: note: in expansion of macro 'GENMASK'
590 | return (map[index] >> offset) & GENMASK(nbits - 1, 0);
| ^~~~~~~
include/linux/bitmap.h: In function 'bitmap_set_value':
>> include/linux/bits.h:26:28: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
include/linux/bitmap.h:630:11: note: in expansion of macro 'GENMASK'
630 | value &= GENMASK(nbits - 1, 0);
| ^~~~~~~
include/linux/bits.h:26:40: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
include/linux/bitmap.h:630:11: note: in expansion of macro 'GENMASK'
630 | value &= GENMASK(nbits - 1, 0);
| ^~~~~~~
arch/x86/kvm/../../../virt/kvm/irqchip.c: At top level:
arch/x86/kvm/../../../virt/kvm/irqchip.c:20:10: fatal error: irq.h: No such file or directory
20 | #include "irq.h"
| ^~~~~~~
cc1: all warnings being treated as errors
compilation terminated.
--
In file included from include/linux/bits.h:23,
from include/linux/bitops.h:5,
from include/linux/kernel.h:12,
from include/linux/list.h:9,
from include/linux/preempt.h:11,
from include/linux/hardirq.h:5,
from include/linux/kvm_host.h:7,
from arch/x86/kvm/mmu/page_track.c:14:
include/linux/bitmap.h: In function 'bitmap_get_value':
>> include/linux/bits.h:26:28: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
include/linux/bitmap.h:590:35: note: in expansion of macro 'GENMASK'
590 | return (map[index] >> offset) & GENMASK(nbits - 1, 0);
| ^~~~~~~
include/linux/bits.h:26:40: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
include/linux/bitmap.h:590:35: note: in expansion of macro 'GENMASK'
590 | return (map[index] >> offset) & GENMASK(nbits - 1, 0);
| ^~~~~~~
include/linux/bitmap.h: In function 'bitmap_set_value':
>> include/linux/bits.h:26:28: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
include/linux/bitmap.h:630:11: note: in expansion of macro 'GENMASK'
630 | value &= GENMASK(nbits - 1, 0);
| ^~~~~~~
include/linux/bits.h:26:40: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
include/linux/bitmap.h:630:11: note: in expansion of macro 'GENMASK'
630 | value &= GENMASK(nbits - 1, 0);
| ^~~~~~~
arch/x86/kvm/mmu/page_track.c: At top level:
arch/x86/kvm/mmu/page_track.c:19:10: fatal error: mmu.h: No such file or directory
19 | #include "mmu.h"
| ^~~~~~~
cc1: all warnings being treated as errors
compilation terminated.
--
In file included from include/linux/bits.h:23,
from include/linux/bitops.h:5,
from include/linux/kernel.h:12,
from include/linux/list.h:9,
from include/linux/wait.h:7,
from include/linux/wait_bit.h:8,
from include/linux/fs.h:6,
from include/linux/highmem.h:5,
from arch/x86/kvm/vmx/vmx.c:17:
include/linux/bitmap.h: In function 'bitmap_get_value':
>> include/linux/bits.h:26:28: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
include/linux/bitmap.h:590:35: note: in expansion of macro 'GENMASK'
590 | return (map[index] >> offset) & GENMASK(nbits - 1, 0);
| ^~~~~~~
include/linux/bits.h:26:40: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
include/linux/bitmap.h:590:35: note: in expansion of macro 'GENMASK'
590 | return (map[index] >> offset) & GENMASK(nbits - 1, 0);
| ^~~~~~~
include/linux/bitmap.h: In function 'bitmap_set_value':
>> include/linux/bits.h:26:28: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
include/linux/bitmap.h:630:11: note: in expansion of macro 'GENMASK'
630 | value &= GENMASK(nbits - 1, 0);
| ^~~~~~~
include/linux/bits.h:26:40: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
include/linux/bitmap.h:630:11: note: in expansion of macro 'GENMASK'
630 | value &= GENMASK(nbits - 1, 0);
| ^~~~~~~
In file included from arch/x86/kvm/vmx/vmx.c:50:
arch/x86/kvm/vmx/capabilities.h: At top level:
arch/x86/kvm/vmx/capabilities.h:7:10: fatal error: lapic.h: No such file or directory
7 | #include "lapic.h"
| ^~~~~~~~~
cc1: all warnings being treated as errors
compilation terminated.
--
In file included from include/linux/bits.h:23,
from include/linux/bitops.h:5,
from include/linux/kernel.h:12,
from include/linux/list.h:9,
from include/linux/preempt.h:11,
from include/linux/hardirq.h:5,
from include/linux/kvm_host.h:7,
from arch/x86/kvm/vmx/pmu_intel.c:12:
include/linux/bitmap.h: In function 'bitmap_get_value':
>> include/linux/bits.h:26:28: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
include/linux/bitmap.h:590:35: note: in expansion of macro 'GENMASK'
590 | return (map[index] >> offset) & GENMASK(nbits - 1, 0);
| ^~~~~~~
include/linux/bits.h:26:40: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
include/linux/bitmap.h:590:35: note: in expansion of macro 'GENMASK'
590 | return (map[index] >> offset) & GENMASK(nbits - 1, 0);
| ^~~~~~~
include/linux/bitmap.h: In function 'bitmap_set_value':
>> include/linux/bits.h:26:28: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
include/linux/bitmap.h:630:11: note: in expansion of macro 'GENMASK'
630 | value &= GENMASK(nbits - 1, 0);
| ^~~~~~~
include/linux/bits.h:26:40: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
26 | __builtin_constant_p((l) > (h)), (l) > (h), 0)))
| ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 | (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
| ^~~~~~~~~~~~~~~~~~~
include/linux/bitmap.h:630:11: note: in expansion of macro 'GENMASK'
630 | value &= GENMASK(nbits - 1, 0);
| ^~~~~~~
arch/x86/kvm/vmx/pmu_intel.c: At top level:
arch/x86/kvm/vmx/pmu_intel.c:15:10: fatal error: x86.h: No such file or directory
15 | #include "x86.h"
| ^~~~~~~
cc1: all warnings being treated as errors
compilation terminated.
..
vim +26 include/linux/bits.h
8bd9cb51daac89 Will Deacon 2018-06-19 15
8bd9cb51daac89 Will Deacon 2018-06-19 16 /*
8bd9cb51daac89 Will Deacon 2018-06-19 17 * Create a contiguous bitmask starting at bit position @l and ending at
8bd9cb51daac89 Will Deacon 2018-06-19 18 * position @h. For example
8bd9cb51daac89 Will Deacon 2018-06-19 19 * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
8bd9cb51daac89 Will Deacon 2018-06-19 20 */
295bcca84916cb Rikard Falkeborn 2020-04-06 21 #if !defined(__ASSEMBLY__) && \
295bcca84916cb Rikard Falkeborn 2020-04-06 22 (!defined(CONFIG_CC_IS_GCC) || CONFIG_GCC_VERSION >= 49000)
295bcca84916cb Rikard Falkeborn 2020-04-06 23 #include <linux/build_bug.h>
295bcca84916cb Rikard Falkeborn 2020-04-06 24 #define GENMASK_INPUT_CHECK(h, l) \
295bcca84916cb Rikard Falkeborn 2020-04-06 25 (BUILD_BUG_ON_ZERO(__builtin_choose_expr( \
295bcca84916cb Rikard Falkeborn 2020-04-06 @26 __builtin_constant_p((l) > (h)), (l) > (h), 0)))
295bcca84916cb Rikard Falkeborn 2020-04-06 27 #else
295bcca84916cb Rikard Falkeborn 2020-04-06 28 /*
295bcca84916cb Rikard Falkeborn 2020-04-06 29 * BUILD_BUG_ON_ZERO is not available in h files included from asm files,
295bcca84916cb Rikard Falkeborn 2020-04-06 30 * disable the input check if that is the case.
295bcca84916cb Rikard Falkeborn 2020-04-06 31 */
295bcca84916cb Rikard Falkeborn 2020-04-06 32 #define GENMASK_INPUT_CHECK(h, l) 0
295bcca84916cb Rikard Falkeborn 2020-04-06 33 #endif
295bcca84916cb Rikard Falkeborn 2020-04-06 34
:::::: The code at line 26 was first introduced by commit
:::::: 295bcca84916cb5079140a89fccb472bb8d1f6e2 linux/bits.h: add compile time sanity check of GENMASK inputs
:::::: TO: Rikard Falkeborn <rikard.falkeborn@gmail.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 44863 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v8 1/4] bitops: Introduce the for_each_set_clump macro
2020-06-16 8:14 ` Andy Shevchenko
@ 2020-06-20 13:45 ` Syed Nayyar Waris
2020-06-20 23:41 ` Syed Nayyar Waris
1 sibling, 0 replies; 10+ messages in thread
From: Syed Nayyar Waris @ 2020-06-20 13:45 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Walleij, Andrew Morton, William Breathitt Gray,
Arnd Bergmann, Linux-Arch, Linux Kernel Mailing List
On Tue, Jun 16, 2020 at 1:44 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, Jun 15, 2020 at 06:21:18PM +0530, Syed Nayyar Waris wrote:
> > This macro iterates for each group of bits (clump) with set bits,
> > within a bitmap memory region. For each iteration, "start" is set to
> > the bit offset of the found clump, while the respective clump value is
> > stored to the location pointed by "clump". Additionally, the
> > bitmap_get_value and bitmap_set_value functions are introduced to
> > respectively get and set a value of n-bits in a bitmap memory region.
> > The n-bits can have any size less than or equal to BITS_PER_LONG.
> > Moreover, during setting value of n-bit in bitmap, if a situation arise
> > that the width of next n-bit is exceeding the word boundary, then it
> > will divide itself such that some portion of it is stored in that word,
> > while the remaining portion is stored in the next higher word. Similar
> > situation occurs while retrieving value of n-bits from bitmap.
>
> On the second view...
>
> > +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 ceiling = roundup(start + 1, BITS_PER_LONG);
>
> This perhaps should use round_up()
Hi Andy. I will see with round_up(). I will check and inform you.
Further below ...
>
> > + const unsigned long space = ceiling - start;
>
> And I think I see a scenario to complain.
>
> If start == 0, then ceiling will be 64.
> space == 64. Not good.
Yes, you are right, when the 'start' is '0', then 'space' will be 64
(on arch where BITS_PER_LONG is 64).
But actually I want this to happen. I need 'space' to hold value 64
when 'start' is '0'. The reason is as follows:
Taking the example of bitmap_set_value(). If the nbits is 16 (as
example) and 'start' is zero, The 'if' condition will be executed
inside bitmap_set_value() when 'start' is zero because space(64) >=
nbits(16) is true. This 'if' condition is for the case when nbits
falls completely into the first word and the nbits doesn't have to
divide itself into another higher word of the bitmap.
This is what I want to happen. I will think more about this and let
you know further.
Kindly let me know If I have misunderstood something. Thanks !
>
> > + unsigned long value_low, value_high;
> > +
> > + if (space >= nbits)
> > + return (map[index] >> offset) & GENMASK(nbits - 1, 0);
> > + else {
> > + 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);
> > + }
> > +}
>
> ...
>
> > +/**
> > + * bitmap_set_value - set n-bit value within a memory region
> > + * @map: address to the bitmap memory region
> > + * @value: value of nbits
> > + * @start: bit offset of the n-bit value
> > + * @nbits: size of value in bits
> > + */
> > +static inline void bitmap_set_value(unsigned long *map,
> > + unsigned long value,
> > + unsigned long start, unsigned long nbits)
> > +{
> > + const size_t index = BIT_WORD(start);
> > + const unsigned long offset = start % BITS_PER_LONG;
>
> > + const unsigned long ceiling = roundup(start + 1, BITS_PER_LONG);
> > + const unsigned long space = ceiling - start;
>
> Ditto for both lines.
>
> > + value &= GENMASK(nbits - 1, 0);
> > +
> > + if (space >= nbits) {
> > + map[index] &= ~(GENMASK(nbits + offset - 1, offset));
> > + map[index] |= value << offset;
> > + } else {
> > + map[index] &= ~BITMAP_FIRST_WORD_MASK(start);
> > + map[index] |= value << offset;
> > + map[index + 1] &= ~BITMAP_LAST_WORD_MASK(start + nbits);
> > + map[index + 1] |= (value >> space);
> > + }
> > +}
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v8 1/4] bitops: Introduce the for_each_set_clump macro
2020-06-16 8:14 ` Andy Shevchenko
2020-06-20 13:45 ` Syed Nayyar Waris
@ 2020-06-20 23:41 ` Syed Nayyar Waris
1 sibling, 0 replies; 10+ messages in thread
From: Syed Nayyar Waris @ 2020-06-20 23:41 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Walleij, Andrew Morton, William Breathitt Gray,
Arnd Bergmann, Linux-Arch, Linux Kernel Mailing List
On Tue, Jun 16, 2020 at 1:44 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, Jun 15, 2020 at 06:21:18PM +0530, Syed Nayyar Waris wrote:
> > This macro iterates for each group of bits (clump) with set bits,
> > within a bitmap memory region. For each iteration, "start" is set to
> > the bit offset of the found clump, while the respective clump value is
> > stored to the location pointed by "clump". Additionally, the
> > bitmap_get_value and bitmap_set_value functions are introduced to
> > respectively get and set a value of n-bits in a bitmap memory region.
> > The n-bits can have any size less than or equal to BITS_PER_LONG.
> > Moreover, during setting value of n-bit in bitmap, if a situation arise
> > that the width of next n-bit is exceeding the word boundary, then it
> > will divide itself such that some portion of it is stored in that word,
> > while the remaining portion is stored in the next higher word. Similar
> > situation occurs while retrieving value of n-bits from bitmap.
>
> On the second view...
>
> > +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 ceiling = roundup(start + 1, BITS_PER_LONG);
>
> This perhaps should use round_up()
I checked with 'round_up'. I am getting the same values as I was
getting with 'roundup'.
I have checked with different clump tests.
Moreover, wherever the 'space' was being evaluated as 64, in the case
of 'roundup', it is also getting evaluated to the same value (of 64),
in case of 'round_up' also.
Further below ...
>
> > + const unsigned long space = ceiling - start;
>
> And I think I see a scenario to complain.
>
> If start == 0, then ceiling will be 64.
> space == 64. Not good.
Yes, you are right, when the 'start' is '0', then 'space' will be 64
(on arch where BITS_PER_LONG is 64).
But actually I want this to happen. I need 'space' to hold value 64
when 'start' is '0'. The reason is as follows:
Taking the example of bitmap_set_value(). If the nbits is 16 (as
example) and 'start' is zero, The 'if' condition will be executed
inside bitmap_set_value() when 'start' is zero because space(64) >=
nbits(16) is true. This 'if' condition is for the case when nbits
falls completely into the first word and the nbits doesn't have to
divide itself into another higher word of the bitmap.
This is what should happen according to me. If space is less than 64,
lets say 63 or 62, then it will not correctly indicate the remaining
space for nbits to fill in (bitmap_set_value) or to extract from
(bitmap_get_value).
Kindly let me know If I have misunderstood something. Thanks !
>
> > + unsigned long value_low, value_high;
> > +
> > + if (space >= nbits)
> > + return (map[index] >> offset) & GENMASK(nbits - 1, 0);
> > + else {
> > + 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);
> > + }
> > +}
>
> ...
>
> > +/**
> > + * bitmap_set_value - set n-bit value within a memory region
> > + * @map: address to the bitmap memory region
> > + * @value: value of nbits
> > + * @start: bit offset of the n-bit value
> > + * @nbits: size of value in bits
> > + */
> > +static inline void bitmap_set_value(unsigned long *map,
> > + unsigned long value,
> > + unsigned long start, unsigned long nbits)
> > +{
> > + const size_t index = BIT_WORD(start);
> > + const unsigned long offset = start % BITS_PER_LONG;
>
> > + const unsigned long ceiling = roundup(start + 1, BITS_PER_LONG);
> > + const unsigned long space = ceiling - start;
>
> Ditto for both lines.
>
> > + value &= GENMASK(nbits - 1, 0);
> > +
> > + if (space >= nbits) {
> > + map[index] &= ~(GENMASK(nbits + offset - 1, offset));
> > + map[index] |= value << offset;
> > + } else {
> > + map[index] &= ~BITMAP_FIRST_WORD_MASK(start);
> > + map[index] |= value << offset;
> > + map[index + 1] &= ~BITMAP_LAST_WORD_MASK(start + nbits);
> > + map[index + 1] |= (value >> space);
> > + }
> > +}
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-06-20 23:41 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-15 12:50 [PATCH v8 0/4] Introduce the for_each_set_clump macro Syed Nayyar Waris
2020-06-15 12:50 ` Syed Nayyar Waris
2020-06-15 12:51 ` [PATCH v8 1/4] bitops: " Syed Nayyar Waris
2020-06-15 13:56 ` Andy Shevchenko
2020-06-15 13:56 ` Andy Shevchenko
2020-06-15 16:47 ` kernel test robot
2020-06-16 8:14 ` Andy Shevchenko
2020-06-20 13:45 ` Syed Nayyar Waris
2020-06-20 23:41 ` Syed Nayyar Waris
2020-06-17 8:00 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).