Introduce the for_each_set_clump macro

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 <[email protected]>

Signed-off-by Syed Nayyar Waris <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

Signed-off-by William Breathitt Gray <[email protected]>

---
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(+)

include/asm-generic/bitops/find.h +19 -0

index 9fdf21302fdf..4e6600759455 100644
@@ -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_ */

include/linux/bitmap.h +15 -0

index 99058eb81042..7ab2c65fc964 100644
@@ -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,

include/linux/bitmap.h +36 -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 */

include/linux/bitops.h +13 -0

index 9acf654f0b19..41c2d9ce63e7 100644
@@ -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;

lib/find_bit.c +14 -0

index 49f875f1baf7..1341bd39b32a 100644
@@ -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);

Signature · 1 line

2.26.2

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.

include/linux/bitmap.h +10 -0

+	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);
+	}
+}

include/linux/bitmap.h +6 -0

+/**
+ * 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

include/linux/bitmap.h +12 -0

+	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);
+	}
+}

Quoted 4 lines · Syed Nayyar Waris

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

Quoted 8 lines

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...

Quoted 6 lines

include/linux/bitmap.h +6 -0

+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;

Quoted 1 line

+ const unsigned long ceiling = roundup(start + 1, BITS_PER_LONG);

This perhaps should use round_up()

Quoted 2 lines

+ 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.

Quoted 11 lines

include/linux/bitmap.h +10 -0

+     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);
+     }
+}

...

Quoted 6 lines

include/linux/bitmap.h +6 -0

+/**
+ * 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

include/linux/bitmap.h static inline void bitmap_set_value8(unsigned long *map, unsigned long value,

Quoted 10 lines

include/linux/bitmap.h +9 -0

+ */
+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.

include/linux/bitmap.h static inline void bitmap_set_value8(unsigned long *map, unsigned long value, · line 641

Quoted 12 lines

include/linux/bitmap.h +12 -0

+     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

Quoted 4 lines · Syed Nayyar Waris

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

Quoted 8 lines

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...

Quoted 6 lines

include/linux/bitmap.h +6 -0

+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;

Quoted 1 line

+ const unsigned long ceiling = roundup(start + 1, BITS_PER_LONG);

This perhaps should use round_up()

Quoted 2 lines

+ 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.

Quoted 11 lines

include/linux/bitmap.h +10 -0

+     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);
+     }
+}

...

Quoted 6 lines

include/linux/bitmap.h +6 -0

+/**
+ * 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

include/linux/bitmap.h static inline void bitmap_set_value8(unsigned long *map, unsigned long value,

Quoted 10 lines

include/linux/bitmap.h +9 -0

+ */
+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.

