From: Yury Norov <ynorov@caviumnetworks.com>
To: linux-kernel@vger.kernel.org
Cc: Yury Norov <ynorov@caviumnetworks.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Ben Hutchings <ben@decadent.org.uk>,
        David Decotigny <decot@googlers.com>,
        "David S . Miller" <davem@davemloft.net>,
        Geert Uytterhoeven <geert@linux-m68k.org>,
        Matthew Wilcox <mawilcox@microsoft.com>,
        Rasmus Villemoes <linux@rasmusvillemoes.dk>
Subject: [PATCH 2/2] bitmap: replace bitmap_{from,to}_u32array
Date: Thu, 28 Dec 2017 18:00:19 +0300
Message-Id: <20171228150019.27953-2-ynorov@caviumnetworks.com>
In-Reply-To: <20171228150019.27953-1-ynorov@caviumnetworks.com>
References: <20171228150019.27953-1-ynorov@caviumnetworks.com>
MIME-Version: 1.0
Content-Type: text/plain
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Dec 2017 15:00:47.3378 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: f8ad405a-810c-414b-ab2f-08d54e03c788
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 711e4ccf-2e9b-4bcf-a551-4094005b6194
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR0701MB3827
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 16807
Lines: 503

with bitmap_{from,to}_arr32 over the kernel. Additionally to it:
* __check_eq_bitmap() now takes single nbits argument.
* __check_eq_u32_array is not used in new test but may be used in
  future. So I don't remove it here, but annotate as __used.

Tested on arm64 and 32-bit BE mips.

CC: Andrew Morton <akpm@linux-foundation.org>
CC: Ben Hutchings <ben@decadent.org.uk>
CC: David Decotigny <decot@googlers.com>,
CC: David S. Miller <davem@davemloft.net>,
CC: Geert Uytterhoeven <geert@linux-m68k.org>
CC: Matthew Wilcox <mawilcox@microsoft.com>
CC: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
---
 arch/arm64/kernel/perf_event.c |   5 +-
 include/linux/bitmap.h         |  11 +--
 lib/bitmap.c                   |  87 -----------------
 lib/test_bitmap.c              | 206 +++++++----------------------------------
 net/core/ethtool.c             |  53 +++++------
 5 files changed, 55 insertions(+), 307 deletions(-)

diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 3affca3dd96a..75b220ba73a3 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -925,9 +925,8 @@ static void __armv8pmu_probe_pmu(void *info)
 	pmceid[0] = read_sysreg(pmceid0_el0);
 	pmceid[1] = read_sysreg(pmceid1_el0);
 
-	bitmap_from_u32array(cpu_pmu->pmceid_bitmap,
-			     ARMV8_PMUV3_MAX_COMMON_EVENTS, pmceid,
-			     ARRAY_SIZE(pmceid));
+	bitmap_from_arr32(cpu_pmu->pmceid_bitmap,
+			     pmceid, ARMV8_PMUV3_MAX_COMMON_EVENTS);
 }
 
 static int armv8pmu_probe_pmu(struct arm_pmu *cpu_pmu)
diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 748eba1622b9..757af9bc4d36 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -64,8 +64,6 @@
  *  bitmap_find_free_region(bitmap, bits, order)  Find and allocate bit region
  *  bitmap_release_region(bitmap, pos, order)   Free specified bit region
  *  bitmap_allocate_region(bitmap, pos, order)  Allocate specified bit region
- *  bitmap_from_u32array(dst, nbits, buf, nwords)  *dst = *buf (nwords 32b words)
- *  bitmap_to_u32array(buf, nwords, src, nbits) *buf = *dst (nwords 32b words)
  *  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
  *
