bitmap: optimize bitmap_weight() usage

Sent: 10 February 2022 22:49

Mellanox driver has an open-coded for_each_set_bit(). Fix it.

Signed-off-by Yury Norov <[email protected]>

Reviewed-by Tariq Toukan <[email protected]>

---
 drivers/net/ethernet/mellanox/mlx4/cmd.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

drivers/net/ethernet/mellanox/mlx4/cmd.c +1 -1

index e10b7b04b894..c56d2194cbfc 100644
@@ -1994,21 +1994,16 @@ static void mlx4_allocate_port_vpps(struct mlx4_dev *dev, int port)
 
 static int mlx4_master_activate_admin_state(struct mlx4_priv *priv, int slave)
 {
-	int port, err;
+	int p, port, err;
 	struct mlx4_vport_state *vp_admin;
 	struct mlx4_vport_oper_state *vp_oper;
 	struct mlx4_slave_state *slave_state =
 		&priv->mfunc.master.slave_state[slave];
 	struct mlx4_active_ports actv_ports = mlx4_get_active_ports(
 			&priv->dev, slave);

iio_simple_dummy_trigger_h() is mostly an opencoded for_each_set_bit(). Using for_each_set_bit() make code much cleaner, and more effective.

Signed-off-by Yury Norov <[email protected]>

---
 drivers/iio/dummy/iio_simple_dummy_buffer.c | 48 ++++++++-------------
 1 file changed, 19 insertions(+), 29 deletions(-)

drivers/iio/dummy/iio_simple_dummy_buffer.c +19 -29

index d81c2b2dad82..3bc1b7529e2a 100644
@@ -45,41 +45,31 @@ static irqreturn_t iio_simple_dummy_trigger_h(int irq, void *p)
 {
 	struct iio_poll_func *pf = p;
 	struct iio_dev *indio_dev = pf->indio_dev;
+	int i = 0, j;
 	u16 *data;
 
 	data = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
 	if (!data)
 		goto done;
 
-	if (!bitmap_empty(indio_dev->active_scan_mask, indio_dev->masklength)) {
-		/*
-		 * Three common options here:
-		 * hardware scans: certain combinations of channels make
-		 *   up a fast read.  The capture will consist of all of them.
-		 *   Hence we just call the grab data function and fill the
-		 *   buffer without processing.
-		 * software scans: can be considered to be random access
-		 *   so efficient reading is just a case of minimal bus
-		 *   transactions.
-		 * software culled hardware scans:
-		 *   occasionally a driver may process the nearest hardware
-		 *   scan to avoid storing elements that are not desired. This
-		 *   is the fiddliest option by far.
-		 * Here let's pretend we have random access. And the values are
-		 * in the constant table fakedata.
-		 */
-		int i, j;
-
-		for (i = 0, j = 0;
-		     i < bitmap_weight(indio_dev->active_scan_mask,
-				       indio_dev->masklength);
-		     i++, j++) {
-			j = find_next_bit(indio_dev->active_scan_mask,
-					  indio_dev->masklength, j);
-			/* random access read from the 'device' */
-			data[i] = fakedata[j];
-		}
-	}
+	/*
+	 * Three common options here:
+	 * hardware scans: certain combinations of channels make
+	 *   up a fast read.  The capture will consist of all of them.
+	 *   Hence we just call the grab data function and fill the
+	 *   buffer without processing.
+	 * software scans: can be considered to be random access
+	 *   so efficient reading is just a case of minimal bus
+	 *   transactions.
+	 * software culled hardware scans:
+	 *   occasionally a driver may process the nearest hardware
+	 *   scan to avoid storing elements that are not desired. This
+	 *   is the fiddliest option by far.
+	 * Here let's pretend we have random access. And the values are
+	 * in the constant table fakedata.
+	 */
+	for_each_set_bit(j, indio_dev->active_scan_mask, indio_dev->masklength)
+		data[i++] = fakedata[j];
 
 	iio_push_to_buffers_with_timestamp(indio_dev, data,
 					   iio_get_time_ns(indio_dev));

Signature · 1 line

2.32.0

iio_simple_dummy_trigger_h() is mostly an opencoded for_each_set_bit(). Using for_each_set_bit() make code much cleaner, and more effective.

Signed-off-by Yury Norov <[email protected]>

---
  drivers/iio/dummy/iio_simple_dummy_buffer.c | 48 ++++++++-------------
  1 file changed, 19 insertions(+), 29 deletions(-)

drivers/iio/dummy/iio_simple_dummy_buffer.c +7 -29

index d81c2b2dad82..3bc1b7529e2a 100644
@@ -45,41 +45,31 @@ static irqreturn_t iio_simple_dummy_trigger_h(int irq, void *p)
  {
  	struct iio_poll_func *pf = p;
  	struct iio_dev *indio_dev = pf->indio_dev;
+	int i = 0, j;
  	u16 *data;
 
  	data = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
  	if (!data)
  		goto done;
 
-	if (!bitmap_empty(indio_dev->active_scan_mask, indio_dev->masklength)) {
-		/*
-		 * Three common options here:
-		 * hardware scans: certain combinations of channels make
-		 *   up a fast read.  The capture will consist of all of them.
-		 *   Hence we just call the grab data function and fill the
-		 *   buffer without processing.
-		 * software scans: can be considered to be random access
-		 *   so efficient reading is just a case of minimal bus
-		 *   transactions.
-		 * software culled hardware scans:
-		 *   occasionally a driver may process the nearest hardware
-		 *   scan to avoid storing elements that are not desired. This
-		 *   is the fiddliest option by far.
-		 * Here let's pretend we have random access. And the values are
-		 * in the constant table fakedata.
-		 */
-		int i, j;
-
-		for (i = 0, j = 0;
-		     i < bitmap_weight(indio_dev->active_scan_mask,
-				       indio_dev->masklength);
-		     i++, j++) {
-			j = find_next_bit(indio_dev->active_scan_mask,
-					  indio_dev->masklength, j);
-			/* random access read from the 'device' */
-			data[i] = fakedata[j];
-		}
-	}
+	/*
+	 * Three common options here:
+	 * hardware scans: certain combinations of channels make
+	 *   up a fast read.  The capture will consist of all of them.
+	 *   Hence we just call the grab data function and fill the
+	 *   buffer without processing.

drivers/iio/dummy/iio_simple_dummy_buffer.c +2 -0

+	for_each_set_bit(j, indio_dev->active_scan_mask, indio_dev->masklength)
+		data[i++] = fakedata[j];
 
  	iio_push_to_buffers_with_timestamp(indio_dev, data,
  					   iio_get_time_ns(indio_dev));

Le 10/02/2022 à 23:48, Yury Norov a écrit :

Quoted 63 lines

iio_simple_dummy_trigger_h() is mostly an opencoded for_each_set_bit(). Using for_each_set_bit() make code much cleaner, and more effective.

Signed-off-by Yury Norov <[email protected]>

---
  drivers/iio/dummy/iio_simple_dummy_buffer.c | 48 ++++++++-------------
  1 file changed, 19 insertions(+), 29 deletions(-)

drivers/iio/dummy/iio_simple_dummy_buffer.c +12 -29

index d81c2b2dad82..3bc1b7529e2a 100644
@@ -45,41 +45,31 @@ static irqreturn_t iio_simple_dummy_trigger_h(int irq, void *p)
  {
  	struct iio_poll_func *pf = p;
  	struct iio_dev *indio_dev = pf->indio_dev;
+	int i = 0, j;
  	u16 *data;
 
  	data = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
  	if (!data)
  		goto done;
 
-	if (!bitmap_empty(indio_dev->active_scan_mask, indio_dev->masklength)) {
-		/*
-		 * Three common options here:
-		 * hardware scans: certain combinations of channels make
-		 *   up a fast read.  The capture will consist of all of them.
-		 *   Hence we just call the grab data function and fill the
-		 *   buffer without processing.
-		 * software scans: can be considered to be random access
-		 *   so efficient reading is just a case of minimal bus
-		 *   transactions.
-		 * software culled hardware scans:
-		 *   occasionally a driver may process the nearest hardware
-		 *   scan to avoid storing elements that are not desired. This
-		 *   is the fiddliest option by far.
-		 * Here let's pretend we have random access. And the values are
-		 * in the constant table fakedata.
-		 */
-		int i, j;
-
-		for (i = 0, j = 0;
-		     i < bitmap_weight(indio_dev->active_scan_mask,
-				       indio_dev->masklength);
-		     i++, j++) {
-			j = find_next_bit(indio_dev->active_scan_mask,
-					  indio_dev->masklength, j);
-			/* random access read from the 'device' */
-			data[i] = fakedata[j];
-		}
-	}
+	/*
+	 * Three common options here:
+	 * hardware scans: certain combinations of channels make
+	 *   up a fast read.  The capture will consist of all of them.
+	 *   Hence we just call the grab data function and fill the
+	 *   buffer without processing.
+	 * software scans: can be considered to be random access
+	 *   so efficient reading is just a case of minimal bus
+	 *   transactions.
+	 * software culled hardware scans:
+	 *   occasionally a driver may process the nearest hardware

drivers/iio/dummy/iio_simple_dummy_buffer.c static irqreturn_t iio_simple_dummy_trigger_h(int irq, void *p) · line 70

Quoted 5 lines

drivers/iio/dummy/iio_simple_dummy_buffer.c +5 -0

+	 *   scan to avoid storing elements that are not desired. This
+	 *   is the fiddliest option by far.
+	 * Here let's pretend we have random access. And the values are
+	 * in the constant table fakedata.
+	 */  

Nitpicking: you could take advantage of the tab you save to use the full width of the line and save some lines of code.

Just my 2c.

CJ

Quoted 5 lines

drivers/iio/dummy/iio_simple_dummy_buffer.c +2 -0

+	for_each_set_bit(j, indio_dev->active_scan_mask, indio_dev->masklength)
+		data[i++] = fakedata[j];
 
  	iio_push_to_buffers_with_timestamp(indio_dev, data,
  					   iio_get_time_ns(indio_dev));  

Quoted 64 lines · On Fri, 11 Feb 2022 18:17:37 +0100 Christophe JAILLET…

Le 10/02/2022 à 23:48, Yury Norov a écrit :

Quoted 63 lines

iio_simple_dummy_trigger_h() is mostly an opencoded for_each_set_bit(). Using for_each_set_bit() make code much cleaner, and more effective.

Signed-off-by Yury Norov <[email protected]>

---
  drivers/iio/dummy/iio_simple_dummy_buffer.c | 48 ++++++++-------------
  1 file changed, 19 insertions(+), 29 deletions(-)

drivers/iio/dummy/iio_simple_dummy_buffer.c +12 -29

index d81c2b2dad82..3bc1b7529e2a 100644
@@ -45,41 +45,31 @@ static irqreturn_t iio_simple_dummy_trigger_h(int irq, void *p)
  {
  	struct iio_poll_func *pf = p;
  	struct iio_dev *indio_dev = pf->indio_dev;
+	int i = 0, j;
  	u16 *data;
 
  	data = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
  	if (!data)
  		goto done;
 
-	if (!bitmap_empty(indio_dev->active_scan_mask, indio_dev->masklength)) {
-		/*
-		 * Three common options here:
-		 * hardware scans: certain combinations of channels make
-		 *   up a fast read.  The capture will consist of all of them.
-		 *   Hence we just call the grab data function and fill the
-		 *   buffer without processing.
-		 * software scans: can be considered to be random access
-		 *   so efficient reading is just a case of minimal bus
-		 *   transactions.
-		 * software culled hardware scans:
-		 *   occasionally a driver may process the nearest hardware
-		 *   scan to avoid storing elements that are not desired. This
-		 *   is the fiddliest option by far.
-		 * Here let's pretend we have random access. And the values are
-		 * in the constant table fakedata.
-		 */
-		int i, j;
-
-		for (i = 0, j = 0;
-		     i < bitmap_weight(indio_dev->active_scan_mask,
-				       indio_dev->masklength);
-		     i++, j++) {
-			j = find_next_bit(indio_dev->active_scan_mask,
-					  indio_dev->masklength, j);
-			/* random access read from the 'device' */
-			data[i] = fakedata[j];
-		}
-	}
+	/*
+	 * Three common options here:
+	 * hardware scans: certain combinations of channels make
+	 *   up a fast read.  The capture will consist of all of them.
+	 *   Hence we just call the grab data function and fill the
+	 *   buffer without processing.
+	 * software scans: can be considered to be random access
+	 *   so efficient reading is just a case of minimal bus
+	 *   transactions.
+	 * software culled hardware scans:
+	 *   occasionally a driver may process the nearest hardware

Quoted 8 lines

drivers/iio/dummy/iio_simple_dummy_buffer.c static irqreturn_t iio_simple_dummy_trigger_h(int irq, void *p) · line 70

Quoted 5 lines

drivers/iio/dummy/iio_simple_dummy_buffer.c +5 -0

+	 *   scan to avoid storing elements that are not desired. This
+	 *   is the fiddliest option by far.
+	 * Here let's pretend we have random access. And the values are
+	 * in the constant table fakedata.
+	 */    

Nitpicking: you could take advantage of the tab you save to use the full width of the line and save some lines of code.

Tweaked whilst applying.

Sorry this one took so long. I marked it as a patch that I'd revisit if and tidy up if there was no v2 sent, but then managed to forget about it until I came to do a clean out of patchwork today.

Anyhow, now applied to the togreg branch of iio.git - initially pushed out as testing for 0-day to see if we missed anything.

Thanks,

Jonathan

Quoted 12 lines

Just my 2c.

CJ

Quoted 5 lines

drivers/iio/dummy/iio_simple_dummy_buffer.c +2 -0

+	for_each_set_bit(j, indio_dev->active_scan_mask, indio_dev->masklength)
+		data[i++] = fakedata[j];
 
  	iio_push_to_buffers_with_timestamp(indio_dev, data,
  					   iio_get_time_ns(indio_dev));    

qed_rdma_bmap_free() is mostly an opencoded version of printk("%*pb"). Using %*pb format simplifies the code, and helps to avoid inefficient usage of bitmap_weight().

While here, reorganize logic to avoid calculating bmap weight if check is false.

Signed-off-by Yury Norov <[email protected]>

---

This is RFC because it changes lines printing format to bitmap %*pb. If it hurts userspace, it's better to drop the patch.

 drivers/net/ethernet/qlogic/qed/qed_rdma.c | 45 +++++++---------------
 1 file changed, 14 insertions(+), 31 deletions(-)

drivers/net/ethernet/qlogic/qed/qed_rdma.c +14 -31

index 23b668de4640..f4c04af9d4dd 100644
@@ -319,44 +319,27 @@ static int qed_rdma_alloc(struct qed_hwfn *p_hwfn)
 void qed_rdma_bmap_free(struct qed_hwfn *p_hwfn,
 			struct qed_bmap *bmap, bool check)
 {
-	int weight = bitmap_weight(bmap->bitmap, bmap->max_count);
-	int last_line = bmap->max_count / (64 * 8);
-	int last_item = last_line * 8 +
-	    DIV_ROUND_UP(bmap->max_count % (64 * 8), 64);
-	u64 *pmap = (u64 *)bmap->bitmap;
-	int line, item, offset;
-	u8 str_last_line[200] = { 0 };
-
-	if (!weight || !check)
+	unsigned int bit, weight, nbits;
+	unsigned long *b;
+
+	if (!check)
+		goto end;
+
+	weight = bitmap_weight(bmap->bitmap, bmap->max_count);
+	if (!weight)
 		goto end;
 
 	DP_NOTICE(p_hwfn,
 		  "%s bitmap not free - size=%d, weight=%d, 512 bits per line\n",
 		  bmap->name, bmap->max_count, weight);
 
-	/* print aligned non-zero lines, if any */
-	for (item = 0, line = 0; line < last_line; line++, item += 8)
-		if (bitmap_weight((unsigned long *)&pmap[item], 64 * 8))
+	for (bit = 0; bit < bmap->max_count; bit += 512) {
+		b =  bmap->bitmap + BITS_TO_LONGS(bit);
+		nbits = min(bmap->max_count - bit, 512);
+
+		if (!bitmap_empty(b, nbits))
 			DP_NOTICE(p_hwfn,
-				  "line 0x%04x: 0x%016llx 0x%016llx 0x%016llx 0x%016llx 0x%016llx 0x%016llx 0x%016llx 0x%016llx\n",
-				  line,
-				  pmap[item],
-				  pmap[item + 1],
-				  pmap[item + 2],
-				  pmap[item + 3],
-				  pmap[item + 4],
-				  pmap[item + 5],
-				  pmap[item + 6], pmap[item + 7]);
-
-	/* print last unaligned non-zero line, if any */
-	if ((bmap->max_count % (64 * 8)) &&
-	    (bitmap_weight((unsigned long *)&pmap[item],
-			   bmap->max_count - item * 64))) {
-		offset = sprintf(str_last_line, "line 0x%04x: ", line);
-		for (; item < last_item; item++)
-			offset += sprintf(str_last_line + offset,
-					  "0x%016llx ", pmap[item]);
-		DP_NOTICE(p_hwfn, "%s\n", str_last_line);
+				  "line 0x%04x: %*pb\n", bit / 512, nbits, b);
 	}
 
 end:

Signature · 1 line

2.32.0

In some places kvm/hyperv.c code calls bitmap_weight() to check if any bit of a given bitmap is set. It's better to use bitmap_empty() in that case because bitmap_empty() stops traversing the bitmap as soon as it finds first set bit, while bitmap_weight() counts all bits unconditionally.

Signed-off-by Yury Norov <[email protected]>

Reviewed-by Vitaly Kuznetsov <[email protected]>

---

In some places kvm/hyperv.c code calls bitmap_weight() to check if any bit of a given bitmap is set. It's better to use bitmap_empty() in that case because bitmap_empty() stops traversing the bitmap as soon as it finds first set bit, while bitmap_weight() counts all bits unconditionally.

Signed-off-by Yury Norov <[email protected]>

Reviewed-by Vitaly Kuznetsov <[email protected]>

---
  arch/x86/kvm/hyperv.c | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)

arch/x86/kvm/hyperv.c +1 -1

index 6e38a7d22e97..06c2a5603123 100644
@@ -90,7 +90,7 @@ static void synic_update_vector(struct kvm_vcpu_hv_synic *synic,
  {
  	struct kvm_vcpu *vcpu = hv_synic_to_vcpu(synic);
  	struct kvm_hv *hv = to_kvm_hv(vcpu->kvm);
-	int auto_eoi_old, auto_eoi_new;
+	bool auto_eoi_old, auto_eoi_new;
 
  	if (vector < HV_SYNIC_FIRST_VALID_VECTOR)
  		return;

arch/x86/kvm/hyperv.c +0 -0

  		return;
 
  	down_write(&vcpu->kvm->arch.apicv_update_lock);

Le 10/02/2022 à 23:48, Yury Norov a écrit :

Quoted 24 lines

In some places kvm/hyperv.c code calls bitmap_weight() to check if any bit of a given bitmap is set. It's better to use bitmap_empty() in that case because bitmap_empty() stops traversing the bitmap as soon as it finds first set bit, while bitmap_weight() counts all bits unconditionally.

Signed-off-by Yury Norov <[email protected]>

Reviewed-by Vitaly Kuznetsov <[email protected]>

---
  arch/x86/kvm/hyperv.c | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)

arch/x86/kvm/hyperv.c +1 -1

index 6e38a7d22e97..06c2a5603123 100644
@@ -90,7 +90,7 @@ static void synic_update_vector(struct kvm_vcpu_hv_synic *synic,
  {
  	struct kvm_vcpu *vcpu = hv_synic_to_vcpu(synic);
  	struct kvm_hv *hv = to_kvm_hv(vcpu->kvm);
-	int auto_eoi_old, auto_eoi_new;
+	bool auto_eoi_old, auto_eoi_new;
  	if (vector < HV_SYNIC_FIRST_VALID_VECTOR)
  		return;

arch/x86/kvm/hyperv.c static void synic_update_vector(struct kvm_vcpu_hv_synic *synic, · line 102

Quoted 4 lines

arch/x86/kvm/hyperv.c +1 -1

@@ -100,16 +100,16 @@ static void synic_update_vector(struct kvm_vcpu_hv_synic *synic,
  	else
  		__clear_bit(vector, synic->vec_bitmap);
-	auto_eoi_old = bitmap_weight(synic->auto_eoi_bitmap, 256);
+	auto_eoi_old = !bitmap_empty(synic->auto_eoi_bitmap, 256);

I think that you can also remove the "!" here, ...

arch/x86/kvm/hyperv.c static void synic_update_vector(struct kvm_vcpu_hv_synic *synic, · line 110

Quoted 6 lines

arch/x86/kvm/hyperv.c +1 -1

  	if (synic_has_vector_auto_eoi(synic, vector))
  		__set_bit(vector, synic->auto_eoi_bitmap);
  	else
  		__clear_bit(vector, synic->auto_eoi_bitmap);
-	auto_eoi_new = bitmap_weight(synic->auto_eoi_bitmap, 256);
+	auto_eoi_new = !bitmap_empty(synic->auto_eoi_bitmap, 256);

... and there...

arch/x86/kvm/hyperv.c static void synic_update_vector(struct kvm_vcpu_hv_synic *synic, · line 112

Quoted 2 lines

arch/x86/kvm/hyperv.c +1 -1

-	if (!!auto_eoi_old == !!auto_eoi_new)
+	if (auto_eoi_old == auto_eoi_new)

... because this test would still give the same result.

Quoted 41 lines · Christophe JAILLET

Le 10/02/2022 à 23:48, Yury Norov a écrit :

Quoted 24 lines

In some places kvm/hyperv.c code calls bitmap_weight() to check if any bit of a given bitmap is set. It's better to use bitmap_empty() in that case because bitmap_empty() stops traversing the bitmap as soon as it finds first set bit, while bitmap_weight() counts all bits unconditionally.

Signed-off-by Yury Norov <[email protected]>

Reviewed-by Vitaly Kuznetsov <[email protected]>

---
  arch/x86/kvm/hyperv.c | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)

arch/x86/kvm/hyperv.c +1 -1

index 6e38a7d22e97..06c2a5603123 100644
@@ -90,7 +90,7 @@ static void synic_update_vector(struct kvm_vcpu_hv_synic *synic,
  {
  	struct kvm_vcpu *vcpu = hv_synic_to_vcpu(synic);
  	struct kvm_hv *hv = to_kvm_hv(vcpu->kvm);
-	int auto_eoi_old, auto_eoi_new;
+	bool auto_eoi_old, auto_eoi_new;
  	if (vector < HV_SYNIC_FIRST_VALID_VECTOR)
  		return;

arch/x86/kvm/hyperv.c static void synic_update_vector(struct kvm_vcpu_hv_synic *synic, · line 102

Quoted 4 lines

arch/x86/kvm/hyperv.c +1 -1

@@ -100,16 +100,16 @@ static void synic_update_vector(struct kvm_vcpu_hv_synic *synic,
  	else
  		__clear_bit(vector, synic->vec_bitmap);
-	auto_eoi_old = bitmap_weight(synic->auto_eoi_bitmap, 256);
+	auto_eoi_old = !bitmap_empty(synic->auto_eoi_bitmap, 256);

I think that you can also remove the "!" here, ...

arch/x86/kvm/hyperv.c static void synic_update_vector(struct kvm_vcpu_hv_synic *synic, · line 110

Quoted 6 lines

arch/x86/kvm/hyperv.c +1 -1

  	if (synic_has_vector_auto_eoi(synic, vector))
  		__set_bit(vector, synic->auto_eoi_bitmap);
  	else
  		__clear_bit(vector, synic->auto_eoi_bitmap);
-	auto_eoi_new = bitmap_weight(synic->auto_eoi_bitmap, 256);
+	auto_eoi_new = !bitmap_empty(synic->auto_eoi_bitmap, 256);

... and there...

Quoted 4 lines

arch/x86/kvm/hyperv.c static void synic_update_vector(struct kvm_vcpu_hv_synic *synic, · line 112

Quoted 2 lines

arch/x86/kvm/hyperv.c +1 -1

-	if (!!auto_eoi_old == !!auto_eoi_new)
+	if (auto_eoi_old == auto_eoi_new)

... because this test would still give the same result.

It would give the same result, but the variable names would be inverted as they track if "auto EOI" is being used. So yes, it's technically unnecessary, but also very deliberate.

smp_request_block() in drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c calls bitmap_weight() to check if any bit of a given bitmap is set. It's better to use bitmap_empty() in that case because bitmap_empty() stops traversing the bitmap as soon as it finds first set bit, while bitmap_weight() counts all bits unconditionally.

Signed-off-by Yury Norov <[email protected]>

---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c +0 -0

index d7fa2c49e741..56a3063545ec 100644

drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c +0 -1

        reserved = smp->reserved[cid];
 
--

2.32.0

In some places, drivers/perf code calls bitmap_weight() to check if any bit of a given bitmap is set. It's better to use bitmap_empty() in that case because bitmap_empty() stops traversing the bitmap as soon as it finds first set bit, while bitmap_weight() counts all bits unconditionally.

Signed-off-by Yury Norov <[email protected]>

---
 drivers/perf/arm-cci.c                   | 2 +-
 drivers/perf/arm_pmu.c                   | 4 ++--
 drivers/perf/hisilicon/hisi_uncore_pmu.c | 2 +-
 drivers/perf/xgene_pmu.c                 | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

drivers/perf/arm-cci.c +1 -1

index 54aca3a62814..96e09fa40909 100644
@@ -1096,7 +1096,7 @@ static void cci_pmu_enable(struct pmu *pmu)
 {
 	struct cci_pmu *cci_pmu = to_cci_pmu(pmu);
 	struct cci_pmu_hw_events *hw_events = &cci_pmu->hw_events;
-	int enabled = bitmap_weight(hw_events->used_mask, cci_pmu->num_cntrs);
+	bool enabled = !bitmap_empty(hw_events->used_mask, cci_pmu->num_cntrs);
 	unsigned long flags;
 
 	if (!enabled)

drivers/perf/arm_pmu.c +2 -2

index 295cc7952d0e..a31b302b0ade 100644
@@ -524,7 +524,7 @@ static void armpmu_enable(struct pmu *pmu)
 {
 	struct arm_pmu *armpmu = to_arm_pmu(pmu);
 	struct pmu_hw_events *hw_events = this_cpu_ptr(armpmu->hw_events);
-	int enabled = bitmap_weight(hw_events->used_mask, armpmu->num_events);
+	bool enabled = !bitmap_empty(hw_events->used_mask, armpmu->num_events);
 
 	/* For task-bound events we may be called on other CPUs */
 	if (!cpumask_test_cpu(smp_processor_id(), &armpmu->supported_cpus))

@@ -785,7 +785,7 @@ static int cpu_pm_pmu_notify(struct notifier_block *b, unsigned long cmd,
 {
 	struct arm_pmu *armpmu = container_of(b, struct arm_pmu, cpu_pm_nb);
 	struct pmu_hw_events *hw_events = this_cpu_ptr(armpmu->hw_events);
-	int enabled = bitmap_weight(hw_events->used_mask, armpmu->num_events);
+	bool enabled = !bitmap_empty(hw_events->used_mask, armpmu->num_events);
 
 	if (!cpumask_test_cpu(smp_processor_id(), &armpmu->supported_cpus))
 		return NOTIFY_DONE;

drivers/perf/hisilicon/hisi_uncore_pmu.c +1 -1

index a738aeab5c04..358e4e284a62 100644
@@ -393,7 +393,7 @@ EXPORT_SYMBOL_GPL(hisi_uncore_pmu_read);
 void hisi_uncore_pmu_enable(struct pmu *pmu)
 {
 	struct hisi_pmu *hisi_pmu = to_hisi_pmu(pmu);
-	int enabled = bitmap_weight(hisi_pmu->pmu_events.used_mask,
+	bool enabled = !bitmap_empty(hisi_pmu->pmu_events.used_mask,
 				    hisi_pmu->num_counters);
 
 	if (!enabled)

drivers/perf/xgene_pmu.c +1 -1

index 5283608dc055..0c32dffc7ede 100644
@@ -867,7 +867,7 @@ static void xgene_perf_pmu_enable(struct pmu *pmu)
 {
 	struct xgene_pmu_dev *pmu_dev = to_pmu_dev(pmu);
 	struct xgene_pmu *xgene_pmu = pmu_dev->parent;
-	int enabled = bitmap_weight(pmu_dev->cntr_assign_mask,
+	bool enabled = !bitmap_empty(pmu_dev->cntr_assign_mask,
 			pmu_dev->max_counters);
 
 	if (!enabled)

Signature · 1 line

2.32.0

Hi Yury,

Quoted 6 lines · Yury Norov

In some places, drivers/perf code calls bitmap_weight() to check if any bit of a given bitmap is set. It's better to use bitmap_empty() in that case because bitmap_empty() stops traversing the bitmap as soon as it finds first set bit, while bitmap_weight() counts all bits unconditionally.

Signed-off-by Yury Norov <[email protected]>

This looks like a nice semantic cleanup to me, so FWIW:

Acked-by Mark Rutland <[email protected]>

How are you expecting to queue all of this?

Should Will and I pick this patch?

Thanks,
Mark.

Quoted 71 lines

---
 drivers/perf/arm-cci.c                   | 2 +-
 drivers/perf/arm_pmu.c                   | 4 ++--
 drivers/perf/hisilicon/hisi_uncore_pmu.c | 2 +-
 drivers/perf/xgene_pmu.c                 | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

drivers/perf/arm-cci.c +1 -1

index 54aca3a62814..96e09fa40909 100644
@@ -1096,7 +1096,7 @@ static void cci_pmu_enable(struct pmu *pmu)
 {
 	struct cci_pmu *cci_pmu = to_cci_pmu(pmu);
 	struct cci_pmu_hw_events *hw_events = &cci_pmu->hw_events;
-	int enabled = bitmap_weight(hw_events->used_mask, cci_pmu->num_cntrs);
+	bool enabled = !bitmap_empty(hw_events->used_mask, cci_pmu->num_cntrs);
 	unsigned long flags;
 
 	if (!enabled)

drivers/perf/arm_pmu.c +2 -2

index 295cc7952d0e..a31b302b0ade 100644
@@ -524,7 +524,7 @@ static void armpmu_enable(struct pmu *pmu)
 {
 	struct arm_pmu *armpmu = to_arm_pmu(pmu);
 	struct pmu_hw_events *hw_events = this_cpu_ptr(armpmu->hw_events);
-	int enabled = bitmap_weight(hw_events->used_mask, armpmu->num_events);
+	bool enabled = !bitmap_empty(hw_events->used_mask, armpmu->num_events);
 
 	/* For task-bound events we may be called on other CPUs */
 	if (!cpumask_test_cpu(smp_processor_id(), &armpmu->supported_cpus))

@@ -785,7 +785,7 @@ static int cpu_pm_pmu_notify(struct notifier_block *b, unsigned long cmd,
 {
 	struct arm_pmu *armpmu = container_of(b, struct arm_pmu, cpu_pm_nb);
 	struct pmu_hw_events *hw_events = this_cpu_ptr(armpmu->hw_events);
-	int enabled = bitmap_weight(hw_events->used_mask, armpmu->num_events);
+	bool enabled = !bitmap_empty(hw_events->used_mask, armpmu->num_events);
 
 	if (!cpumask_test_cpu(smp_processor_id(), &armpmu->supported_cpus))
 		return NOTIFY_DONE;

drivers/perf/hisilicon/hisi_uncore_pmu.c +1 -1

index a738aeab5c04..358e4e284a62 100644
@@ -393,7 +393,7 @@ EXPORT_SYMBOL_GPL(hisi_uncore_pmu_read);
 void hisi_uncore_pmu_enable(struct pmu *pmu)
 {
 	struct hisi_pmu *hisi_pmu = to_hisi_pmu(pmu);
-	int enabled = bitmap_weight(hisi_pmu->pmu_events.used_mask,
+	bool enabled = !bitmap_empty(hisi_pmu->pmu_events.used_mask,
 				    hisi_pmu->num_counters);
 
 	if (!enabled)

drivers/perf/xgene_pmu.c +1 -1

index 5283608dc055..0c32dffc7ede 100644
@@ -867,7 +867,7 @@ static void xgene_perf_pmu_enable(struct pmu *pmu)
 {
 	struct xgene_pmu_dev *pmu_dev = to_pmu_dev(pmu);
 	struct xgene_pmu *xgene_pmu = pmu_dev->parent;
-	int enabled = bitmap_weight(pmu_dev->cntr_assign_mask,
+	bool enabled = !bitmap_empty(pmu_dev->cntr_assign_mask,
 			pmu_dev->max_counters);
 
 	if (!enabled)

Signature · 1 line

2.32.0

In some places, drivers/perf code calls bitmap_weight() to check if any bit of a given bitmap is set. It's better to use bitmap_empty() in that case because bitmap_empty() stops traversing the bitmap as soon as it finds first set bit, while bitmap_weight() counts all bits unconditionally.

Signed-off-by Yury Norov <[email protected]>

---
  drivers/perf/arm-cci.c                   | 2 +-
  drivers/perf/arm_pmu.c                   | 4 ++--
  drivers/perf/hisilicon/hisi_uncore_pmu.c | 2 +-
  drivers/perf/xgene_pmu.c                 | 2 +-
  4 files changed, 5 insertions(+), 5 deletions(-)

drivers/perf/arm-cci.c +1 -1

index 54aca3a62814..96e09fa40909 100644
@@ -1096,7 +1096,7 @@ static void cci_pmu_enable(struct pmu *pmu)
  {
  	struct cci_pmu *cci_pmu = to_cci_pmu(pmu);
  	struct cci_pmu_hw_events *hw_events = &cci_pmu->hw_events;
-	int enabled = bitmap_weight(hw_events->used_mask, cci_pmu->num_cntrs);
+	bool enabled = !bitmap_empty(hw_events->used_mask, cci_pmu->num_cntrs);
  	unsigned long flags;
 
  	if (!enabled)

drivers/perf/arm_pmu.c +2 -2

index 295cc7952d0e..a31b302b0ade 100644
@@ -524,7 +524,7 @@ static void armpmu_enable(struct pmu *pmu)
  {
  	struct arm_pmu *armpmu = to_arm_pmu(pmu);
  	struct pmu_hw_events *hw_events = this_cpu_ptr(armpmu->hw_events);
-	int enabled = bitmap_weight(hw_events->used_mask, armpmu->num_events);
+	bool enabled = !bitmap_empty(hw_events->used_mask, armpmu->num_events);
 
  	/* For task-bound events we may be called on other CPUs */
  	if (!cpumask_test_cpu(smp_processor_id(), &armpmu->supported_cpus))

@@ -785,7 +785,7 @@ static int cpu_pm_pmu_notify(struct notifier_block *b, unsigned long cmd,
  {
  	struct arm_pmu *armpmu = container_of(b, struct arm_pmu, cpu_pm_nb);
  	struct pmu_hw_events *hw_events = this_cpu_ptr(armpmu->hw_events);
-	int enabled = bitmap_weight(hw_events->used_mask, armpmu->num_events);
+	bool enabled = !bitmap_empty(hw_events->used_mask, armpmu->num_events);
 
  	if (!cpumask_test_cpu(smp_processor_id(), &armpmu->supported_cpus))
  		return NOTIFY_DONE;

drivers/perf/hisilicon/hisi_uncore_pmu.c +1 -1

index a738aeab5c04..358e4e284a62 100644
@@ -393,7 +393,7 @@ EXPORT_SYMBOL_GPL(hisi_uncore_pmu_read);
  void hisi_uncore_pmu_enable(struct pmu *pmu)
  {
  	struct hisi_pmu *hisi_pmu = to_hisi_pmu(pmu);
-	int enabled = bitmap_weight(hisi_pmu->pmu_events.used_mask,
+	bool enabled = !bitmap_empty(hisi_pmu->pmu_events.used_mask,
  				    hisi_pmu->num_counters);
 
  	if (!enabled)

drivers/perf/xgene_pmu.c +0 -0

index 5283608dc055..0c32dffc7ede 100644

Le 10/02/2022 à 23:48, Yury Norov a écrit :

Quoted 62 lines

In some places, drivers/perf code calls bitmap_weight() to check if any bit of a given bitmap is set. It's better to use bitmap_empty() in that case because bitmap_empty() stops traversing the bitmap as soon as it finds first set bit, while bitmap_weight() counts all bits unconditionally.

Signed-off-by Yury Norov <[email protected]>

---
  drivers/perf/arm-cci.c                   | 2 +-
  drivers/perf/arm_pmu.c                   | 4 ++--
  drivers/perf/hisilicon/hisi_uncore_pmu.c | 2 +-
  drivers/perf/xgene_pmu.c                 | 2 +-
  4 files changed, 5 insertions(+), 5 deletions(-)

drivers/perf/arm-cci.c +1 -1

index 54aca3a62814..96e09fa40909 100644
@@ -1096,7 +1096,7 @@ static void cci_pmu_enable(struct pmu *pmu)
  {
  	struct cci_pmu *cci_pmu = to_cci_pmu(pmu);
  	struct cci_pmu_hw_events *hw_events = &cci_pmu->hw_events;
-	int enabled = bitmap_weight(hw_events->used_mask, cci_pmu->num_cntrs);
+	bool enabled = !bitmap_empty(hw_events->used_mask, cci_pmu->num_cntrs);
  	unsigned long flags;
  	if (!enabled)

drivers/perf/arm_pmu.c +2 -2

index 295cc7952d0e..a31b302b0ade 100644
@@ -524,7 +524,7 @@ static void armpmu_enable(struct pmu *pmu)
  {
  	struct arm_pmu *armpmu = to_arm_pmu(pmu);
  	struct pmu_hw_events *hw_events = this_cpu_ptr(armpmu->hw_events);
-	int enabled = bitmap_weight(hw_events->used_mask, armpmu->num_events);
+	bool enabled = !bitmap_empty(hw_events->used_mask, armpmu->num_events);
  	/* For task-bound events we may be called on other CPUs */
  	if (!cpumask_test_cpu(smp_processor_id(), &armpmu->supported_cpus))

@@ -785,7 +785,7 @@ static int cpu_pm_pmu_notify(struct notifier_block *b, unsigned long cmd,
  {
  	struct arm_pmu *armpmu = container_of(b, struct arm_pmu, cpu_pm_nb);
  	struct pmu_hw_events *hw_events = this_cpu_ptr(armpmu->hw_events);
-	int enabled = bitmap_weight(hw_events->used_mask, armpmu->num_events);
+	bool enabled = !bitmap_empty(hw_events->used_mask, armpmu->num_events);
  	if (!cpumask_test_cpu(smp_processor_id(), &armpmu->supported_cpus))
  		return NOTIFY_DONE;

drivers/perf/hisilicon/hisi_uncore_pmu.c +1 -1

index a738aeab5c04..358e4e284a62 100644
@@ -393,7 +393,7 @@ EXPORT_SYMBOL_GPL(hisi_uncore_pmu_read);
  void hisi_uncore_pmu_enable(struct pmu *pmu)
  {
  	struct hisi_pmu *hisi_pmu = to_hisi_pmu(pmu);
-	int enabled = bitmap_weight(hisi_pmu->pmu_events.used_mask,
+	bool enabled = !bitmap_empty(hisi_pmu->pmu_events.used_mask,
  				    hisi_pmu->num_counters);
  	if (!enabled)

drivers/perf/xgene_pmu.c +0 -0

index 5283608dc055..0c32dffc7ede 100644

drivers/perf/xgene_pmu.c static void xgene_perf_pmu_enable(struct pmu *pmu) · line 871

Quoted 6 lines

drivers/perf/xgene_pmu.c +1 -1

@@ -867,7 +867,7 @@ static void xgene_perf_pmu_enable(struct pmu *pmu)
  {
  	struct xgene_pmu_dev *pmu_dev = to_pmu_dev(pmu);
  	struct xgene_pmu *xgene_pmu = pmu_dev->parent;
-	int enabled = bitmap_weight(pmu_dev->cntr_assign_mask,
+	bool enabled = !bitmap_empty(pmu_dev->cntr_assign_mask,
  			pmu_dev->max_counters);

Would it make sense to call it 'disabled', remove the "!"...

drivers/perf/xgene_pmu.c static void xgene_perf_pmu_enable(struct pmu *pmu) · line 872

Quoted 1 line

drivers/perf/xgene_pmu.c +0 -0

  	if (!enabled)

... and 'if (disabled)' here?

drivers/cpufreq calls cpumask_weight() to check if any bit of a given cpumask is set. We can do it more efficiently with cpumask_empty() because cpumask_empty() stops traversing the cpumask as soon as it finds first set bit, while cpumask_weight() counts all bits unconditionally.

Signed-off-by Yury Norov <[email protected]>

Reviewed-by Sudeep Holla <[email protected]> (for SCMI cpufreq driver)

---
 drivers/cpufreq/qcom-cpufreq-hw.c | 2 +-
 drivers/cpufreq/scmi-cpufreq.c    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Quoted 11 lines · Yury Norov

drivers/cpufreq calls cpumask_weight() to check if any bit of a given cpumask is set. We can do it more efficiently with cpumask_empty() because cpumask_empty() stops traversing the cpumask as soon as it finds first set bit, while cpumask_weight() counts all bits unconditionally.

Signed-off-by Yury Norov <[email protected]>

Reviewed-by Sudeep Holla <[email protected]> (for SCMI cpufreq driver)

---
 drivers/cpufreq/qcom-cpufreq-hw.c | 2 +-
 drivers/cpufreq/scmi-cpufreq.c    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

I already applied it yesterday and replied to you as well. Did I miss something ?

drivers/infiniband/hw/hfi1/affinity.c code calls cpumask_weight() to check if any bit of a given cpumask is set. We can do it more efficiently with cpumask_empty() because cpumask_empty() stops traversing the cpumask as soon as it finds first set bit, while cpumask_weight() counts all bits unconditionally.

Signed-off-by Yury Norov <[email protected]>

Reviewed-by Leon Romanovsky <[email protected]>

---
 drivers/infiniband/hw/hfi1/affinity.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

10.04.2022, 23:27, "tip-bot2 for Yury Norov" <[email protected]>:

The following commit has been merged into the irq/core branch of tip:

Commit-ID: 911488de0565f1d53bd36174d20917ebc4b44c0e
Gitweb: https://git.kernel.org/tip/911488de0565f1d53bd36174d20917ebc4b44c0e
Author: Yury Norov <[email protected]>
AuthorDate: Thu, 10 Feb 2022 14:49:05 -08:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Sun, 10 Apr 2022 22:20:28 +02:00

genirq/affinity: Replace cpumask_weight() with cpumask_empty() where appropriate

__irq_build_affinity_masks() calls cpumask_weight() to check if any bit of a given cpumask is set.

This can be done more efficiently with cpumask_empty() because cpumask_empty() stops traversing the cpumask as soon as it finds first set bit, while cpumask_weight() counts all bits unconditionally.

Hello,
in this patch, struct cpumask *nmsk will also be affected because is called ncpus = cpumask_weight(nmsk); right?

Signed-off-by Yury Norov <[email protected]>

Signed-off-by Thomas Gleixner <[email protected]>

Link https://lore.kernel.org/r/[email protected] open here

---
 kernel/irq/affinity.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/irq/affinity.c +1 -1

index f7ff891..18740fa 100644
@@ -258,7 +258,7 @@ static int __irq_build_affinity_masks(unsigned int startvec,
         nodemask_t nodemsk = NODE_MASK_NONE;
         struct node_vectors *node_vectors;
 
- if (!cpumask_weight(cpu_mask))
+ if (cpumask_empty(cpu_mask))
                 return 0;
 
         nodes = get_nodes_in_cpumask(node_to_cpumask, cpu_mask, &nodemsk);

Ozgur

In some places, kernel/sched code calls cpumask_weight() to check if any bit of a given cpumask is set. We can do it more efficiently with cpumask_empty() because cpumask_empty() stops traversing the cpumask as soon as it finds first set bit, while cpumask_weight() counts all bits unconditionally.

Signed-off-by Yury Norov <[email protected]>

---
 kernel/sched/core.c     | 2 +-
 kernel/sched/topology.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

kernel/sched/core.c +1 -1

index 28d1b7af03dc..ed7b392945b7 100644
@@ -8711,7 +8711,7 @@ int cpuset_cpumask_can_shrink(const struct cpumask *cur,
 {
 	int ret = 1;
 
-	if (!cpumask_weight(cur))
+	if (cpumask_empty(cur))
 		return ret;
 
 	ret = dl_cpuset_cpumask_can_shrink(cur, trial);

kernel/sched/topology.c +1 -1

index d201a7052a29..8478e2a8cd65 100644
@@ -74,7 +74,7 @@ static int sched_domain_debug_one(struct sched_domain *sd, int cpu, int level,
 			break;
 		}
 
-		if (!cpumask_weight(sched_group_span(group))) {
+		if (cpumask_empty(sched_group_span(group))) {
 			printk(KERN_CONT "\n");
 			printk(KERN_ERR "ERROR: empty group\n");
 			break;

Signature · 1 line

2.32.0

Quoted 6 lines · Yury Norov

In some places, kernel/sched code calls cpumask_weight() to check if any bit of a given cpumask is set. We can do it more efficiently with cpumask_empty() because cpumask_empty() stops traversing the cpumask as soon as it finds first set bit, while cpumask_weight() counts all bits unconditionally.

Quoted 1 line

Signed-off-by Yury Norov <[email protected]>

Neither of these paths are really performance sentitive, but whatever.

Do you want me to take this now,

or do you want to merge the whole
series somewere else? In which case:

Acked-by Peter Zijlstra (Intel) <[email protected]>

Quoted 34 lines

---
 kernel/sched/core.c     | 2 +-
 kernel/sched/topology.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

kernel/sched/core.c +1 -1

index 28d1b7af03dc..ed7b392945b7 100644
@@ -8711,7 +8711,7 @@ int cpuset_cpumask_can_shrink(const struct cpumask *cur,
 {
 	int ret = 1;
 
-	if (!cpumask_weight(cur))
+	if (cpumask_empty(cur))
 		return ret;
 
 	ret = dl_cpuset_cpumask_can_shrink(cur, trial);

kernel/sched/topology.c +1 -1

index d201a7052a29..8478e2a8cd65 100644
@@ -74,7 +74,7 @@ static int sched_domain_debug_one(struct sched_domain *sd, int cpu, int level,
 			break;
 		}
 
-		if (!cpumask_weight(sched_group_span(group))) {
+		if (cpumask_empty(sched_group_span(group))) {
 			printk(KERN_CONT "\n");
 			printk(KERN_ERR "ERROR: empty group\n");
 			break;

Signature · 1 line

2.32.0

mm/vmstat.c code calls cpumask_weight() to check if any bit of a given cpumask is set. We can do it more efficiently with cpumask_empty() because cpumask_empty() stops traversing the cpumask as soon as it finds first set bit, while cpumask_weight() counts all bits unconditionally.

Signed-off-by Yury Norov <[email protected]>

---
 mm/vmstat.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

mm/vmstat.c +2 -2

index d5cc8d739fac..27a94afd4ee5 100644
@@ -2041,7 +2041,7 @@ static void __init init_cpu_node_state(void)
 	int node;
 
 	for_each_online_node(node) {
-		if (cpumask_weight(cpumask_of_node(node)) > 0)
+		if (!cpumask_empty(cpumask_of_node(node)))
 			node_set_state(node, N_CPU);
 	}
 }

@@ -2068,7 +2068,7 @@ static int vmstat_cpu_dead(unsigned int cpu)
 
 	refresh_zone_stat_thresholds();
 	node_cpus = cpumask_of_node(node);
-	if (cpumask_weight(node_cpus) > 0)
+	if (!cpumask_empty(node_cpus))
 		return 0;
 
 	node_clear_state(node, N_CPU);

Signature · 1 line

2.32.0

msb_validate_used_block_bitmap() calls bitmap_weight() to compare the weight of bitmap with a given number. We can do it more efficiently with bitmap_weight_eq because conditional bitmap_weight may stop traversing the bitmap earlier, as soon as condition is (or can't be) met.

Signed-off-by Yury Norov <[email protected]>

Acked-by Ulf Hansson <[email protected]>

Acked-by Shubhankar Kuranagatti <[email protected]>

---
 drivers/memstick/core/ms_block.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/memstick/core/ms_block.c +2 -3

index 0cda6c6baefc..5cdd987e78f7 100644
@@ -155,8 +155,8 @@ static int msb_validate_used_block_bitmap(struct msb_data *msb)
        for (i = 0; i < msb->zone_count; i++)
                total_free_blocks += msb->free_block_count[i];
 
-       if (msb->block_count - bitmap_weight(msb->used_blocks_bitmap,
-                                       msb->block_count) == total_free_blocks)
+       if (bitmap_weight_eq(msb->used_blocks_bitmap, msb->block_count,
+                               msb->block_count - total_free_blocks))
                return 0;
 
        pr_err("BUG: free block counts don't match the bitmap");
--

2.32.0

Quoted 6 lines · Yury Norov

msb_validate_used_block_bitmap() calls bitmap_weight() to compare the weight of bitmap with a given number. We can do it more efficiently with bitmap_weight_eq because conditional bitmap_weight may stop traversing the bitmap earlier, as soon as condition is (or can't be) met.

Quoted 3 lines

Signed-off-by Yury Norov <[email protected]>

Acked-by Ulf Hansson <[email protected]>

Acked-by Shubhankar Kuranagatti <[email protected]>

Applied for next, thanks!

Quoted 12 lines · Ulf Hansson

Quoted 9 lines · Yury Norov

msb_validate_used_block_bitmap() calls bitmap_weight() to compare the weight of bitmap with a given number. We can do it more efficiently with bitmap_weight_eq because conditional bitmap_weight may stop traversing the bitmap earlier, as soon as condition is (or can't be) met.

Signed-off-by Yury Norov <[email protected]>

Acked-by Ulf Hansson <[email protected]>

Acked-by Shubhankar Kuranagatti <[email protected]>

Quoted 1 line

Applied for next, thanks!

Hi Ulf,

This patch depends on patch 26/49 "bitmap: add bitmap_weight_{cmp, eq, gt, ge, lt, le} functions"
from this series. Can you make sure you applied them together? Or I can apply it later.

tx2_uncore_event_start() calls bitmap_weight() to compare the weight of bitmap with a given number. We can do it more efficiently with bitmap_weight_eq because conditional bitmap_weight may stop traversing the bitmap earlier, as soon as condition is (or can't be) met.

Signed-off-by Yury Norov <[email protected]>

---
 drivers/perf/thunderx2_pmu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/perf/thunderx2_pmu.c +2 -2

index 1edb9c03704f..97d5b39778fa 100644
@@ -623,8 +623,8 @@ static void tx2_uncore_event_start(struct perf_event *event, int flags)
 		return;
 
 	/* Start timer for first event */
-	if (bitmap_weight(tx2_pmu->active_counters,
-				tx2_pmu->max_counters) == 1) {
+	if (bitmap_weight_eq(tx2_pmu->active_counters,
+				tx2_pmu->max_counters, 1)) {
 		hrtimer_start(&tx2_pmu->hrtimer,
 			ns_to_ktime(tx2_pmu->hrtimer_interval),
 			HRTIMER_MODE_REL_PINNED);

Signature · 1 line

2.32.0

tegra_channel_enum_format() calls bitmap_weight() to compare the weight of bitmap with a given number. We can do it more efficiently with bitmap_weight_le() because conditional bitmap_weight may stop traversing the bitmap earlier, as soon as condition is (or can't be) met.

Signed-off-by Yury Norov <[email protected]>

---
 drivers/staging/media/tegra-video/vi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/staging/media/tegra-video/vi.c +1 -1

index d1f43f465c22..4e79a80e9307 100644
@@ -436,7 +436,7 @@ static int tegra_channel_enum_format(struct file *file, void *fh,
 	if (!IS_ENABLED(CONFIG_VIDEO_TEGRA_TPG))
 		fmts_bitmap = chan->fmts_bitmap;
 
-	if (f->index >= bitmap_weight(fmts_bitmap, MAX_FORMAT_NUM))
+	if (bitmap_weight_le(fmts_bitmap, MAX_FORMAT_NUM, f->index))
 		return -EINVAL;
 
 	for (i = 0; i < f->index + 1; i++, index++)