include/linux/bitmap.h static inline void bitmap_set_value8(unsigned long *map, unsigned long value, · line 641

Quoted 12 lines

include/linux/bitmap.h +12 -0

+     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

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: sparc64-randconfig-s032-20200615 (attached as .config)
compiler: sparc64-linux-gcc (GCC) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.2-rc1-3-g55607964-dirty
        # save the attached .config to linux build tree
        make W=1 C=1 ARCH=sparc64 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag as appropriate

Reported-by kernel test robot <[email protected]>

sparse warnings: (new ones prefixed by >>)

Quoted 2 lines

Quoted 2 lines

include/linux/bitmap.h:639:45: sparse: sparse: shift too big (64) for type unsigned long include/linux/bitmap.h:639:45: sparse: sparse: shift too big (64) for type unsigned long

include/linux/bitmap.h:594:63: sparse: sparse: shift too big (64) for type unsigned long

Quoted 2 lines

Quoted 2 lines

include/linux/bitmap.h:639:45: sparse: sparse: shift too big (64) for type unsigned long include/linux/bitmap.h:638:17: sparse: sparse: invalid access past the end of 'old' (8 8)

vim +639 include/linux/bitmap.h

169c474fb22d8a William Breathitt Gray 2019-12-04  613
803024b6c8a375 Syed Nayyar Waris      2020-06-15  614  /**
803024b6c8a375 Syed Nayyar Waris      2020-06-15  615   * bitmap_set_value - set n-bit value within a memory region
803024b6c8a375 Syed Nayyar Waris      2020-06-15  616   * @map: address to the bitmap memory region
803024b6c8a375 Syed Nayyar Waris      2020-06-15  617   * @value: value of nbits
803024b6c8a375 Syed Nayyar Waris      2020-06-15  618   * @start: bit offset of the n-bit value
803024b6c8a375 Syed Nayyar Waris      2020-06-15  619   * @nbits: size of value in bits
803024b6c8a375 Syed Nayyar Waris      2020-06-15  620   */
803024b6c8a375 Syed Nayyar Waris      2020-06-15  621  static inline void bitmap_set_value(unsigned long *map,
803024b6c8a375 Syed Nayyar Waris      2020-06-15  622                               unsigned long value,
803024b6c8a375 Syed Nayyar Waris      2020-06-15  623                               unsigned long start, unsigned long nbits)
803024b6c8a375 Syed Nayyar Waris      2020-06-15  624  {
803024b6c8a375 Syed Nayyar Waris      2020-06-15  625   const size_t index = BIT_WORD(start);
803024b6c8a375 Syed Nayyar Waris      2020-06-15  626   const unsigned long offset = start % BITS_PER_LONG;
803024b6c8a375 Syed Nayyar Waris      2020-06-15  627   const unsigned long ceiling = roundup(start + 1, BITS_PER_LONG);
803024b6c8a375 Syed Nayyar Waris      2020-06-15  628   const unsigned long space = ceiling - start;
803024b6c8a375 Syed Nayyar Waris      2020-06-15  629
803024b6c8a375 Syed Nayyar Waris      2020-06-15  630   value &= GENMASK(nbits - 1, 0);
803024b6c8a375 Syed Nayyar Waris      2020-06-15  631
803024b6c8a375 Syed Nayyar Waris      2020-06-15  632   if (space >= nbits) {
803024b6c8a375 Syed Nayyar Waris      2020-06-15  633           map[index] &= ~(GENMASK(nbits + offset - 1, offset));
803024b6c8a375 Syed Nayyar Waris      2020-06-15  634           map[index] |= value << offset;

803024b6c8a375 Syed Nayyar Waris      2020-06-15  635   } else {
803024b6c8a375 Syed Nayyar Waris      2020-06-15  636           map[index] &= ~BITMAP_FIRST_WORD_MASK(start);
803024b6c8a375 Syed Nayyar Waris      2020-06-15  637           map[index] |= value << offset;
803024b6c8a375 Syed Nayyar Waris      2020-06-15 @638           map[index + 1] &= ~BITMAP_LAST_WORD_MASK(start + nbits);
803024b6c8a375 Syed Nayyar Waris      2020-06-15 @639           map[index + 1] |= (value >> space);
803024b6c8a375 Syed Nayyar Waris      2020-06-15  640   }
803024b6c8a375 Syed Nayyar Waris      2020-06-15  641  }
803024b6c8a375 Syed Nayyar Waris      2020-06-15  642

Regarding the compilation warning reported above:

"sparse: shift too big (64) for type unsigned long" at line 639 "sparse: invalid access past the end of 'old' (8 8)" at line 638

Kindly refer to the code above, at these line numbers.

I am in the process of fixing this warning. But what would be the fix? ? At the moment can't think of a code-fix to make the compilation warning disappear (specially at line 639). Can anyone please explain to me the meaning of the compilation warning more deeply?

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: sparc64-randconfig-s032-20200615 (attached as .config)
compiler: sparc64-linux-gcc (GCC) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.2-rc1-3-g55607964-dirty
        # save the attached .config to linux build tree
        make W=1 C=1 ARCH=sparc64 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

sparse warnings: (new ones prefixed by >>)

Quoted 2 lines

Quoted 2 lines

include/linux/bitmap.h:639:45: sparse: sparse: shift too big (64) for type unsigned long include/linux/bitmap.h:639:45: sparse: sparse: shift too big (64) for type unsigned long

include/linux/bitmap.h:594:63: sparse: sparse: shift too big (64) for type unsigned long

Quoted 2 lines

Quoted 2 lines

include/linux/bitmap.h:639:45: sparse: sparse: shift too big (64) for type unsigned long include/linux/bitmap.h:638:17: sparse: sparse: invalid access past the end of 'old' (8 8)

vim +639 include/linux/bitmap.h

169c474fb22d8a William Breathitt Gray 2019-12-04  613
803024b6c8a375 Syed Nayyar Waris      2020-06-15  614  /**
803024b6c8a375 Syed Nayyar Waris      2020-06-15  615   * bitmap_set_value - set n-bit value within a memory region
803024b6c8a375 Syed Nayyar Waris      2020-06-15  616   * @map: address to the bitmap memory region
803024b6c8a375 Syed Nayyar Waris      2020-06-15  617   * @value: value of nbits
803024b6c8a375 Syed Nayyar Waris      2020-06-15  618   * @start: bit offset of the n-bit value
803024b6c8a375 Syed Nayyar Waris      2020-06-15  619   * @nbits: size of value in bits
803024b6c8a375 Syed Nayyar Waris      2020-06-15  620   */
803024b6c8a375 Syed Nayyar Waris      2020-06-15  621  static inline void bitmap_set_value(unsigned long *map,
803024b6c8a375 Syed Nayyar Waris      2020-06-15  622                               unsigned long value,
803024b6c8a375 Syed Nayyar Waris      2020-06-15  623                               unsigned long start, unsigned long nbits)
803024b6c8a375 Syed Nayyar Waris      2020-06-15  624  {
803024b6c8a375 Syed Nayyar Waris      2020-06-15  625   const size_t index = BIT_WORD(start);
803024b6c8a375 Syed Nayyar Waris      2020-06-15  626   const unsigned long offset = start % BITS_PER_LONG;
803024b6c8a375 Syed Nayyar Waris      2020-06-15  627   const unsigned long ceiling = roundup(start + 1, BITS_PER_LONG);
803024b6c8a375 Syed Nayyar Waris      2020-06-15  628   const unsigned long space = ceiling - start;
803024b6c8a375 Syed Nayyar Waris      2020-06-15  629
803024b6c8a375 Syed Nayyar Waris      2020-06-15  630   value &= GENMASK(nbits - 1, 0);
803024b6c8a375 Syed Nayyar Waris      2020-06-15  631
803024b6c8a375 Syed Nayyar Waris      2020-06-15  632   if (space >= nbits) {
803024b6c8a375 Syed Nayyar Waris      2020-06-15  633           map[index] &= ~(GENMASK(nbits + offset - 1, offset));
803024b6c8a375 Syed Nayyar Waris      2020-06-15  634           map[index] |= value << offset;
803024b6c8a375 Syed Nayyar Waris      2020-06-15  635   } else {
803024b6c8a375 Syed Nayyar Waris      2020-06-15  636           map[index] &= ~BITMAP_FIRST_WORD_MASK(start);
803024b6c8a375 Syed Nayyar Waris      2020-06-15  637           map[index] |= value << offset;
803024b6c8a375 Syed Nayyar Waris      2020-06-15 @638           map[index + 1] &= ~BITMAP_LAST_WORD_MASK(start + nbits);
803024b6c8a375 Syed Nayyar Waris      2020-06-15 @639           map[index + 1] |= (value >> space);
803024b6c8a375 Syed Nayyar Waris      2020-06-15  640   }
803024b6c8a375 Syed Nayyar Waris      2020-06-15  641  }
803024b6c8a375 Syed Nayyar Waris      2020-06-15  642

:::::: The code at line 639 was first introduced by commit
:::::: 803024b6c8a375ba9e9e9467595d7d52d4f6a38e bitops: Introduce the for_each_set_clump macro

:::::: TO: Syed Nayyar Waris <[email protected]>

Quoted 7 lines

config: sparc64-randconfig-s032-20200615 (attached as .config)
compiler: sparc64-linux-gcc (GCC) 9.3.0
reproduce:

        # apt-get install sparse
        # sparse version: v0.6.2-rc1-3-g55607964-dirty
        # save the attached .config to linux build tree
        make W=1 C=1 ARCH=sparc64 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

Quoted 7 lines

sparse warnings: (new ones prefixed by >>)

Quoted 2 lines

Quoted 2 lines

include/linux/bitmap.h:639:45: sparse: sparse: shift too big (64) for type unsigned long include/linux/bitmap.h:639:45: sparse: sparse: shift too big (64) for type unsigned long

include/linux/bitmap.h:594:63: sparse: sparse: shift too big (64) for type unsigned long

Quoted 2 lines

Quoted 2 lines

include/linux/bitmap.h:639:45: sparse: sparse: shift too big (64) for type unsigned long include/linux/bitmap.h:638:17: sparse: sparse: invalid access past the end of 'old' (8 8)

It seems to me that to reproduce this warning, I have to use the sparc64 compiler. I have installed 'sparc64-linux-gnu-gcc' on my computer.

I have to specify that this compiler needs to be used for build process. How/ Where do I specify this?

I have downloaded the config.gz (has config file) and placed it at the root of the linux kernel project tree. But the Makefile STILL has 'gcc' as the compiler. When I build, it is the 'gcc' compiler being used and not 'sparc64-linux-gnu-gcc'. I know I can manually change the Makefile to use sparc64 compiler, but I think there must be some more elegant way to do this, perhaps using make menuconfig?

Kindly illuminate as to how shall I reproduce the compiler warning.

Quoted 2 lines

803024b6c8a375 Syed Nayyar Waris      2020-06-15 @638           map[index + 1] &= ~BITMAP_LAST_WORD_MASK(start + nbits);
803024b6c8a375 Syed Nayyar Waris      2020-06-15 @639           map[index + 1] |= (value >> space);

On Fri, Jun 19, 2020 at 10:02 AM Syed Nayyar Waris <[email protected]> wrote:

...

Quoted 7 lines

Quoted 7 lines

config: sparc64-randconfig-s032-20200615 (attached as .config)
compiler: sparc64-linux-gcc (GCC) 9.3.0
reproduce:

        # apt-get install sparse
        # sparse version: v0.6.2-rc1-3-g55607964-dirty
        # save the attached .config to linux build tree
        make W=1 C=1 ARCH=sparc64 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

...

Quoted 5 lines

Quoted 5 lines

sparse warnings: (new ones prefixed by >>)

Quoted 2 lines

Quoted 2 lines

include/linux/bitmap.h:639:45: sparse: sparse: shift too big (64) for type unsigned long include/linux/bitmap.h:639:45: sparse: sparse: shift too big (64) for type unsigned long

include/linux/bitmap.h:594:63: sparse: sparse: shift too big (64) for type unsigned long

Quoted 6 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

include/linux/bitmap.h:639:45: sparse: sparse: shift too big (64) for type unsigned long include/linux/bitmap.h:638:17: sparse: sparse: invalid access past the end of 'old' (8 8)

It seems to me that to reproduce this warning, I have to use the sparc64 compiler. I have installed 'sparc64-linux-gnu-gcc' on my computer.

Sparse is not a compiler.

Quoted 9 lines

I have to specify that this compiler needs to be used for build process. How/ Where do I specify this?

I have downloaded the config.gz (has config file) and placed it at the root of the linux kernel project tree. But the Makefile STILL has 'gcc' as the compiler. When I build, it is the 'gcc' compiler being used and not 'sparc64-linux-gnu-gcc'. I know I can manually change the Makefile to use sparc64 compiler, but I think there must be some more elegant way to do this, perhaps using make menuconfig?

If you wish to run a compilation, download a compiler from [1], and, after adding its bin/ folder to PATH, run
make CROSS_COMPILE=sparc64-linux- ARCH=sparc64 ... # first generate .config

Quoted 4 lines

Kindly illuminate as to how shall I reproduce the compiler warning.

Quoted 2 lines

803024b6c8a375 Syed Nayyar Waris      2020-06-15 @638           map[index + 1] &= ~BITMAP_LAST_WORD_MASK(start + nbits);
803024b6c8a375 Syed Nayyar Waris      2020-06-15 @639           map[index + 1] |= (value >> space);

Hmm... I think I sent a reply [2] where I explained how space can be
64. Do you agree with analysis?

[1]: https://mirrors.edge.kernel.org/pub/tools/crosstool/
[2]: https://lore.kernel.org/lkml/20200616081428.GP2428291(a)smile.fi.intel.com/

This patch reimplements the xgpio_set_multiple function in drivers/gpio/gpio-xilinx.c to use the new for_each_set_clump macro. Instead of looping for each bit in xgpio_set_multiple function, now we can check each channel at a time and save cycles.

Cc Bartosz Golaszewski <[email protected]>

Cc Michal Simek <[email protected]>

Signed-off-by Syed Nayyar Waris <[email protected]>

Signed-off-by William Breathitt Gray <[email protected]>

---
Changes in v8:
- No change.

Changes in v7:
- No change.

Changes in v6:
- No change.

Changes in v5:
- Minor change: Inline values '32' and '64' in code for better code readability.

Changes in v4:
- Minor change: Inline values '32' and '64' in code for better code readability.

Changes in v3:
- No change.

Changes in v2:
- No change.

  drivers/gpio/gpio-xilinx.c | 62 ++++++++++++++++++++------------------
  1 file changed, 32 insertions(+), 30 deletions(-)

drivers/gpio/gpio-xilinx.c +1 -18

index 67f9f82e0db0..e81092dea27e 100644
@@ -136,39 +136,41 @@ static void xgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
  static void xgpio_set_multiple(struct gpio_chip *gc, unsigned long *mask,
  			       unsigned long *bits)
  {
-	unsigned long flags;
+	unsigned long flags[2];
  	struct xgpio_instance *chip = gpiochip_get_data(gc);
-	int index = xgpio_index(chip, 0);
-	int offset, i;
-
-	spin_lock_irqsave(&chip->gpio_lock[index], flags);
-
-	/* Write to GPIO signals */
-	for (i = 0; i < gc->ngpio; i++) {
-		if (*mask == 0)
-			break;
-		/* Once finished with an index write it out to the register */
-		if (index !=  xgpio_index(chip, i)) {
-			xgpio_writereg(chip->regs + XGPIO_DATA_OFFSET +
-				       index * XGPIO_CHANNEL_OFFSET,
-				       chip->gpio_state[index]);
-			spin_unlock_irqrestore(&chip->gpio_lock[index], flags);
-			index =  xgpio_index(chip, i);
-			spin_lock_irqsave(&chip->gpio_lock[index], flags);

drivers/gpio/gpio-xilinx.c +9 -0

+
+	bitmap_set_value(old, state[0], 0, width[0]);
+	bitmap_set_value(old, state[1], width[0], width[1]);
+	bitmap_replace(new, old, bits, mask, gc->ngpio);
+
+	bitmap_set_value(old, state[0], 0, 32);
+	bitmap_set_value(old, state[1], 32, 32);
+	state[0] = bitmap_get_value(new, 0, width[0]);
+	state[1] = bitmap_get_value(new, width[0], width[1]);

drivers/gpio/gpio-xilinx.c +2 -4

-	xgpio_writereg(chip->regs + XGPIO_DATA_OFFSET +
-		       index * XGPIO_CHANNEL_OFFSET, chip->gpio_state[index]);
-
-	spin_unlock_irqrestore(&chip->gpio_lock[index], flags);
+	spin_unlock_irqrestore(&chip->gpio_lock[1], flags[1]);
+	spin_unlock_irqrestore(&chip->gpio_lock[0], flags[0]);
  }
 
  /**