@@ -176,14 +174,7 @@ extern void bitmap_fold(unsigned long *dst, const unsigned long *orig,
 extern int bitmap_find_free_region(unsigned long *bitmap, unsigned int bits, int order);
 extern void bitmap_release_region(unsigned long *bitmap, unsigned int pos, int order);
 extern int bitmap_allocate_region(unsigned long *bitmap, unsigned int pos, int order);
-extern unsigned int bitmap_from_u32array(unsigned long *bitmap,
-					 unsigned int nbits,
-					 const u32 *buf,
-					 unsigned int nwords);
-extern unsigned int bitmap_to_u32array(u32 *buf,
-				       unsigned int nwords,
-				       const unsigned long *bitmap,
-				       unsigned int nbits);
+
 #ifdef __BIG_ENDIAN
 extern void bitmap_copy_le(unsigned long *dst, const unsigned long *src, unsigned int nbits);
 #else
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 47fe6441562c..9e498c77ed0e 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -1106,93 +1106,6 @@ int bitmap_allocate_region(unsigned long *bitmap, unsigned int pos, int order)
 EXPORT_SYMBOL(bitmap_allocate_region);
 
 /**
- * bitmap_from_u32array - copy the contents of a u32 array of bits to bitmap
- *	@bitmap: array of unsigned longs, the destination bitmap, non NULL
- *	@nbits: number of bits in @bitmap
- *	@buf: array of u32 (in host byte order), the source bitmap, non NULL
- *	@nwords: number of u32 words in @buf
- *
- * copy min(nbits, 32*nwords) bits from @buf to @bitmap, remaining
- * bits between nword and nbits in @bitmap (if any) are cleared. In
- * last word of @bitmap, the bits beyond nbits (if any) are kept
- * unchanged.
- *
- * Return the number of bits effectively copied.
- */
-unsigned int
-bitmap_from_u32array(unsigned long *bitmap, unsigned int nbits,
-		     const u32 *buf, unsigned int nwords)
-{
-	unsigned int dst_idx, src_idx;
-
-	for (src_idx = dst_idx = 0; dst_idx < BITS_TO_LONGS(nbits); ++dst_idx) {
-		unsigned long part = 0;
-
-		if (src_idx < nwords)
-			part = buf[src_idx++];
-
-#if BITS_PER_LONG == 64
-		if (src_idx < nwords)
-			part |= ((unsigned long) buf[src_idx++]) << 32;
-#endif
-
-		if (dst_idx < nbits/BITS_PER_LONG)
-			bitmap[dst_idx] = part;
-		else {
-			unsigned long mask = BITMAP_LAST_WORD_MASK(nbits);
-
-			bitmap[dst_idx] = (bitmap[dst_idx] & ~mask)
-				| (part & mask);
-		}
-	}
-
-	return min_t(unsigned int, nbits, 32*nwords);
-}
-EXPORT_SYMBOL(bitmap_from_u32array);
-
-/**
- * bitmap_to_u32array - copy the contents of bitmap to a u32 array of bits
- *	@buf: array of u32 (in host byte order), the dest bitmap, non NULL
- *	@nwords: number of u32 words in @buf
- *	@bitmap: array of unsigned longs, the source bitmap, non NULL
- *	@nbits: number of bits in @bitmap
- *
- * copy min(nbits, 32*nwords) bits from @bitmap to @buf. Remaining
- * bits after nbits in @buf (if any) are cleared.
- *
- * Return the number of bits effectively copied.
- */
-unsigned int
-bitmap_to_u32array(u32 *buf, unsigned int nwords,
-		   const unsigned long *bitmap, unsigned int nbits)
-{
-	unsigned int dst_idx = 0, src_idx = 0;
-
-	while (dst_idx < nwords) {
-		unsigned long part = 0;
-
-		if (src_idx < BITS_TO_LONGS(nbits)) {
-			part = bitmap[src_idx];
-			if (src_idx >= nbits/BITS_PER_LONG)
-				part &= BITMAP_LAST_WORD_MASK(nbits);
-			src_idx++;
-		}
-
-		buf[dst_idx++] = part & 0xffffffffUL;
-
-#if BITS_PER_LONG == 64
-		if (dst_idx < nwords) {
-			part >>= 32;
-			buf[dst_idx++] = part & 0xffffffffUL;
-		}
-#endif
-	}
-
-	return min_t(unsigned int, nbits, 32*nwords);
-}
-EXPORT_SYMBOL(bitmap_to_u32array);
-
-/**
  * bitmap_copy_le - copy a bitmap, putting the bits into little-endian order.
  * @dst:   destination buffer
  * @src:   bitmap to copy
diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
index aa1f2669bdd5..de7ef2996a07 100644
--- a/lib/test_bitmap.c
+++ b/lib/test_bitmap.c
@@ -23,7 +23,7 @@ __check_eq_uint(const char *srcfile, unsigned int line,
 		const unsigned int exp_uint, unsigned int x)
 {
 	if (exp_uint != x) {
-		pr_warn("[%s:%u] expected %u, got %u\n",
+		pr_err("[%s:%u] expected %u, got %u\n",
 			srcfile, line, exp_uint, x);
 		return false;
 	}
@@ -33,19 +33,13 @@ __check_eq_uint(const char *srcfile, unsigned int line,
 
 static bool __init
 __check_eq_bitmap(const char *srcfile, unsigned int line,
-		  const unsigned long *exp_bmap, unsigned int exp_nbits,
-		  const unsigned long *bmap, unsigned int nbits)
+		  const unsigned long *exp_bmap, const unsigned long *bmap,
+		  unsigned int nbits)
 {
-	if (exp_nbits != nbits) {
-		pr_warn("[%s:%u] bitmap length mismatch: expected %u, got %u\n",
-			srcfile, line, exp_nbits, nbits);
-		return false;
-	}
-
 	if (!bitmap_equal(exp_bmap, bmap, nbits)) {
 		pr_warn("[%s:%u] bitmaps contents differ: expected \"%*pbl\", got \"%*pbl\"\n",
 			srcfile, line,
-			exp_nbits, exp_bmap, nbits, bmap);
+			nbits, exp_bmap, nbits, bmap);
 		return false;
 	}
 	return true;
@@ -69,6 +63,10 @@ __check_eq_pbl(const char *srcfile, unsigned int line,
 static bool __init
 __check_eq_u32_array(const char *srcfile, unsigned int line,
 		     const u32 *exp_arr, unsigned int exp_len,
+		     const u32 *arr, unsigned int len) __used;
+static bool __init
+__check_eq_u32_array(const char *srcfile, unsigned int line,
+		     const u32 *exp_arr, unsigned int exp_len,
 		     const u32 *arr, unsigned int len)
 {
 	if (exp_len != len) {
@@ -255,171 +253,29 @@ static void __init test_bitmap_parselist(void)
 	}
 }
 
-static void __init test_bitmap_u32_array_conversions(void)
+static void __init test_bitmap_arr32(void)
 {
-	DECLARE_BITMAP(bmap1, 1024);
-	DECLARE_BITMAP(bmap2, 1024);
-	u32 exp_arr[32], arr[32];
-	unsigned nbits;
-
-	for (nbits = 0 ; nbits < 257 ; ++nbits) {
-		const unsigned int used_u32s = DIV_ROUND_UP(nbits, 32);
-		unsigned int i, rv;
-
-		bitmap_zero(bmap1, nbits);
-		bitmap_set(bmap1, nbits, 1024 - nbits);  /* garbage */
-
-		memset(arr, 0xff, sizeof(arr));
-		rv = bitmap_to_u32array(arr, used_u32s, bmap1, nbits);
-		expect_eq_uint(nbits, rv);
-
-		memset(exp_arr, 0xff, sizeof(exp_arr));
-		memset(exp_arr, 0, used_u32s*sizeof(*exp_arr));
-		expect_eq_u32_array(exp_arr, 32, arr, 32);
-
-		bitmap_fill(bmap2, 1024);
-		rv = bitmap_from_u32array(bmap2, nbits, arr, used_u32s);
-		expect_eq_uint(nbits, rv);
-		expect_eq_bitmap(bmap1, 1024, bmap2, 1024);
-
-		for (i = 0 ; i < nbits ; ++i) {
-			/*
-			 * test conversion bitmap -> u32[]
-			 */
-
-			bitmap_zero(bmap1, 1024);
-			__set_bit(i, bmap1);
-			bitmap_set(bmap1, nbits, 1024 - nbits);  /* garbage */
-
-			memset(arr, 0xff, sizeof(arr));
-			rv = bitmap_to_u32array(arr, used_u32s, bmap1, nbits);
-			expect_eq_uint(nbits, rv);
-
-			/* 1st used u32 words contain expected bit set, the
-			 * remaining words are left unchanged (0xff)
-			 */
-			memset(exp_arr, 0xff, sizeof(exp_arr));
-			memset(exp_arr, 0, used_u32s*sizeof(*exp_arr));
-			exp_arr[i/32] = (1U<<(i%32));
-			expect_eq_u32_array(exp_arr, 32, arr, 32);
-
-
-			/* same, with longer array to fill
-			 */
-			memset(arr, 0xff, sizeof(arr));
-			rv = bitmap_to_u32array(arr, 32, bmap1, nbits);
-			expect_eq_uint(nbits, rv);
-
-			/* 1st used u32 words contain expected bit set, the
-			 * remaining words are all 0s
-			 */
-			memset(exp_arr, 0, sizeof(exp_arr));
-			exp_arr[i/32] = (1U<<(i%32));
-			expect_eq_u32_array(exp_arr, 32, arr, 32);
-
-			/*
-			 * test conversion u32[] -> bitmap
-			 */
-
-			/* the 1st nbits of bmap2 are identical to
-			 * bmap1, the remaining bits of bmap2 are left
-			 * unchanged (all 1s)
-			 */
-			bitmap_fill(bmap2, 1024);
-			rv = bitmap_from_u32array(bmap2, nbits,
-						  exp_arr, used_u32s);
-			expect_eq_uint(nbits, rv);
-
-			expect_eq_bitmap(bmap1, 1024, bmap2, 1024);
-
-			/* same, with more bits to fill
-			 */
-			memset(arr, 0xff, sizeof(arr));  /* garbage */
-			memset(arr, 0, used_u32s*sizeof(u32));
-			arr[i/32] = (1U<<(i%32));
-
-			bitmap_fill(bmap2, 1024);
-			rv = bitmap_from_u32array(bmap2, 1024, arr, used_u32s);
-			expect_eq_uint(used_u32s*32, rv);
-
-			/* the 1st nbits of bmap2 are identical to
-			 * bmap1, the remaining bits of bmap2 are cleared
-			 */
-			bitmap_zero(bmap1, 1024);
-			__set_bit(i, bmap1);
-			expect_eq_bitmap(bmap1, 1024, bmap2, 1024);
-
-
-			/*
-			 * test short conversion bitmap -> u32[] (1
-			 * word too short)
-			 */
-			if (used_u32s > 1) {
-				bitmap_zero(bmap1, 1024);
-				__set_bit(i, bmap1);
-				bitmap_set(bmap1, nbits,
-					   1024 - nbits);  /* garbage */
-				memset(arr, 0xff, sizeof(arr));
-
-				rv = bitmap_to_u32array(arr, used_u32s - 1,
-							bmap1, nbits);
-				expect_eq_uint((used_u32s - 1)*32, rv);
-
-				/* 1st used u32 words contain expected
-				 * bit set, the remaining words are
-				 * left unchanged (0xff)
-				 */
-				memset(exp_arr, 0xff, sizeof(exp_arr));
-				memset(exp_arr, 0,
-				       (used_u32s-1)*sizeof(*exp_arr));
-				if ((i/32) < (used_u32s - 1))
-					exp_arr[i/32] = (1U<<(i%32));
-				expect_eq_u32_array(exp_arr, 32, arr, 32);
-			}
-
-			/*
-			 * test short conversion u32[] -> bitmap (3
-			 * bits too short)
-			 */
-			if (nbits > 3) {
-				memset(arr, 0xff, sizeof(arr));  /* garbage */
-				memset(arr, 0, used_u32s*sizeof(*arr));
-				arr[i/32] = (1U<<(i%32));
-
-				bitmap_zero(bmap1, 1024);
-				rv = bitmap_from_u32array(bmap1, nbits - 3,
-							  arr, used_u32s);
-				expect_eq_uint(nbits - 3, rv);
-
-				/* we are expecting the bit < nbits -
-				 * 3 (none otherwise), and the rest of
-				 * bmap1 unchanged (0-filled)
-				 */
-				bitmap_zero(bmap2, 1024);
-				if (i < nbits - 3)
-					__set_bit(i, bmap2);
-				expect_eq_bitmap(bmap2, 1024, bmap1, 1024);
-
-				/* do the same with bmap1 initially
-				 * 1-filled
-				 */
-
-				bitmap_fill(bmap1, 1024);
-				rv = bitmap_from_u32array(bmap1, nbits - 3,
-							 arr, used_u32s);
-				expect_eq_uint(nbits - 3, rv);
-
-				/* we are expecting the bit < nbits -
-				 * 3 (none otherwise), and the rest of
-				 * bmap1 unchanged (1-filled)
-				 */
-				bitmap_zero(bmap2, 1024);
-				if (i < nbits - 3)
-					__set_bit(i, bmap2);
-				bitmap_set(bmap2, nbits-3, 1024 - nbits + 3);
-				expect_eq_bitmap(bmap2, 1024, bmap1, 1024);
-			}
-		}
+	unsigned int nbits, next_bit, len = sizeof(exp) * 8;
+	u32 arr[sizeof(exp) / 4];
+	DECLARE_BITMAP(bmap2, len);
+
+	memset(arr, 0xa5, sizeof(arr));
+
+	for (nbits = 0; nbits < len; ++nbits) {
+		bitmap_to_arr32(arr, exp, nbits);
+		bitmap_from_arr32(bmap2, arr, nbits);
+		expect_eq_bitmap(bmap2, exp, nbits);
+
+		next_bit = find_next_bit(bmap2,
+				round_up(nbits, BITS_PER_LONG), nbits);
+		if (next_bit < round_up(nbits, BITS_PER_LONG))
+			pr_err("bitmap_copy_arr32(nbits == %d:"
+				" tail is not safely cleared: %d\n",
+				nbits, next_bit);
+
+		if (nbits < len - 32)
+			expect_eq_uint(arr[DIV_ROUND_UP(nbits, 32)],
+								0xa5a5a5a5);
 	}
 }
 
@@ -454,7 +310,7 @@ static void noinline __init test_mem_optimisations(void)
 static int __init test_bitmap_init(void)
 {
 	test_zero_fill_copy();
-	test_bitmap_u32_array_conversions();
+	test_bitmap_arr32();
 	test_bitmap_parselist();
 	test_mem_optimisations();
 
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index f8fcf450a36e..76f6ae37dd15 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -615,18 +615,15 @@ static int load_link_ksettings_from_user(struct ethtool_link_ksettings *to,
 		return -EFAULT;
 
 	memcpy(&to->base, &link_usettings.base, sizeof(to->base));
-	bitmap_from_u32array(to->link_modes.supported,
-			     __ETHTOOL_LINK_MODE_MASK_NBITS,
-			     link_usettings.link_modes.supported,
-			     __ETHTOOL_LINK_MODE_MASK_NU32);
-	bitmap_from_u32array(to->link_modes.advertising,
-			     __ETHTOOL_LINK_MODE_MASK_NBITS,
-			     link_usettings.link_modes.advertising,
-			     __ETHTOOL_LINK_MODE_MASK_NU32);
-	bitmap_from_u32array(to->link_modes.lp_advertising,
-			     __ETHTOOL_LINK_MODE_MASK_NBITS,
-			     link_usettings.link_modes.lp_advertising,
-			     __ETHTOOL_LINK_MODE_MASK_NU32);
+	bitmap_from_arr32(to->link_modes.supported,
+			  link_usettings.link_modes.supported,
+			  __ETHTOOL_LINK_MODE_MASK_NBITS);
+	bitmap_from_arr32(to->link_modes.advertising,
+			  link_usettings.link_modes.advertising,
+			  __ETHTOOL_LINK_MODE_MASK_NBITS);
+	bitmap_from_arr32(to->link_modes.lp_advertising,
+			  link_usettings.link_modes.lp_advertising,
+			  __ETHTOOL_LINK_MODE_MASK_NBITS);
 
 	return 0;
 }
@@ -642,18 +639,15 @@ store_link_ksettings_for_user(void __user *to,
 	struct ethtool_link_usettings link_usettings;
 
 	memcpy(&link_usettings.base, &from->base, sizeof(link_usettings));
-	bitmap_to_u32array(link_usettings.link_modes.supported,
-			   __ETHTOOL_LINK_MODE_MASK_NU32,
-			   from->link_modes.supported,
-			   __ETHTOOL_LINK_MODE_MASK_NBITS);
-	bitmap_to_u32array(link_usettings.link_modes.advertising,
-			   __ETHTOOL_LINK_MODE_MASK_NU32,
-			   from->link_modes.advertising,
-			   __ETHTOOL_LINK_MODE_MASK_NBITS);
-	bitmap_to_u32array(link_usettings.link_modes.lp_advertising,
-			   __ETHTOOL_LINK_MODE_MASK_NU32,
-			   from->link_modes.lp_advertising,
-			   __ETHTOOL_LINK_MODE_MASK_NBITS);
+	bitmap_to_arr32(link_usettings.link_modes.supported,
+			from->link_modes.supported,
+			__ETHTOOL_LINK_MODE_MASK_NU32);
+	bitmap_to_arr32(link_usettings.link_modes.advertising,
+			from->link_modes.advertising,
+			__ETHTOOL_LINK_MODE_MASK_NU32);
+	bitmap_to_arr32(link_usettings.link_modes.lp_advertising,
+			from->link_modes.lp_advertising,
+			__ETHTOOL_LINK_MODE_MASK_NU32);
 
 	if (copy_to_user(to, &link_usettings, sizeof(link_usettings)))
 		return -EFAULT;
@@ -2359,10 +2353,8 @@ static int ethtool_get_per_queue_coalesce(struct net_device *dev,
 
 	useraddr += sizeof(*per_queue_opt);
 
-	bitmap_from_u32array(queue_mask,
-			     MAX_NUM_QUEUE,
-			     per_queue_opt->queue_mask,
-			     DIV_ROUND_UP(MAX_NUM_QUEUE, 32));
+	bitmap_from_arr32(queue_mask, per_queue_opt->queue_mask,
+			  MAX_NUM_QUEUE);
 
 	for_each_set_bit(bit, queue_mask, MAX_NUM_QUEUE) {
 		struct ethtool_coalesce coalesce = { .cmd = ETHTOOL_GCOALESCE };
@@ -2394,10 +2386,7 @@ static int ethtool_set_per_queue_coalesce(struct net_device *dev,
 
 	useraddr += sizeof(*per_queue_opt);
 
-	bitmap_from_u32array(queue_mask,
-			     MAX_NUM_QUEUE,
-			     per_queue_opt->queue_mask,
-			     DIV_ROUND_UP(MAX_NUM_QUEUE, 32));
+	bitmap_from_arr32(queue_mask, per_queue_opt->queue_mask, MAX_NUM_QUEUE);
 	n_queue = bitmap_weight(queue_mask, MAX_NUM_QUEUE);
 	tmp = backup = kmalloc_array(n_queue, sizeof(*backup), GFP_KERNEL);
 	if (!backup)
-- 
2.11.0