[PATCH 00/21] add coccinelle scripts for {bitmap,cpumask,nodes}_empty()

[PATCH 00/21] add coccinelle scripts for {bitmap,cpumask,nodes}_empty()

[Yury Norov]

In this series:
 - introduce bitmap.cocci, cpumask.cocci and nodemask.cocci;
 - add checks for cases where bitmap_weight() may be replaced with one of
   "empty, full, gt, lt, ge, le or eq" versions; and
 - address issued found with those scripts.

On top of next-2022-05-06.

Yury Norov (22):
  introduce bitmap.cocci
  introduce cpumask.cocci
  introduce nodemask.cocci
  ice: use bitmap_empty() in ice_vf_has_no_qs_ena()
  iio: replace bitmap_weight with bitmap_weitght_{eq,le} where
    appropriate
  octeontx2: use bitmap_empty() instead of bitmap_weight()
  risc-v: replace bitmap_weight with bitmap_empty in riscv_fill_hwcap()
  bitops: introduce MANY_BITS() macro
  qed: replace bitmap_weight() with MANY_BITS()
  net/mlx5e: simplify mlx5e_set_fecparam()
  KVM: x86: hyper-v: replace bitmap_weight() with hweight64()
  ia64: cleanup remove_siblinginfo()
  x86: smp: move cpumask_weight() out of for-loop in remove_siblinginfo
  x86: smp: use cpumask_weight_eq() in remove_siblinginfo
  net/mlx5: use cpumask_weight_gt() in irq_pool_request_irq()
  x86/tsc: use cpumask_weight_gt() in loop_timeout()
  sched/core: fix opencoded cpumask_any_but() in
    sched_core_cpu_{starting,deactivate}
  sched/core: remove unneeded cpumask_weight in
    sched_core_cpu_{starting,deactivate}
  sched/core: replace cpumask_weight() with cpumask_weight_eq() where
    appropriate
  sched/topology: replace cpumask_weight() with cpumask_weight_eq where
    appropriate
  cpufreq: use cpumask_weight_gt() in policy_is_shared()
  clockevents: use cpumask_weight_eq() in tick_cleanup_dead_cpu()

 MAINTAINERS                                   |   3 +
 arch/ia64/kernel/smpboot.c                    |   4 -
 arch/riscv/kernel/cpufeature.c                |   7 +-
 arch/x86/kernel/smpboot.c                     |   7 +-
 arch/x86/kernel/tsc_sync.c                    |   2 +-
 arch/x86/kvm/hyperv.c                         |   4 +-
 arch/xtensa/kernel/traps.c                    |   5 +-
 drivers/iio/adc/ad_sigma_delta.c              |   2 +-
 drivers/iio/industrialio-buffer.c             |   2 +-
 drivers/net/ethernet/intel/ice/ice_vf_lib.c   |   4 +-
 .../net/ethernet/marvell/octeontx2/af/cgx.c   |   6 +-
 .../net/ethernet/marvell/octeontx2/af/rpm.c   |   2 +-
 .../ethernet/mellanox/mlx5/core/en_ethtool.c  |   4 +-
 .../mellanox/mlx5/core/irq_affinity.c         |   2 +-
 drivers/net/ethernet/qlogic/qed/qed_dev.c     |   3 +-
 include/linux/bitops.h                        |   3 +
 include/linux/cpufreq.h                       |   2 +-
 include/linux/log2.h                          |   2 +-
 kernel/sched/core.c                           |  47 +++-----
 kernel/sched/topology.c                       |   4 +-
 kernel/time/clockevents.c                     |   2 +-
 scripts/coccinelle/api/bitmap.cocci           | 104 ++++++++++++++++++
 scripts/coccinelle/api/cpumask.cocci          |  51 +++++++++
 scripts/coccinelle/api/nodemask.cocci         |  51 +++++++++
 24 files changed, 256 insertions(+), 67 deletions(-)
 create mode 100644 scripts/coccinelle/api/bitmap.cocci
 create mode 100644 scripts/coccinelle/api/cpumask.cocci
 create mode 100644 scripts/coccinelle/api/nodemask.cocci

-- 
2.32.0

[PATCH 01/22] introduce bitmap.cocci

[Yury Norov]

Using bitmap_weight() to compare weight of bitmap against a number or
expression is common but wrong pattern. The more efficient way is to
use bitmap_weight_{empty,full,gt,lt,ge,le,eq} as appropriate.

This patch adds cocci script to test sources for this.

Because for short bitmaps there's no performance adwantage of conditional
bitmap_weight over regular one, and because readability of latter
may be considered better, bitmap_weight_{gt,lt,ge,le,eq} cases are
marked with WARNING.

CC: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
CC: Rasmus Villemoes <linux@rasmusvillemoes.dk>
CC: Julia Lawall <Julia.Lawall@inria.fr>
CC: Nicolas Palix <nicolas.palix@imag.fr>
CC: linux-kernel@vger.kernel.org
CC: cocci@inria.fr
CC: Yury Norov <yury.norov@gmail.com>
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 MAINTAINERS                         |   1 +
 scripts/coccinelle/api/bitmap.cocci | 104 ++++++++++++++++++++++++++++
 2 files changed, 105 insertions(+)
 create mode 100644 scripts/coccinelle/api/bitmap.cocci

diff --git a/MAINTAINERS b/MAINTAINERS
index f57e6d38a542..17fd10824636 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3523,6 +3523,7 @@ F:	lib/find_bit.c
 F:	lib/find_bit_benchmark.c
 F:	lib/nodemask.c
 F:	lib/test_bitmap.c
+F:	scripts/coccinelle/api/bitmap.cocci
 F:	tools/include/linux/bitmap.h
 F:	tools/include/linux/find.h
 F:	tools/lib/bitmap.c
diff --git a/scripts/coccinelle/api/bitmap.cocci b/scripts/coccinelle/api/bitmap.cocci
new file mode 100644
index 000000000000..24ff1809ba1f
--- /dev/null
+++ b/scripts/coccinelle/api/bitmap.cocci
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/// Use bitmap_empty rather than bitmap_weight() == 0 etc
+///
+// Confidence: High
+// Copyright: (C) 2022 Yury Norov
+// URL: http://coccinelle.lip6.fr/
+// Comments:
+// Options: --no-includes --include-headers
+
+virtual org
+virtual report
+virtual context
+virtual patch
+
+@rfull depends on !patch@
+position p;
+expression E1, E2;
+binary operator cmp = {==, !=, <};
+@@
+
+ bitmap_weight(E1,E2) cmp@p E2
+
+@script:python depends on report@
+p << rfull.p;
+@@
+
+coccilib.report.print_report(p[0], "ERROR: use bitmap_full()")
+
+@script:python depends on org@
+p << rfull.p;
+@@
+
+@rempty1 depends on !patch@
+position p;
+statement S;
+@@
+
+ if (bitmap_weight@p(...)) S
+
+@script:python depends on report@
+p << rempty1.p;
+@@
+
+for p0 in p:
+        coccilib.report.print_report(p0, "ERROR: use !bitmap_empty()")
+
+@script:python depends on org@
+p << rempty1.p;
+@@
+
+@rempty depends on !patch@
+position p;
+@@
+
+	bitmap_weight@p(...) == 0
+
+@script:python depends on report@
+p << rempty.p;
+@@
+
+for p0 in p:
+        coccilib.report.print_report(p0, "ERROR: use bitmap_empty()")
+
+@script:python depends on org@
+p << rempty.p;
+@@
+
+@not_rempty depends on !patch@
+position p;
+@@
+
+ bitmap_weight(...) @p> 0
+
+@script:python depends on report@
+p << not_rempty.p;
+@@
+
+for p0 in p:
+        coccilib.report.print_report(p0, "ERROR: use \"!bitmap_empty()\"")
+
+@script:python depends on org@
+p << not_rempty.p;
+@@
+
+
+@rcmp depends on !patch@
+expression exp;
+binary operator cmp = {>, <, >=, <=, ==, !=};
+position p;
+@@
+
+ bitmap_weight(...) cmp@p exp
+
+@script:python depends on report@
+p << rcmp.p;
+@@
+
+for p0 in p:
+        coccilib.report.print_report(p0,
+		"WARNING: use bitmap_weight_{gt,lt,ge,le,eq} as appropriate")
+
+@script:python depends on org@
+p << rcmp.p;
+@@
-- 
2.32.0

[cocci] [PATCH 01/22] introduce bitmap.cocci

[Yury Norov]

Using bitmap_weight() to compare weight of bitmap against a number or
expression is common but wrong pattern. The more efficient way is to
use bitmap_weight_{empty,full,gt,lt,ge,le,eq} as appropriate.

This patch adds cocci script to test sources for this.

Because for short bitmaps there's no performance adwantage of conditional
bitmap_weight over regular one, and because readability of latter
may be considered better, bitmap_weight_{gt,lt,ge,le,eq} cases are
marked with WARNING.

CC: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
CC: Rasmus Villemoes <linux@rasmusvillemoes.dk>
CC: Julia Lawall <Julia.Lawall@inria.fr>
CC: Nicolas Palix <nicolas.palix@imag.fr>
CC: linux-kernel@vger.kernel.org
CC: cocci@inria.fr
CC: Yury Norov <yury.norov@gmail.com>
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 MAINTAINERS                         |   1 +
 scripts/coccinelle/api/bitmap.cocci | 104 ++++++++++++++++++++++++++++
 2 files changed, 105 insertions(+)
 create mode 100644 scripts/coccinelle/api/bitmap.cocci

diff --git a/MAINTAINERS b/MAINTAINERS
index f57e6d38a542..17fd10824636 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3523,6 +3523,7 @@ F:	lib/find_bit.c
 F:	lib/find_bit_benchmark.c
 F:	lib/nodemask.c
 F:	lib/test_bitmap.c
+F:	scripts/coccinelle/api/bitmap.cocci
 F:	tools/include/linux/bitmap.h
 F:	tools/include/linux/find.h
 F:	tools/lib/bitmap.c
diff --git a/scripts/coccinelle/api/bitmap.cocci b/scripts/coccinelle/api/bitmap.cocci
new file mode 100644
index 000000000000..24ff1809ba1f
--- /dev/null
+++ b/scripts/coccinelle/api/bitmap.cocci
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/// Use bitmap_empty rather than bitmap_weight() == 0 etc
+///
+// Confidence: High
+// Copyright: (C) 2022 Yury Norov
+// URL: http://coccinelle.lip6.fr/
+// Comments:
+// Options: --no-includes --include-headers
+
+virtual org
+virtual report
+virtual context
+virtual patch
+
+@rfull depends on !patch@
+position p;
+expression E1, E2;
+binary operator cmp = {==, !=, <};
+@@
+
+ bitmap_weight(E1,E2) cmp@p E2
+
+@script:python depends on report@
+p << rfull.p;
+@@
+
+coccilib.report.print_report(p[0], "ERROR: use bitmap_full()")
+
+@script:python depends on org@
+p << rfull.p;
+@@
+
+@rempty1 depends on !patch@
+position p;
+statement S;
+@@
+
+ if (bitmap_weight@p(...)) S
+
+@script:python depends on report@
+p << rempty1.p;
+@@
+
+for p0 in p:
+        coccilib.report.print_report(p0, "ERROR: use !bitmap_empty()")
+
+@script:python depends on org@
+p << rempty1.p;
+@@
+
+@rempty depends on !patch@
+position p;
+@@
+
+	bitmap_weight@p(...) == 0
+
+@script:python depends on report@
+p << rempty.p;
+@@
+
+for p0 in p:
+        coccilib.report.print_report(p0, "ERROR: use bitmap_empty()")
+
+@script:python depends on org@
+p << rempty.p;
+@@
+
+@not_rempty depends on !patch@
+position p;
+@@
+
+ bitmap_weight(...) @p> 0
+
+@script:python depends on report@
+p << not_rempty.p;
+@@
+
+for p0 in p:
+        coccilib.report.print_report(p0, "ERROR: use \"!bitmap_empty()\"")
+
+@script:python depends on org@
+p << not_rempty.p;
+@@
+
+
+@rcmp depends on !patch@
+expression exp;
+binary operator cmp = {>, <, >=, <=, ==, !=};
+position p;
+@@
+
+ bitmap_weight(...) cmp@p exp
+
+@script:python depends on report@
+p << rcmp.p;
+@@
+
+for p0 in p:
+        coccilib.report.print_report(p0,
+		"WARNING: use bitmap_weight_{gt,lt,ge,le,eq} as appropriate")
+
+@script:python depends on org@
+p << rcmp.p;
+@@
-- 
2.32.0

[PATCH 02/22] introduce cpumask.cocci

Using cpumask_weight() to compare weight of cpumask against a number
or expression is common but wrong pattern. The more efficient way is
to use cpumask_weight_{empty,full,gt,lt,ge,le,eq} as appropriate.

This patch adds cocci script to test sources for this.

CC: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
CC: Julia Lawall <Julia.Lawall@inria.fr>
CC: Nicolas Palix <nicolas.palix@imag.fr>
CC: Rasmus Villemoes <linux@rasmusvillemoes.dk>
CC: linux-kernel@vger.kernel.org
CC: cocci@inria.fr
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 MAINTAINERS                          |  1 +
 scripts/coccinelle/api/cpumask.cocci | 51 ++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)
 create mode 100644 scripts/coccinelle/api/cpumask.cocci

diff --git a/MAINTAINERS b/MAINTAINERS
index 17fd10824636..ecd97fdd5d67 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3524,6 +3524,7 @@ F:	lib/find_bit_benchmark.c
 F:	lib/nodemask.c
 F:	lib/test_bitmap.c
 F:	scripts/coccinelle/api/bitmap.cocci
+F:	scripts/coccinelle/api/cpumask.cocci
 F:	tools/include/linux/bitmap.h
 F:	tools/include/linux/find.h
 F:	tools/lib/bitmap.c
diff --git a/scripts/coccinelle/api/cpumask.cocci b/scripts/coccinelle/api/cpumask.cocci
new file mode 100644
index 000000000000..2eb930dbe9c4
--- /dev/null
+++ b/scripts/coccinelle/api/cpumask.cocci
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/// Use cpumask_empty rather than cpumask_weight() == 0 etc
+///
+// Confidence: High
+// Copyright: (C) 2022 Yury Norov
+// URL: http://coccinelle.lip6.fr/
+// Comments:
+// Options: --no-includes --include-headers
+
+virtual org
+virtual report
+virtual context
+virtual patch
+
+@rempty1 depends on !patch@
+position p;
+statement S;
+@@
+
+if (cpumask_weight@p(...)) S
+
+@script:python depends on report@
+p << rempty1.p;
+@@
+
+for p0 in p:
+        coccilib.report.print_report(p0, "ERROR: use !cpumask_empty()")
+
+@script:python depends on org@
+p << rempty1.p;
+@@
+
+@rcmp depends on !patch@
+expression exp;
+binary operator cmp = {>, <, >=, <=, ==, !=};
+position p;
+@@
+
+ cpumask_weight(...) cmp@p exp
+
+@script:python depends on report@
+p << rcmp.p;
+@@
+
+for p0 in p:
+        coccilib.report.print_report(p0,
+		"ERROR: use cpumask_weight_{empty,full,gt,lt,ge,le,eq} as appropriate")
+
+@script:python depends on org@
+p << rcmp.p;
+@@
-- 
2.32.0

[cocci] [PATCH 02/22] introduce cpumask.cocci

[Yury Norov]

Using cpumask_weight() to compare weight of cpumask against a number
or expression is common but wrong pattern. The more efficient way is
to use cpumask_weight_{empty,full,gt,lt,ge,le,eq} as appropriate.

This patch adds cocci script to test sources for this.

CC: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
CC: Julia Lawall <Julia.Lawall@inria.fr>
CC: Nicolas Palix <nicolas.palix@imag.fr>
CC: Rasmus Villemoes <linux@rasmusvillemoes.dk>
CC: linux-kernel@vger.kernel.org
CC: cocci@inria.fr
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 MAINTAINERS                          |  1 +
 scripts/coccinelle/api/cpumask.cocci | 51 ++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)
 create mode 100644 scripts/coccinelle/api/cpumask.cocci

diff --git a/MAINTAINERS b/MAINTAINERS
index 17fd10824636..ecd97fdd5d67 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3524,6 +3524,7 @@ F:	lib/find_bit_benchmark.c
 F:	lib/nodemask.c
 F:	lib/test_bitmap.c
 F:	scripts/coccinelle/api/bitmap.cocci
+F:	scripts/coccinelle/api/cpumask.cocci
 F:	tools/include/linux/bitmap.h
 F:	tools/include/linux/find.h
 F:	tools/lib/bitmap.c
diff --git a/scripts/coccinelle/api/cpumask.cocci b/scripts/coccinelle/api/cpumask.cocci
new file mode 100644
index 000000000000..2eb930dbe9c4
--- /dev/null
+++ b/scripts/coccinelle/api/cpumask.cocci
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/// Use cpumask_empty rather than cpumask_weight() == 0 etc
+///
+// Confidence: High
+// Copyright: (C) 2022 Yury Norov
+// URL: http://coccinelle.lip6.fr/
+// Comments:
+// Options: --no-includes --include-headers
+
+virtual org
+virtual report
+virtual context
+virtual patch
+
+@rempty1 depends on !patch@
+position p;
+statement S;
+@@
+
+if (cpumask_weight@p(...)) S
+
+@script:python depends on report@
+p << rempty1.p;
+@@
+
+for p0 in p:
+        coccilib.report.print_report(p0, "ERROR: use !cpumask_empty()")
+
+@script:python depends on org@
+p << rempty1.p;
+@@
+
+@rcmp depends on !patch@
+expression exp;
+binary operator cmp = {>, <, >=, <=, ==, !=};
+position p;
+@@
+
+ cpumask_weight(...) cmp@p exp
+
+@script:python depends on report@
+p << rcmp.p;
+@@
+
+for p0 in p:
+        coccilib.report.print_report(p0,
+		"ERROR: use cpumask_weight_{empty,full,gt,lt,ge,le,eq} as appropriate")
+
+@script:python depends on org@
+p << rcmp.p;
+@@
-- 
2.32.0

[PATCH 03/22] introduce nodemask.cocci

[Yury Norov]

Using nodemask_weight() to compare weight of nodemask against a number
or expression is common but wrong pattern. The more efficient way is
to use nodemask_weight_{empty,full,gt,lt,ge,le,eq} as appropriate.

This patch adds cocci script to test sources for this.

CC: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
CC: Julia Lawall <Julia.Lawall@inria.fr>
CC: Nicolas Palix <nicolas.palix@imag.fr>
CC: Rasmus Villemoes <linux@rasmusvillemoes.dk>
CC: linux-kernel@vger.kernel.org
CC: cocci@inria.fr
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 MAINTAINERS                           |  1 +
 scripts/coccinelle/api/nodemask.cocci | 51 +++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)
 create mode 100644 scripts/coccinelle/api/nodemask.cocci

diff --git a/MAINTAINERS b/MAINTAINERS
index ecd97fdd5d67..69f95d79e5a3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3525,6 +3525,7 @@ F:	lib/nodemask.c
 F:	lib/test_bitmap.c
 F:	scripts/coccinelle/api/bitmap.cocci
 F:	scripts/coccinelle/api/cpumask.cocci
+F:	scripts/coccinelle/api/nodemask.cocci
 F:	tools/include/linux/bitmap.h
 F:	tools/include/linux/find.h
 F:	tools/lib/bitmap.c
diff --git a/scripts/coccinelle/api/nodemask.cocci b/scripts/coccinelle/api/nodemask.cocci
new file mode 100644
index 000000000000..1d8578640d97
--- /dev/null
+++ b/scripts/coccinelle/api/nodemask.cocci
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/// Use nodes_empty rather than nodes_weight() == 0 etc
+///
+// Confidence: High
+// Copyright: (C) 2022 Yury Norov
+// URL: http://coccinelle.lip6.fr/
+// Comments:
+// Options: --no-includes --include-headers
+
+virtual org
+virtual report
+virtual context
+virtual patch
+
+@rempty1 depends on !patch@
+position p;
+statement S;
+@@
+
+if (nodes_weight@p(...)) S
+
+@script:python depends on report@
+p << rempty1.p;
+@@
+
+for p0 in p:
+        coccilib.report.print_report(p0, "ERROR: use !nodes_empty()")
+
+@script:python depends on org@
+p << rempty1.p;
+@@
+
+@rcmp depends on !patch@
+expression exp;
+binary operator cmp = {>, <, >=, <=, ==, !=};
+position p;
+@@
+
+ nodes_weight(...) cmp@p exp
+
+@script:python depends on report@
+p << rcmp.p;
+@@
+
+for p0 in p:
+        coccilib.report.print_report(p0,
+		"ERROR: use nodes_weight_{empty,full,gt,lt,ge,le,eq} as appropriate")
+
+@script:python depends on org@
+p << rcmp.p;
+@@
-- 
2.32.0

[cocci] [PATCH 03/22] introduce nodemask.cocci

[Yury Norov]

Using nodemask_weight() to compare weight of nodemask against a number
or expression is common but wrong pattern. The more efficient way is
to use nodemask_weight_{empty,full,gt,lt,ge,le,eq} as appropriate.

This patch adds cocci script to test sources for this.

CC: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
CC: Julia Lawall <Julia.Lawall@inria.fr>
CC: Nicolas Palix <nicolas.palix@imag.fr>
CC: Rasmus Villemoes <linux@rasmusvillemoes.dk>
CC: linux-kernel@vger.kernel.org
CC: cocci@inria.fr
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 MAINTAINERS                           |  1 +
 scripts/coccinelle/api/nodemask.cocci | 51 +++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)
 create mode 100644 scripts/coccinelle/api/nodemask.cocci

diff --git a/MAINTAINERS b/MAINTAINERS
index ecd97fdd5d67..69f95d79e5a3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3525,6 +3525,7 @@ F:	lib/nodemask.c
 F:	lib/test_bitmap.c
 F:	scripts/coccinelle/api/bitmap.cocci
 F:	scripts/coccinelle/api/cpumask.cocci
+F:	scripts/coccinelle/api/nodemask.cocci
 F:	tools/include/linux/bitmap.h
 F:	tools/include/linux/find.h
 F:	tools/lib/bitmap.c
diff --git a/scripts/coccinelle/api/nodemask.cocci b/scripts/coccinelle/api/nodemask.cocci
new file mode 100644
index 000000000000..1d8578640d97
--- /dev/null
+++ b/scripts/coccinelle/api/nodemask.cocci
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/// Use nodes_empty rather than nodes_weight() == 0 etc
+///
+// Confidence: High
+// Copyright: (C) 2022 Yury Norov
+// URL: http://coccinelle.lip6.fr/
+// Comments:
+// Options: --no-includes --include-headers
+
+virtual org
+virtual report
+virtual context
+virtual patch
+
+@rempty1 depends on !patch@
+position p;
+statement S;
+@@
+
+if (nodes_weight@p(...)) S
+
+@script:python depends on report@
+p << rempty1.p;
+@@
+
+for p0 in p:
+        coccilib.report.print_report(p0, "ERROR: use !nodes_empty()")
+
+@script:python depends on org@
+p << rempty1.p;
+@@
+
+@rcmp depends on !patch@
+expression exp;
+binary operator cmp = {>, <, >=, <=, ==, !=};
+position p;
+@@
+
+ nodes_weight(...) cmp@p exp
+
+@script:python depends on report@
+p << rcmp.p;
+@@
+
+for p0 in p:
+        coccilib.report.print_report(p0,
+		"ERROR: use nodes_weight_{empty,full,gt,lt,ge,le,eq} as appropriate")
+
+@script:python depends on org@
+p << rcmp.p;
+@@
-- 
2.32.0

[PATCH 04/22] ice: use bitmap_empty() in ice_vf_has_no_qs_ena()

[Yury Norov]

bitmap_empty() is better than bitmap_weight() because it may return
earlier, and improves on readability.

CC: David S. Miller <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jakub Kicinski <kuba@kernel.org>
CC: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Paolo Abeni <pabeni@redhat.com>
CC: Tony Nguyen <anthony.l.nguyen@intel.com>
CC: intel-wired-lan@lists.osuosl.org
CC: netdev@vger.kernel.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 drivers/net/ethernet/intel/ice/ice_vf_lib.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
index 6578059d9479..de67ac4075f0 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
@@ -740,8 +740,8 @@ bool ice_is_vf_trusted(struct ice_vf *vf)
  */
 bool ice_vf_has_no_qs_ena(struct ice_vf *vf)
 {
-	return (!bitmap_weight(vf->rxq_ena, ICE_MAX_RSS_QS_PER_VF) &&
-		!bitmap_weight(vf->txq_ena, ICE_MAX_RSS_QS_PER_VF));
+	return bitmap_empty(vf->rxq_ena, ICE_MAX_RSS_QS_PER_VF) &&
+	       bitmap_empty(vf->txq_ena, ICE_MAX_RSS_QS_PER_VF);
 }
 
 /**
-- 
2.32.0

[Intel-wired-lan] [PATCH 04/22] ice: use bitmap_empty() in ice_vf_has_no_qs_ena()

[Yury Norov]

bitmap_empty() is better than bitmap_weight() because it may return
earlier, and improves on readability.

CC: David S. Miller <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jakub Kicinski <kuba@kernel.org>
CC: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Paolo Abeni <pabeni@redhat.com>
CC: Tony Nguyen <anthony.l.nguyen@intel.com>
CC: intel-wired-lan at lists.osuosl.org
CC: netdev at vger.kernel.org
CC: linux-kernel at vger.kernel.org
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 drivers/net/ethernet/intel/ice/ice_vf_lib.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
index 6578059d9479..de67ac4075f0 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
@@ -740,8 +740,8 @@ bool ice_is_vf_trusted(struct ice_vf *vf)
  */
 bool ice_vf_has_no_qs_ena(struct ice_vf *vf)
 {
-	return (!bitmap_weight(vf->rxq_ena, ICE_MAX_RSS_QS_PER_VF) &&
-		!bitmap_weight(vf->txq_ena, ICE_MAX_RSS_QS_PER_VF));
+	return bitmap_empty(vf->rxq_ena, ICE_MAX_RSS_QS_PER_VF) &&
+	       bitmap_empty(vf->txq_ena, ICE_MAX_RSS_QS_PER_VF);
 }
 
 /**
-- 
2.32.0

[PATCH 05/22] iio: replace bitmap_weight with bitmap_weitght_{eq,le} where appropriate

[Yury Norov]

bitmap_weight_{eq,le} is better than bitmap_weight because it
may return earlier.

CC: Jonathan Cameron <jic23@kernel.org>
CC: Lars-Peter Clausen <lars@metafoo.de>
CC: Michael Hennerich <Michael.Hennerich@analog.com>
CC: linux-iio@vger.kernel.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 drivers/iio/adc/ad_sigma_delta.c  | 2 +-
 drivers/iio/industrialio-buffer.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
index 261a9a6b45e1..6445b591f071 100644
--- a/drivers/iio/adc/ad_sigma_delta.c
+++ b/drivers/iio/adc/ad_sigma_delta.c
@@ -525,7 +525,7 @@ static bool ad_sd_validate_scan_mask(struct iio_dev *indio_dev, const unsigned l
 {
 	struct ad_sigma_delta *sigma_delta = iio_device_get_drvdata(indio_dev);
 
-	return bitmap_weight(mask, indio_dev->masklength) <= sigma_delta->num_slots;
+	return bitmap_weight_le(mask, indio_dev->masklength, sigma_delta->num_slots);
 }
 
 static const struct iio_buffer_setup_ops ad_sd_buffer_setup_ops = {
diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 06141ca27e1f..18d3d756aee1 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -1824,7 +1824,7 @@ void iio_buffers_free_sysfs_and_mask(struct iio_dev *indio_dev)
 bool iio_validate_scan_mask_onehot(struct iio_dev *indio_dev,
 	const unsigned long *mask)
 {
-	return bitmap_weight(mask, indio_dev->masklength) == 1;
+	return bitmap_weight_eq(mask, indio_dev->masklength, 1);
 }
 EXPORT_SYMBOL_GPL(iio_validate_scan_mask_onehot);
 
-- 
2.32.0

Re: [PATCH 05/22] iio: replace bitmap_weight with bitmap_weitght_{eq,le} where appropriate

[Jonathan Cameron]

On Tue, 10 May 2022 08:47:33 -0700
Yury Norov <yury.norov@gmail.com> wrote:

> bitmap_weight_{eq,le} is better than bitmap_weight because it
> may return earlier.
> 
> CC: Jonathan Cameron <jic23@kernel.org>
> CC: Lars-Peter Clausen <lars@metafoo.de>
> CC: Michael Hennerich <Michael.Hennerich@analog.com>
> CC: linux-iio@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> Signed-off-by: Yury Norov <yury.norov@gmail.com>
without being cc'd on the cover letter, there is no obvious way for
me to know this is reliant in some series to be found in next.

Please call out the exact dependency and whilst it's a long list,
it is good to cc all people cc'd on individual patches also
on the cover letter so they have that background information.

Change seems fine, but I've no idea when/if to pick it up because of
that lack of information.

Jonathan

> ---
>  drivers/iio/adc/ad_sigma_delta.c  | 2 +-
>  drivers/iio/industrialio-buffer.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
> index 261a9a6b45e1..6445b591f071 100644
> --- a/drivers/iio/adc/ad_sigma_delta.c
> +++ b/drivers/iio/adc/ad_sigma_delta.c
> @@ -525,7 +525,7 @@ static bool ad_sd_validate_scan_mask(struct iio_dev *indio_dev, const unsigned l
>  {
>  	struct ad_sigma_delta *sigma_delta = iio_device_get_drvdata(indio_dev);
>  
> -	return bitmap_weight(mask, indio_dev->masklength) <= sigma_delta->num_slots;
> +	return bitmap_weight_le(mask, indio_dev->masklength, sigma_delta->num_slots);
>  }
>  
>  static const struct iio_buffer_setup_ops ad_sd_buffer_setup_ops = {
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index 06141ca27e1f..18d3d756aee1 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -1824,7 +1824,7 @@ void iio_buffers_free_sysfs_and_mask(struct iio_dev *indio_dev)
>  bool iio_validate_scan_mask_onehot(struct iio_dev *indio_dev,
>  	const unsigned long *mask)
>  {
> -	return bitmap_weight(mask, indio_dev->masklength) == 1;
> +	return bitmap_weight_eq(mask, indio_dev->masklength, 1);
>  }
>  EXPORT_SYMBOL_GPL(iio_validate_scan_mask_onehot);
>  

Re: [PATCH 05/22] iio: replace bitmap_weight with bitmap_weitght_{eq,le} where appropriate

[Joe Perches]

On Sat, 2022-05-14 at 16:53 +0100, Jonathan Cameron wrote:
> On Tue, 10 May 2022 08:47:33 -0700
> Yury Norov <yury.norov@gmail.com> wrote:
> 
> > bitmap_weight_{eq,le} is better than bitmap_weight because it
> > may return earlier.
> > 
> > CC: Jonathan Cameron <jic23@kernel.org>
> > CC: Lars-Peter Clausen <lars@metafoo.de>
> > CC: Michael Hennerich <Michael.Hennerich@analog.com>
> > CC: linux-iio@vger.kernel.org
> > CC: linux-kernel@vger.kernel.org
> > Signed-off-by: Yury Norov <yury.norov@gmail.com>
> without being cc'd on the cover letter, there is no obvious way for
> me to know this is reliant in some series to be found in next.
> 
> Please call out the exact dependency and whilst it's a long list,
> it is good to cc all people cc'd on individual patches also
> on the cover letter so they have that background information.

When doing a treewide change like this, vger would commonly
reject the message because of too many recipients.

> 
> Change seems fine, but I've no idea when/if to pick it up because of
> that lack of information.

You could try using lore with the in-reply-to message header id

https://lore.kernel.org/lkml/20220510154750.212913-6-yury.norov@gmail.com/

that gives you the entire thread.

Re: [PATCH 05/22] iio: replace bitmap_weight with bitmap_weitght_{eq,le} where appropriate

[Jonathan Cameron]

On Sat, 14 May 2022 09:31:23 -0700
Joe Perches <joe@perches.com> wrote:

> On Sat, 2022-05-14 at 16:53 +0100, Jonathan Cameron wrote:
> > On Tue, 10 May 2022 08:47:33 -0700
> > Yury Norov <yury.norov@gmail.com> wrote:
> >   
> > > bitmap_weight_{eq,le} is better than bitmap_weight because it
> > > may return earlier.
> > > 
> > > CC: Jonathan Cameron <jic23@kernel.org>
> > > CC: Lars-Peter Clausen <lars@metafoo.de>
> > > CC: Michael Hennerich <Michael.Hennerich@analog.com>
> > > CC: linux-iio@vger.kernel.org
> > > CC: linux-kernel@vger.kernel.org
> > > Signed-off-by: Yury Norov <yury.norov@gmail.com>  
> > without being cc'd on the cover letter, there is no obvious way for
> > me to know this is reliant in some series to be found in next.
> > 
> > Please call out the exact dependency and whilst it's a long list,
> > it is good to cc all people cc'd on individual patches also
> > on the cover letter so they have that background information.  
> 
> When doing a treewide change like this, vger would commonly
> reject the message because of too many recipients.

Hmm. I took a look via lore before sending this moan and didn't think
this actually had that large a list of CCs but maybe my counting wasn't
great (lots of overlaps between different patches).

The series is also not a tree wide change.
It's a set of changes related only by the fact they are using
a call to the same set of functions and the series is based
on next (which is usually a bad idea as a tree to base anything on).
Arguably there are two different sets of functions at that (the bitmap
ones and the cpumask ones)

This is a good set of changes, but taking it slowly and sending these
out as a number of different series after rc1 would have made
much more sense to me.  That way visibility would have been good
and they could have been applied through the various individual trees.

If there is a reason to want to take this via a common tree then
that information needs to be conveyed to all the subsystem maintainers.

Jonathan


> 
> > 
> > Change seems fine, but I've no idea when/if to pick it up because of
> > that lack of information.  
> 
> You could try using lore with the in-reply-to message header id
> 
> https://lore.kernel.org/lkml/20220510154750.212913-6-yury.norov@gmail.com/
> 
> that gives you the entire thread.
> 

[PATCH 06/22] octeontx2: use bitmap_empty() instead of bitmap_weight()

[Yury Norov]

bitmap_empty() is better than bitmap_weight() because it may return
earlier, and improves on readability.

CC: David S. Miller <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Geetha sowjanya <gakula@marvell.com>
CC: Jakub Kicinski <kuba@kernel.org>
CC: Jerin Jacob <jerinj@marvell.com>
CC: Linu Cherian <lcherian@marvell.com>
CC: Paolo Abeni <pabeni@redhat.com>
CC: Subbaraya Sundeep <sbhatta@marvell.com>
CC: Sunil Goutham <sgoutham@marvell.com>
CC: hariprasad <hkelam@marvell.com>
CC: netdev@vger.kernel.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 drivers/net/ethernet/marvell/octeontx2/af/cgx.c | 6 +++---
 drivers/net/ethernet/marvell/octeontx2/af/rpm.c | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
index 25491edc35ce..921bf9cb707b 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
@@ -615,7 +615,7 @@ void cgx_lmac_enadis_rx_pause_fwding(void *cgxd, int lmac_id, bool enable)
 		return;
 
 	/* Pause frames are not enabled just return */
-	if (!bitmap_weight(lmac->rx_fc_pfvf_bmap.bmap, lmac->rx_fc_pfvf_bmap.max))
+	if (bitmap_empty(lmac->rx_fc_pfvf_bmap.bmap, lmac->rx_fc_pfvf_bmap.max))
 		return;
 
 	cgx_lmac_get_pause_frm_status(cgx, lmac_id, &rx_pause, &tx_pause);
@@ -870,13 +870,13 @@ int verify_lmac_fc_cfg(void *cgxd, int lmac_id, u8 tx_pause, u8 rx_pause,
 		set_bit(pfvf_idx, lmac->tx_fc_pfvf_bmap.bmap);
 
 	/* check if other pfvfs are using flow control */
-	if (!rx_pause && bitmap_weight(lmac->rx_fc_pfvf_bmap.bmap, lmac->rx_fc_pfvf_bmap.max)) {
+	if (!rx_pause && !bitmap_empty(lmac->rx_fc_pfvf_bmap.bmap, lmac->rx_fc_pfvf_bmap.max)) {
 		dev_warn(&cgx->pdev->dev,
 			 "Receive Flow control disable not permitted as its used by other PFVFs\n");
 		return -EPERM;
 	}
 
-	if (!tx_pause && bitmap_weight(lmac->tx_fc_pfvf_bmap.bmap, lmac->tx_fc_pfvf_bmap.max)) {
+	if (!tx_pause && !bitmap_empty(lmac->tx_fc_pfvf_bmap.bmap, lmac->tx_fc_pfvf_bmap.max)) {
 		dev_warn(&cgx->pdev->dev,
 			 "Transmit Flow control disable not permitted as its used by other PFVFs\n");
 		return -EPERM;
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rpm.c b/drivers/net/ethernet/marvell/octeontx2/af/rpm.c
index 47e83d7a5804..f2c866825c81 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rpm.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rpm.c
@@ -109,7 +109,7 @@ void rpm_lmac_enadis_rx_pause_fwding(void *rpmd, int lmac_id, bool enable)
 		return;
 
 	/* Pause frames are not enabled just return */
-	if (!bitmap_weight(lmac->rx_fc_pfvf_bmap.bmap, lmac->rx_fc_pfvf_bmap.max))
+	if (bitmap_empty(lmac->rx_fc_pfvf_bmap.bmap, lmac->rx_fc_pfvf_bmap.max))
 		return;
 
 	if (enable) {
-- 
2.32.0

Re: [PATCH 06/22] octeontx2: use bitmap_empty() instead of bitmap_weight()

[Jakub Kicinski]

On Tue, 10 May 2022 08:47:34 -0700 Yury Norov wrote:
> bitmap_empty() is better than bitmap_weight() because it may return
> earlier, and improves on readability.

resend patches 6 and 9 separately please. if you CC the ML only on 
a portion of the patches our build bot will ignore it.

[PATCH 07/22] risc-v: replace bitmap_weight with bitmap_empty in riscv_fill_hwcap()

[Yury Norov]

bitmap_empty() is better than bitmap_weight() because it may return
earlier, and improves on readability.

CC: Albert Ou <aou@eecs.berkeley.edu>
CC: Anup Patel <anup@brainfault.org>
CC: Atish Patra <atishp@atishpatra.org>
CC: Jisheng Zhang <jszhang@kernel.org>
CC: Palmer Dabbelt <palmer@dabbelt.com>
CC: Paul Walmsley <paul.walmsley@sifive.com>
CC: Tsukasa OI <research_trasio@irq.a4lg.com>
CC: linux-riscv@lists.infradead.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 arch/riscv/kernel/cpufeature.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 1b2d42d7f589..f0298d756f66 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -206,11 +206,10 @@ void __init riscv_fill_hwcap(void)
 		else
 			elf_hwcap = this_hwcap;
 
-		if (bitmap_weight(riscv_isa, RISCV_ISA_EXT_MAX))
-			bitmap_and(riscv_isa, riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
-		else
+		if (bitmap_empty(riscv_isa, RISCV_ISA_EXT_MAX))
 			bitmap_copy(riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
-
+		else
+			bitmap_and(riscv_isa, riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
 	}
 
 	/* We don't support systems with F but without D, so mask those out
-- 
2.32.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 07/22] risc-v: replace bitmap_weight with bitmap_empty in riscv_fill_hwcap()

[Yury Norov]

bitmap_empty() is better than bitmap_weight() because it may return
earlier, and improves on readability.

CC: Albert Ou <aou@eecs.berkeley.edu>
CC: Anup Patel <anup@brainfault.org>
CC: Atish Patra <atishp@atishpatra.org>
CC: Jisheng Zhang <jszhang@kernel.org>
CC: Palmer Dabbelt <palmer@dabbelt.com>
CC: Paul Walmsley <paul.walmsley@sifive.com>
CC: Tsukasa OI <research_trasio@irq.a4lg.com>
CC: linux-riscv@lists.infradead.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 arch/riscv/kernel/cpufeature.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 1b2d42d7f589..f0298d756f66 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -206,11 +206,10 @@ void __init riscv_fill_hwcap(void)
 		else
 			elf_hwcap = this_hwcap;
 
-		if (bitmap_weight(riscv_isa, RISCV_ISA_EXT_MAX))
-			bitmap_and(riscv_isa, riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
-		else
+		if (bitmap_empty(riscv_isa, RISCV_ISA_EXT_MAX))
 			bitmap_copy(riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
-
+		else
+			bitmap_and(riscv_isa, riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
 	}
 
 	/* We don't support systems with F but without D, so mask those out
-- 
2.32.0

Re: [PATCH 07/22] risc-v: replace bitmap_weight with bitmap_empty in riscv_fill_hwcap()

[Anup Patel]

On Tue, May 10, 2022 at 9:18 PM Yury Norov <yury.norov@gmail.com> wrote:
>
> bitmap_empty() is better than bitmap_weight() because it may return
> earlier, and improves on readability.
>
> CC: Albert Ou <aou@eecs.berkeley.edu>
> CC: Anup Patel <anup@brainfault.org>
> CC: Atish Patra <atishp@atishpatra.org>
> CC: Jisheng Zhang <jszhang@kernel.org>
> CC: Palmer Dabbelt <palmer@dabbelt.com>
> CC: Paul Walmsley <paul.walmsley@sifive.com>
> CC: Tsukasa OI <research_trasio@irq.a4lg.com>
> CC: linux-riscv@lists.infradead.org
> CC: linux-kernel@vger.kernel.org
> Signed-off-by: Yury Norov <yury.norov@gmail.com>

Looks good to me.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup

> ---
>  arch/riscv/kernel/cpufeature.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 1b2d42d7f589..f0298d756f66 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -206,11 +206,10 @@ void __init riscv_fill_hwcap(void)
>                 else
>                         elf_hwcap = this_hwcap;
>
> -               if (bitmap_weight(riscv_isa, RISCV_ISA_EXT_MAX))
> -                       bitmap_and(riscv_isa, riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
> -               else
> +               if (bitmap_empty(riscv_isa, RISCV_ISA_EXT_MAX))
>                         bitmap_copy(riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
> -
> +               else
> +                       bitmap_and(riscv_isa, riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
>         }
>
>         /* We don't support systems with F but without D, so mask those out
> --
> 2.32.0
>

Re: [PATCH 07/22] risc-v: replace bitmap_weight with bitmap_empty in riscv_fill_hwcap()

[Anup Patel]

On Tue, May 10, 2022 at 9:18 PM Yury Norov <yury.norov@gmail.com> wrote:
>
> bitmap_empty() is better than bitmap_weight() because it may return
> earlier, and improves on readability.
>
> CC: Albert Ou <aou@eecs.berkeley.edu>
> CC: Anup Patel <anup@brainfault.org>
> CC: Atish Patra <atishp@atishpatra.org>
> CC: Jisheng Zhang <jszhang@kernel.org>
> CC: Palmer Dabbelt <palmer@dabbelt.com>
> CC: Paul Walmsley <paul.walmsley@sifive.com>
> CC: Tsukasa OI <research_trasio@irq.a4lg.com>
> CC: linux-riscv@lists.infradead.org
> CC: linux-kernel@vger.kernel.org
> Signed-off-by: Yury Norov <yury.norov@gmail.com>

Looks good to me.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup

> ---
>  arch/riscv/kernel/cpufeature.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 1b2d42d7f589..f0298d756f66 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -206,11 +206,10 @@ void __init riscv_fill_hwcap(void)
>                 else
>                         elf_hwcap = this_hwcap;
>
> -               if (bitmap_weight(riscv_isa, RISCV_ISA_EXT_MAX))
> -                       bitmap_and(riscv_isa, riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
> -               else
> +               if (bitmap_empty(riscv_isa, RISCV_ISA_EXT_MAX))
>                         bitmap_copy(riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
> -
> +               else
> +                       bitmap_and(riscv_isa, riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
>         }
>
>         /* We don't support systems with F but without D, so mask those out
> --
> 2.32.0
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 08/22] bitops: introduce MANY_BITS() macro

[Yury Norov]

arch/xtensa/kernel/traps.c and include/linux/log2.h define very similar
functions with different behaviour. XTENSA defines IS_POW2(), and
log2.h defines is_power_of_2(). The difference is that IS_POW2()
considers 0 as power of 2, while is_power_of_2() - does not.

This discrepancy may confuse reader. From mathematical point of view,
0 is not a power of 2. So let's introduce macro MANY_BITS(), which
returns true if 2 or more bits are set in a number (which is what
XTENSA actually needs), and use it in is_power_of_2().

CC: Alexei Starovoitov <ast@kernel.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Chris Zankel <chris@zankel.net>
CC: Christophe Leroy <christophe.leroy@csgroup.eu>
CC: Eric W. Biederman <ebiederm@xmission.com>
CC: Kumar Kartikeya Dwivedi <memxor@gmail.com>
CC: Max Filippov <jcmvbkbc@gmail.com>
CC: Toke Høiland-Jørgensen <toke@redhat.com>
CC: linux-xtensa@linux-xtensa.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 arch/xtensa/kernel/traps.c | 5 +----
 include/linux/bitops.h     | 3 +++
 include/linux/log2.h       | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/xtensa/kernel/traps.c b/arch/xtensa/kernel/traps.c
index 138a86fbe9d7..040ec38bfce2 100644
--- a/arch/xtensa/kernel/traps.c
+++ b/arch/xtensa/kernel/traps.c
@@ -203,10 +203,7 @@ static void do_multihit(struct pt_regs *regs)
 
 #if XTENSA_FAKE_NMI
 
-#define IS_POW2(v) (((v) & ((v) - 1)) == 0)
-
-#if !(PROFILING_INTLEVEL == XCHAL_EXCM_LEVEL && \
-      IS_POW2(XTENSA_INTLEVEL_MASK(PROFILING_INTLEVEL)))
+#if (MANY_BITS(XTENSA_INTLEVEL_MASK(PROFILING_INTLEVEL)) || PROFILING_INTLEVEL != XCHAL_EXCM_LEVEL)
 #warning "Fake NMI is requested for PMM, but there are other IRQs at or above its level."
 #warning "Fake NMI will be used, but there will be a bugcheck if one of those IRQs fire."
 
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index 7aaed501f768..96bc6a2552d6 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -21,6 +21,9 @@
 #define BITS_TO_U32(nr)		__KERNEL_DIV_ROUND_UP(nr, BITS_PER_TYPE(u32))
 #define BITS_TO_BYTES(nr)	__KERNEL_DIV_ROUND_UP(nr, BITS_PER_TYPE(char))
 
+/* Return: nonzero if 2 or more bits are set */
+#define MANY_BITS(n)		((n) & ((n) - 1))
+
 extern unsigned int __sw_hweight8(unsigned int w);
 extern unsigned int __sw_hweight16(unsigned int w);
 extern unsigned int __sw_hweight32(unsigned int w);
diff --git a/include/linux/log2.h b/include/linux/log2.h
index 9f30d087a128..335b9dbd302d 100644
--- a/include/linux/log2.h
+++ b/include/linux/log2.h
@@ -44,7 +44,7 @@ int __ilog2_u64(u64 n)
 static inline __attribute__((const))
 bool is_power_of_2(unsigned long n)
 {
-	return (n != 0 && ((n & (n - 1)) == 0));
+	return n != 0 && !MANY_BITS(n);
 }
 
 /**
-- 
2.32.0

Re: [PATCH 08/22] bitops: introduce MANY_BITS() macro

[Alexei Starovoitov]

On Tue, May 10, 2022 at 8:48 AM Yury Norov <yury.norov@gmail.com> wrote:
>
> arch/xtensa/kernel/traps.c and include/linux/log2.h define very similar
> functions with different behaviour. XTENSA defines IS_POW2(), and
> log2.h defines is_power_of_2(). The difference is that IS_POW2()
> considers 0 as power of 2, while is_power_of_2() - does not.
>
> This discrepancy may confuse reader. From mathematical point of view,
> 0 is not a power of 2. So let's introduce macro MANY_BITS(), which
> returns true if 2 or more bits are set in a number (which is what
> XTENSA actually needs), and use it in is_power_of_2().
>
> CC: Alexei Starovoitov <ast@kernel.org>
> CC: Andrew Morton <akpm@linux-foundation.org>
> CC: Chris Zankel <chris@zankel.net>
> CC: Christophe Leroy <christophe.leroy@csgroup.eu>
> CC: Eric W. Biederman <ebiederm@xmission.com>
> CC: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> CC: Max Filippov <jcmvbkbc@gmail.com>
> CC: Toke Høiland-Jørgensen <toke@redhat.com>
> CC: linux-xtensa@linux-xtensa.org
> CC: linux-kernel@vger.kernel.org
> Signed-off-by: Yury Norov <yury.norov@gmail.com>
> ---
>  arch/xtensa/kernel/traps.c | 5 +----
>  include/linux/bitops.h     | 3 +++
>  include/linux/log2.h       | 2 +-
>  3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/xtensa/kernel/traps.c b/arch/xtensa/kernel/traps.c
> index 138a86fbe9d7..040ec38bfce2 100644
> --- a/arch/xtensa/kernel/traps.c
> +++ b/arch/xtensa/kernel/traps.c
> @@ -203,10 +203,7 @@ static void do_multihit(struct pt_regs *regs)
>
>  #if XTENSA_FAKE_NMI
>
> -#define IS_POW2(v) (((v) & ((v) - 1)) == 0)
> -
> -#if !(PROFILING_INTLEVEL == XCHAL_EXCM_LEVEL && \
> -      IS_POW2(XTENSA_INTLEVEL_MASK(PROFILING_INTLEVEL)))
> +#if (MANY_BITS(XTENSA_INTLEVEL_MASK(PROFILING_INTLEVEL)) || PROFILING_INTLEVEL != XCHAL_EXCM_LEVEL)
>  #warning "Fake NMI is requested for PMM, but there are other IRQs at or above its level."
>  #warning "Fake NMI will be used, but there will be a bugcheck if one of those IRQs fire."
>
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> index 7aaed501f768..96bc6a2552d6 100644
> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -21,6 +21,9 @@
>  #define BITS_TO_U32(nr)                __KERNEL_DIV_ROUND_UP(nr, BITS_PER_TYPE(u32))
>  #define BITS_TO_BYTES(nr)      __KERNEL_DIV_ROUND_UP(nr, BITS_PER_TYPE(char))
>
> +/* Return: nonzero if 2 or more bits are set */
> +#define MANY_BITS(n)           ((n) & ((n) - 1))
> +
>  extern unsigned int __sw_hweight8(unsigned int w);
>  extern unsigned int __sw_hweight16(unsigned int w);
>  extern unsigned int __sw_hweight32(unsigned int w);
> diff --git a/include/linux/log2.h b/include/linux/log2.h
> index 9f30d087a128..335b9dbd302d 100644
> --- a/include/linux/log2.h
> +++ b/include/linux/log2.h
> @@ -44,7 +44,7 @@ int __ilog2_u64(u64 n)
>  static inline __attribute__((const))
>  bool is_power_of_2(unsigned long n)
>  {
> -       return (n != 0 && ((n & (n - 1)) == 0));
> +       return n != 0 && !MANY_BITS(n);
>  }

Please don't. Open coded version is much easier to read.

RE: [PATCH 08/22] bitops: introduce MANY_BITS() macro

[David Laight]

From: Alexei Starovoitov
> Sent: 10 May 2022 17:51
...
> +/* Return: nonzero if 2 or more bits are set */
> +#define MANY_BITS(n)           ((n) & ((n) - 1))

You can't have a macro that expands its argument twice.

...
> >  static inline __attribute__((const))
> >  bool is_power_of_2(unsigned long n)
> >  {
> > -       return (n != 0 && ((n & (n - 1)) == 0));
> > +       return n != 0 && !MANY_BITS(n);
> >  }
> 
> Please don't. Open coded version is much easier to read.

Especially if you remove all the spare parenthesis.
	return n && !(n & (n - 1));

I bet a lot of callers know the value is non-zero.

I suspect you'll find at least one caller that uses
is_power_of_2() assuming it is !(n & (n - 1)) and
so is wrong for zero.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH 08/22] bitops: introduce MANY_BITS() macro

[Yury Norov]

On Tue, May 10, 2022 at 05:54:13PM +0000, David Laight wrote:
> From: Alexei Starovoitov
> > Sent: 10 May 2022 17:51
> ...
> > +/* Return: nonzero if 2 or more bits are set */
> > +#define MANY_BITS(n)           ((n) & ((n) - 1))
> 
> You can't have a macro that expands its argument twice.

Yes, I'll fix it.
 
> ...
> > >  static inline __attribute__((const))
> > >  bool is_power_of_2(unsigned long n)
> > >  {
> > > -       return (n != 0 && ((n & (n - 1)) == 0));
> > > +       return n != 0 && !MANY_BITS(n);
> > >  }
> > 
> > Please don't. Open coded version is much easier to read.

To me the human-readable version is easier to read. Still, if you thing
that n & (n - 1) is simpler, what for this function is needed at all?

> Especially if you remove all the spare parenthesis.
> 	return n && !(n & (n - 1));
> 
> I bet a lot of callers know the value is non-zero.
> 
> I suspect you'll find at least one caller that uses
> is_power_of_2() assuming it is !(n & (n - 1)) and
> so is wrong for zero.
 
Another thing is that despite __attribute__(const), gcc sometimes doesn't
recognize it as constant expression, and people have to workaround it.
XTENSA is the example for 1st case, and for the 2nd:

arch/powerpc/mm/init-common.c:
        unsigned long minalign = max(MAX_PGTABLE_INDEX_SIZE + 1,
                                                     HUGEPD_SHIFT_MASK + 1);

        /* It would be nice if this was a BUILD_BUG_ON(), but at the
         * moment, gcc doesn't seem to recognize is_power_of_2
         * as a constant expression, so so much for that. */
        BUG_ON(!is_power_of_2(minalign));

This convinced me that we need a simple macro that is decoupled with
pow_2 semantics and can be used in another macros like BUILD_BUG_ON().

Thanks,
Yury

Re: [PATCH 08/22] bitops: introduce MANY_BITS() macro

[Rasmus Villemoes]

On 10/05/2022 21.11, Yury Norov wrote:

> Another thing is that despite __attribute__(const), gcc sometimes doesn't
> recognize it as constant expression, 

An ICE, Integer Constant Expression, is a C language thing.
__attribute__(const) has nothing to do with that, and no use of that
attribute is ever gonna convince gcc to treat an expression containing a
function call as an ICE [C++ is of course a whole different story].

The __attribute__(const) on a static inline function is utterly
pointless, the compiler can certainly see for itself that the function
has no side effects and that the expression can be CSE'ed.

All that aside, please drop that MANY_BITS thing. It will not make any
code any easier to read (MANY, is that four or five? or is it "more than
half the bit width"?). The n&(n-1) pattern is a well-known idiom, I
expect anybody hacking on an OS kernel to recognize

Rasmus

Re: [PATCH 08/22] bitops: introduce MANY_BITS() macro

[Max Filippov]

Hi Yury,

On Tue, May 10, 2022 at 8:48 AM Yury Norov <yury.norov@gmail.com> wrote:
> arch/xtensa/kernel/traps.c and include/linux/log2.h define very similar
> functions with different behaviour. XTENSA defines IS_POW2(), and
> log2.h defines is_power_of_2(). The difference is that IS_POW2()
> considers 0 as power of 2, while is_power_of_2() - does not.

IS_POW2 is constructed this way because we know that there is at least
one non-zero bit in the value that it tests.

> This discrepancy may confuse reader. From mathematical point of view,
> 0 is not a power of 2.

If it would reduce the confusion we can add a check that the value is
non-zero in the IS_POW2 macro.

I'd really like to not introduce the local macro and just use something
standard, but I can't use is_power_of_2 in a preprocessor condition,
can I?

-- 
Thanks.
-- Max

Re: [PATCH 08/22] bitops: introduce MANY_BITS() macro

[Yury Norov]

Hi Max,

On Tue, May 10, 2022 at 10:57:25AM -0700, Max Filippov wrote:
> Hi Yury,
> 
> On Tue, May 10, 2022 at 8:48 AM Yury Norov <yury.norov@gmail.com> wrote:
> > arch/xtensa/kernel/traps.c and include/linux/log2.h define very similar
> > functions with different behaviour. XTENSA defines IS_POW2(), and
> > log2.h defines is_power_of_2(). The difference is that IS_POW2()
> > considers 0 as power of 2, while is_power_of_2() - does not.
> 
> IS_POW2 is constructed this way because we know that there is at least
> one non-zero bit in the value that it tests.
> 
> > This discrepancy may confuse reader. From mathematical point of view,
> > 0 is not a power of 2.
> 
> If it would reduce the confusion we can add a check that the value is
> non-zero in the IS_POW2 macro.
> 
> I'd really like to not introduce the local macro and just use something
> standard,

This patch introduces a macro MANY_BITS() in include/linux/bitops.h, which
is a full analogue of IS_POW2(). Would it work for you to switch to
MANY_BITS()? 

> but I can't use is_power_of_2 in a preprocessor condition, can I?

I believe you can't.

Thanks,
Yury

Re: [PATCH 08/22] bitops: introduce MANY_BITS() macro

[Max Filippov]

On Tue, May 10, 2022 at 12:16 PM Yury Norov <yury.norov@gmail.com> wrote:
>
> Hi Max,
>
> On Tue, May 10, 2022 at 10:57:25AM -0700, Max Filippov wrote:
> > Hi Yury,
> >
> > On Tue, May 10, 2022 at 8:48 AM Yury Norov <yury.norov@gmail.com> wrote:
> > > arch/xtensa/kernel/traps.c and include/linux/log2.h define very similar
> > > functions with different behaviour. XTENSA defines IS_POW2(), and
> > > log2.h defines is_power_of_2(). The difference is that IS_POW2()
> > > considers 0 as power of 2, while is_power_of_2() - does not.
> >
> > IS_POW2 is constructed this way because we know that there is at least
> > one non-zero bit in the value that it tests.
> >
> > > This discrepancy may confuse reader. From mathematical point of view,
> > > 0 is not a power of 2.
> >
> > If it would reduce the confusion we can add a check that the value is
> > non-zero in the IS_POW2 macro.
> >
> > I'd really like to not introduce the local macro and just use something
> > standard,
>
> This patch introduces a macro MANY_BITS() in include/linux/bitops.h, which
> is a full analogue of IS_POW2(). Would it work for you to switch to
> MANY_BITS()?

It would, I guess. It would also work if is_power_of_2 was a macro.

-- 
Thanks.
-- Max

[PATCH 09/22] qed: replace bitmap_weight() with MANY_BITS()

[Yury Norov]

qed_init_qm_get_idx_from_flags() uses bitmap_weight() to check if
number of bits in pq_flags is greater than 1.

It's a bad practice to use bitmap API for things like flags, because flags
are not bitmaps (and it's bloating and potentially not safe - for example
if flags are not declared as unsigned long).

In this case, MANY_BITS() fits better than bitmap_weight(), and
switching to MANY_BITS() silences scripts/coccinelle/api/bitmap.cocci.

CC: Ariel Elior <aelior@marvell.com>
CC: David S. Miller <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jakub Kicinski <kuba@kernel.org>
CC: Manish Chopra <manishc@marvell.com>
CC: Paolo Abeni <pabeni@redhat.com>
CC: netdev@vger.kernel.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 drivers/net/ethernet/qlogic/qed/qed_dev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c
index 672480c9d195..fbe69e538f53 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_dev.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c
@@ -1702,8 +1702,7 @@ static u16 *qed_init_qm_get_idx_from_flags(struct qed_hwfn *p_hwfn,
 	struct qed_qm_info *qm_info = &p_hwfn->qm_info;
 
 	/* Can't have multiple flags set here */
-	if (bitmap_weight(&pq_flags,
-			  sizeof(pq_flags) * BITS_PER_BYTE) > 1) {
+	if (MANY_BITS(pq_flags)) {
 		DP_ERR(p_hwfn, "requested multiple pq flags 0x%lx\n", pq_flags);
 		goto err;
 	}
-- 
2.32.0

[PATCH 10/22] net/mlx5e: simplify mlx5e_set_fecparam()

[Yury Norov]

mlx5e_set_fecparam() used bitmap API to handle fecparam->fec. This is a
bad practice because ->fec is not a bitmap - it's an u32 mask.

The code wants to prevent user from passing ->fec with many modes
enables. For this purpose it's better to use MANY_BITS() macro, which
allows to avoid converting u32 to bitmap.

CC: David S. Miller <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jakub Kicinski <kuba@kernel.org>
CC: Leon Romanovsky <leon@kernel.org>
CC: Paolo Abeni <pabeni@redhat.com>
CC: Saeed Mahameed <saeedm@nvidia.com>
CC: netdev@vger.kernel.org
CC: linux-rdma@vger.kernel.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index 6e80585d731f..316cb72c4cc8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -1664,13 +1664,11 @@ static int mlx5e_set_fecparam(struct net_device *netdev,
 {
 	struct mlx5e_priv *priv = netdev_priv(netdev);
 	struct mlx5_core_dev *mdev = priv->mdev;
-	unsigned long fec_bitmap;
 	u16 fec_policy = 0;
 	int mode;
 	int err;
 
-	bitmap_from_arr32(&fec_bitmap, &fecparam->fec, sizeof(fecparam->fec) * BITS_PER_BYTE);
-	if (bitmap_weight(&fec_bitmap, ETHTOOL_FEC_LLRS_BIT + 1) > 1)
+	if (MANY_BITS(fecparam->fec))
 		return -EOPNOTSUPP;
 
 	for (mode = 0; mode < ARRAY_SIZE(pplm_fec_2_ethtool); mode++) {
-- 
2.32.0

[PATCH 11/22] KVM: x86: hyper-v: replace bitmap_weight() with hweight64()

[Yury Norov]

kvm_hv_flush_tlb() applies bitmap API to a u64 variable valid_bank_mask.
Since valid_bank_mask has a fixed size, we can use hweight64() and avoid
excessive bloating.

CC: Borislav Petkov <bp@alien8.de>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: H. Peter Anvin <hpa@zytor.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: Jim Mattson <jmattson@google.com>
CC: Joerg Roedel <joro@8bytes.org>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Sean Christopherson <seanjc@google.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Vitaly Kuznetsov <vkuznets@redhat.com>
CC: Wanpeng Li <wanpengli@tencent.com>
CC: kvm@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: x86@kernel.org
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 arch/x86/kvm/hyperv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 41585f0edf1e..b652b856df2b 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1855,7 +1855,7 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
 		all_cpus = flush_ex.hv_vp_set.format !=
 			HV_GENERIC_SET_SPARSE_4K;
 
-		if (hc->var_cnt != bitmap_weight((unsigned long *)&valid_bank_mask, 64))
+		if (hc->var_cnt != hweight64(valid_bank_mask))
 			return HV_STATUS_INVALID_HYPERCALL_INPUT;
 
 		if (all_cpus)
@@ -1956,7 +1956,7 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
 		valid_bank_mask = send_ipi_ex.vp_set.valid_bank_mask;
 		all_cpus = send_ipi_ex.vp_set.format == HV_GENERIC_SET_ALL;
 
-		if (hc->var_cnt != bitmap_weight(&valid_bank_mask, 64))
+		if (hc->var_cnt != hweight64(valid_bank_mask))
 			return HV_STATUS_INVALID_HYPERCALL_INPUT;
 
 		if (all_cpus)
-- 
2.32.0

Re: [PATCH 11/22] KVM: x86: hyper-v: replace bitmap_weight() with hweight64()

[Vitaly Kuznetsov]

Yury Norov <yury.norov@gmail.com> writes:

> kvm_hv_flush_tlb() applies bitmap API to a u64 variable valid_bank_mask.
> Since valid_bank_mask has a fixed size, we can use hweight64() and avoid
> excessive bloating.
>
> CC: Borislav Petkov <bp@alien8.de>
> CC: Dave Hansen <dave.hansen@linux.intel.com>
> CC: H. Peter Anvin <hpa@zytor.com>
> CC: Ingo Molnar <mingo@redhat.com>
> CC: Jim Mattson <jmattson@google.com>
> CC: Joerg Roedel <joro@8bytes.org>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Sean Christopherson <seanjc@google.com>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: Vitaly Kuznetsov <vkuznets@redhat.com>
> CC: Wanpeng Li <wanpengli@tencent.com>
> CC: kvm@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> CC: x86@kernel.org
> Signed-off-by: Yury Norov <yury.norov@gmail.com>
> ---
>  arch/x86/kvm/hyperv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 41585f0edf1e..b652b856df2b 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -1855,7 +1855,7 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
>  		all_cpus = flush_ex.hv_vp_set.format !=
>  			HV_GENERIC_SET_SPARSE_4K;
>  
> -		if (hc->var_cnt != bitmap_weight((unsigned long *)&valid_bank_mask, 64))
> +		if (hc->var_cnt != hweight64(valid_bank_mask))
>  			return HV_STATUS_INVALID_HYPERCALL_INPUT;
>  
>  		if (all_cpus)
> @@ -1956,7 +1956,7 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
>  		valid_bank_mask = send_ipi_ex.vp_set.valid_bank_mask;
>  		all_cpus = send_ipi_ex.vp_set.format == HV_GENERIC_SET_ALL;
>  
> -		if (hc->var_cnt != bitmap_weight(&valid_bank_mask, 64))
> +		if (hc->var_cnt != hweight64(valid_bank_mask))
>  			return HV_STATUS_INVALID_HYPERCALL_INPUT;
>  
>  		if (all_cpus)

(please Cc: me on the whole series next time)

Acked-by: Vitaly Kuznetsov <vkuznets@redhat.com>

-- 
Vitaly

Re: [PATCH 11/22] KVM: x86: hyper-v: replace bitmap_weight() with hweight64()

[Guenter Roeck]

On Tue, May 10, 2022 at 08:47:39AM -0700, Yury Norov wrote:
> kvm_hv_flush_tlb() applies bitmap API to a u64 variable valid_bank_mask.
> Since valid_bank_mask has a fixed size, we can use hweight64() and avoid
> excessive bloating.

In kvm_hv_send_ipi(), valid_bank_mask is unsigned long, not u64.

This results in:

arch/x86/kvm/hyperv.c: In function 'kvm_hv_send_ipi':
include/asm-generic/bitops/const_hweight.h:21:76: error: right shift count >= width of type

on all 32-bit builds.

Guenter

> 
> CC: Borislav Petkov <bp@alien8.de>
> CC: Dave Hansen <dave.hansen@linux.intel.com>
> CC: H. Peter Anvin <hpa@zytor.com>
> CC: Ingo Molnar <mingo@redhat.com>
> CC: Jim Mattson <jmattson@google.com>
> CC: Joerg Roedel <joro@8bytes.org>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Sean Christopherson <seanjc@google.com>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: Vitaly Kuznetsov <vkuznets@redhat.com>
> CC: Wanpeng Li <wanpengli@tencent.com>
> CC: kvm@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> CC: x86@kernel.org
> Signed-off-by: Yury Norov <yury.norov@gmail.com>
> ---
>  arch/x86/kvm/hyperv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 41585f0edf1e..b652b856df2b 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -1855,7 +1855,7 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
>  		all_cpus = flush_ex.hv_vp_set.format !=
>  			HV_GENERIC_SET_SPARSE_4K;
>  
> -		if (hc->var_cnt != bitmap_weight((unsigned long *)&valid_bank_mask, 64))
> +		if (hc->var_cnt != hweight64(valid_bank_mask))
>  			return HV_STATUS_INVALID_HYPERCALL_INPUT;
>  
>  		if (all_cpus)
> @@ -1956,7 +1956,7 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
>  		valid_bank_mask = send_ipi_ex.vp_set.valid_bank_mask;
>  		all_cpus = send_ipi_ex.vp_set.format == HV_GENERIC_SET_ALL;
>  
> -		if (hc->var_cnt != bitmap_weight(&valid_bank_mask, 64))
> +		if (hc->var_cnt != hweight64(valid_bank_mask))
>  			return HV_STATUS_INVALID_HYPERCALL_INPUT;
>  
>  		if (all_cpus)
> -- 
> 2.32.0
> 

Re: [PATCH 11/22] KVM: x86: hyper-v: replace bitmap_weight() with hweight64()

[Yury Norov]

On Sun, May 22, 2022 at 07:53:57AM -0700, Guenter Roeck wrote:
> On Tue, May 10, 2022 at 08:47:39AM -0700, Yury Norov wrote:
> > kvm_hv_flush_tlb() applies bitmap API to a u64 variable valid_bank_mask.
> > Since valid_bank_mask has a fixed size, we can use hweight64() and avoid
> > excessive bloating.
> 
> In kvm_hv_send_ipi(), valid_bank_mask is unsigned long, not u64.
> 
> This results in:
> 
> arch/x86/kvm/hyperv.c: In function 'kvm_hv_send_ipi':
> include/asm-generic/bitops/const_hweight.h:21:76: error: right shift count >= width of type
> 
> on all 32-bit builds.
> 
> Guenter

Hi Guenter,

The fix is in Paolo's tree:
https://lore.kernel.org/lkml/20220519171504.1238724-1-yury.norov@gmail.com/T/

Thanks,
Yury

[PATCH 12/22] ia64: cleanup remove_siblinginfo()

[Yury Norov]

remove_siblinginfo() initialises variable 'last', but never uses it.
Drop unneeded code.

CC: Ingo Molnar <mingo@kernel.org>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Valentin Schneider <vschneid@redhat.com>
CC: linux-ia64@vger.kernel.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 arch/ia64/kernel/smpboot.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/ia64/kernel/smpboot.c b/arch/ia64/kernel/smpboot.c
index d10f780c13b9..d0e935cf2093 100644
--- a/arch/ia64/kernel/smpboot.c
+++ b/arch/ia64/kernel/smpboot.c
@@ -576,8 +576,6 @@ clear_cpu_sibling_map(int cpu)
 static void
 remove_siblinginfo(int cpu)
 {
-	int last = 0;
-
 	if (cpu_data(cpu)->threads_per_core == 1 &&
 	    cpu_data(cpu)->cores_per_socket == 1) {
 		cpumask_clear_cpu(cpu, &cpu_core_map[cpu]);
@@ -585,8 +583,6 @@ remove_siblinginfo(int cpu)
 		return;
 	}
 
-	last = (cpumask_weight(&cpu_core_map[cpu]) == 1 ? 1 : 0);
-
 	/* remove it from all sibling map's */
 	clear_cpu_sibling_map(cpu);
 }
-- 
2.32.0

[PATCH 12/22] ia64: cleanup remove_siblinginfo()

[Yury Norov]

remove_siblinginfo() initialises variable 'last', but never uses it.
Drop unneeded code.

CC: Ingo Molnar <mingo@kernel.org>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Valentin Schneider <vschneid@redhat.com>
CC: linux-ia64@vger.kernel.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 arch/ia64/kernel/smpboot.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/ia64/kernel/smpboot.c b/arch/ia64/kernel/smpboot.c
index d10f780c13b9..d0e935cf2093 100644
--- a/arch/ia64/kernel/smpboot.c
+++ b/arch/ia64/kernel/smpboot.c
@@ -576,8 +576,6 @@ clear_cpu_sibling_map(int cpu)
 static void
 remove_siblinginfo(int cpu)
 {
-	int last = 0;
-
 	if (cpu_data(cpu)->threads_per_core = 1 &&
 	    cpu_data(cpu)->cores_per_socket = 1) {
 		cpumask_clear_cpu(cpu, &cpu_core_map[cpu]);
@@ -585,8 +583,6 @@ remove_siblinginfo(int cpu)
 		return;
 	}
 
-	last = (cpumask_weight(&cpu_core_map[cpu]) = 1 ? 1 : 0);
-
 	/* remove it from all sibling map's */
 	clear_cpu_sibling_map(cpu);
 }
-- 
2.32.0

Re: [PATCH 12/22] ia64: cleanup remove_siblinginfo()

[Andrew Morton]

On Tue, 10 May 2022 08:47:40 -0700 Yury Norov <yury.norov@gmail.com> wrote:

> remove_siblinginfo() initialises variable 'last', but never uses it.
> Drop unneeded code.

ack.  I'll assume you'll be merging this up.

Re: [PATCH 12/22] ia64: cleanup remove_siblinginfo()

[Andrew Morton]

On Tue, 10 May 2022 08:47:40 -0700 Yury Norov <yury.norov@gmail.com> wrote:

> remove_siblinginfo() initialises variable 'last', but never uses it.
> Drop unneeded code.

ack.  I'll assume you'll be merging this up.

[PATCH 13/22] x86: smp: move cpumask_weight() out of for-loop in remove_siblinginfo

[Yury Norov]

Because argument of the function is constant inside for_each_cpu()
loop, the cpumask_weight() does the same work O(NR_CPUS) times, while
it may be calculated only once.

This patch moves cpumask_weight() out of the loop and replaces it
with cpumask_weight_eq(). While here, fix comment format.

CC: Balbir Singh <sblbir@amazon.com>
CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Borislav Petkov <bp@alien8.de>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: H. Peter Anvin <hpa@zytor.com>
CC: Huang Rui <ray.huang@amd.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: Paul E. McKenney <paulmck@kernel.org>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
CC: Sean Christopherson <seanjc@google.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Tim Chen <tim.c.chen@linux.intel.com>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 arch/x86/kernel/smpboot.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 5e7f9532a10d..7d948f79ef31 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1605,13 +1605,14 @@ static void remove_siblinginfo(int cpu)
 {
 	int sibling;
 	struct cpuinfo_x86 *c = &cpu_data(cpu);
+	bool last_thread_sibling = cpumask_weight_eq(topology_sibling_cpumask(cpu), 1);
 
 	for_each_cpu(sibling, topology_core_cpumask(cpu)) {
 		cpumask_clear_cpu(cpu, topology_core_cpumask(sibling));
-		/*/
+		/*
 		 * last thread sibling in this cpu core going down
 		 */
-		if (cpumask_weight(topology_sibling_cpumask(cpu)) == 1)
+		if (last_thread_sibling)
 			cpu_data(sibling).booted_cores--;
 	}
 
-- 
2.32.0

Re: [PATCH 13/22] x86: smp: move cpumask_weight() out of for-loop in remove_siblinginfo

[Thomas Gleixner]

On Tue, May 10 2022 at 08:47, Yury Norov wrote:

Subject: x86: smp: move cpumask_weight() out of for-loop in remove_siblinginfo

Can you please format this according to

https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#patch-submission-notes

> Because argument of the function is constant inside for_each_cpu()
> loop, the cpumask_weight() does the same work O(NR_CPUS) times, while
> it may be calculated only once.
>
> This patch moves cpumask_weight() out of the loop and replaces it

Ditto.

Thanks,

        tglx

[PATCH 14/22] x86: smp: use cpumask_weight_eq() in remove_siblinginfo

[Yury Norov]

cpumask_weight_eq() is more efficient because it may stop traversing
cpumask depending on condition.

CC: Balbir Singh <sblbir@amazon.com>
CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Borislav Petkov <bp@alien8.de>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: H. Peter Anvin <hpa@zytor.com>
CC: Huang Rui <ray.huang@amd.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: Paul E. McKenney <paulmck@kernel.org>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
CC: Sean Christopherson <seanjc@google.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Tim Chen <tim.c.chen@linux.intel.com>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 arch/x86/kernel/smpboot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 7d948f79ef31..74ca98b3ab2a 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1621,7 +1621,7 @@ static void remove_siblinginfo(int cpu)
 
 	for_each_cpu(sibling, topology_sibling_cpumask(cpu)) {
 		cpumask_clear_cpu(cpu, topology_sibling_cpumask(sibling));
-		if (cpumask_weight(topology_sibling_cpumask(sibling)) == 1)
+		if (cpumask_weight_eq(topology_sibling_cpumask(sibling), 1))
 			cpu_data(sibling).smt_active = false;
 	}
 
-- 
2.32.0

[PATCH 15/22] net/mlx5: use cpumask_weight_gt() in irq_pool_request_irq()

[Yury Norov]

cpumask_weight_gt() is more efficient because it may stop traversing
cpumask depending on condition.

CC: David S. Miller <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jakub Kicinski <kuba@kernel.org>
CC: Leon Romanovsky <leon@kernel.org>
CC: Paolo Abeni <pabeni@redhat.com>
CC: Saeed Mahameed <saeedm@nvidia.com>
CC: netdev@vger.kernel.org
CC: linux-rdma@vger.kernel.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/irq_affinity.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/irq_affinity.c b/drivers/net/ethernet/mellanox/mlx5/core/irq_affinity.c
index 380a208ab137..d57f804ee934 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/irq_affinity.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/irq_affinity.c
@@ -58,7 +58,7 @@ irq_pool_request_irq(struct mlx5_irq_pool *pool, const struct cpumask *req_mask)
 	if (err)
 		return ERR_PTR(err);
 	if (pool->irqs_per_cpu) {
-		if (cpumask_weight(req_mask) > 1)
+		if (cpumask_weight_gt(req_mask, 1))
 			/* if req_mask contain more then one CPU, set the least loadad CPU
 			 * of req_mask
 			 */
-- 
2.32.0

[PATCH 16/22] x86/tsc: use cpumask_weight_gt() in loop_timeout()

[Yury Norov]

cpumask_weight_gt() is more efficient because it may stop traversing
cpumask depending on condition.

This piece is not performance-critical, but helps keeping consistency of
cpumask_weight() usage.

CC: Borislav Petkov <bp@alien8.de>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: Feng Tang <feng.tang@intel.com>
CC: H. Peter Anvin <hpa@zytor.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 arch/x86/kernel/tsc_sync.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/tsc_sync.c b/arch/x86/kernel/tsc_sync.c
index 9452dc9664b5..2dc80e6f0a4f 100644
--- a/arch/x86/kernel/tsc_sync.c
+++ b/arch/x86/kernel/tsc_sync.c
@@ -340,7 +340,7 @@ static cycles_t check_tsc_warp(unsigned int timeout)
  */
 static inline unsigned int loop_timeout(int cpu)
 {
-	return (cpumask_weight(topology_core_cpumask(cpu)) > 1) ? 2 : 20;
+	return cpumask_weight_gt(topology_core_cpumask(cpu), 1) ? 2 : 20;
 }
 
 /*
-- 
2.32.0

[PATCH 17/22] sched/core: fix opencoded cpumask_any_but()

[Yury Norov]

sched_core_cpu_starting() and sched_core_cpu_deactivate() implement
opencoded cpumask_any_but(). Fix it.

CC: Ben Segall <bsegall@google.com>
CC: Daniel Bristot de Oliveira <bristot@redhat.com>
CC: Dietmar Eggemann <dietmar.eggemann@arm.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: Juri Lelli <juri.lelli@redhat.com>
CC: Mel Gorman <mgorman@suse.de>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Valentin Schneider <vschneid@redhat.com>
CC: Vincent Guittot <vincent.guittot@linaro.org>
CC: linux-kernel@vger.kernel.org
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 kernel/sched/core.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index f5ebc392493d..9700001948d0 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6125,7 +6125,7 @@ static void queue_core_balance(struct rq *rq)
 static void sched_core_cpu_starting(unsigned int cpu)
 {
 	const struct cpumask *smt_mask = cpu_smt_mask(cpu);
-	struct rq *rq = cpu_rq(cpu), *core_rq = NULL;
+	struct rq *rq = cpu_rq(cpu), *core_rq;
 	unsigned long flags;
 	int t;
 
@@ -6138,19 +6138,16 @@ static void sched_core_cpu_starting(unsigned int cpu)
 		goto unlock;
 
 	/* find the leader */
-	for_each_cpu(t, smt_mask) {
-		if (t == cpu)
-			continue;
-		rq = cpu_rq(t);
-		if (rq->core == rq) {
-			core_rq = rq;
-			break;
-		}
-	}
+	t = cpumask_any_but(smt_mask, cpu);
+	if (t >= nr_cpu_ids)
+		goto unlock;
 
-	if (WARN_ON_ONCE(!core_rq)) /* whoopsie */
+	rq = cpu_rq(t);
+	if (WARN_ON_ONCE(rq->core != rq)) /* whoopsie */
 		goto unlock;
 
+	core_rq = rq;
+
 	/* install and validate core_rq */
 	for_each_cpu(t, smt_mask) {
 		rq = cpu_rq(t);
@@ -6168,7 +6165,7 @@ static void sched_core_cpu_starting(unsigned int cpu)
 static void sched_core_cpu_deactivate(unsigned int cpu)
 {
 	const struct cpumask *smt_mask = cpu_smt_mask(cpu);
-	struct rq *rq = cpu_rq(cpu), *core_rq = NULL;
+	struct rq *rq = cpu_rq(cpu), *core_rq;
 	unsigned long flags;
 	int t;
 
@@ -6185,16 +6182,12 @@ static void sched_core_cpu_deactivate(unsigned int cpu)
 		goto unlock;
 
 	/* find a new leader */
-	for_each_cpu(t, smt_mask) {
-		if (t == cpu)
-			continue;
-		core_rq = cpu_rq(t);
-		break;
-	}
-
-	if (WARN_ON_ONCE(!core_rq)) /* impossible */
+	t = cpumask_any_but(smt_mask, cpu);
+	if (t >= nr_cpu_ids)
 		goto unlock;
 
+	core_rq = cpu_rq(t);
+
 	/* copy the shared state to the new leader */
 	core_rq->core_task_seq             = rq->core_task_seq;
 	core_rq->core_pick_seq             = rq->core_pick_seq;
-- 
2.32.0

Re: [PATCH 17/22] sched/core: fix opencoded cpumask_any_but()

[Peter Zijlstra]

On Tue, May 10, 2022 at 08:47:45AM -0700, Yury Norov wrote:
> sched_core_cpu_starting() and sched_core_cpu_deactivate() implement
> opencoded cpumask_any_but(). Fix it.
> 
> CC: Ben Segall <bsegall@google.com>
> CC: Daniel Bristot de Oliveira <bristot@redhat.com>
> CC: Dietmar Eggemann <dietmar.eggemann@arm.com>
> CC: Ingo Molnar <mingo@redhat.com>
> CC: Juri Lelli <juri.lelli@redhat.com>
> CC: Mel Gorman <mgorman@suse.de>
> CC: Peter Zijlstra <peterz@infradead.org>
> CC: Steven Rostedt <rostedt@goodmis.org>
> CC: Valentin Schneider <vschneid@redhat.com>
> CC: Vincent Guittot <vincent.guittot@linaro.org>
> CC: linux-kernel@vger.kernel.org
> Signed-off-by: Yury Norov <yury.norov@gmail.com>
> ---
>  kernel/sched/core.c | 33 +++++++++++++--------------------
>  1 file changed, 13 insertions(+), 20 deletions(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index f5ebc392493d..9700001948d0 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -6125,7 +6125,7 @@ static void queue_core_balance(struct rq *rq)
>  static void sched_core_cpu_starting(unsigned int cpu)
>  {
>  	const struct cpumask *smt_mask = cpu_smt_mask(cpu);
> -	struct rq *rq = cpu_rq(cpu), *core_rq = NULL;
> +	struct rq *rq = cpu_rq(cpu), *core_rq;
>  	unsigned long flags;
>  	int t;
>  
> @@ -6138,19 +6138,16 @@ static void sched_core_cpu_starting(unsigned int cpu)
>  		goto unlock;
>  
>  	/* find the leader */
> -	for_each_cpu(t, smt_mask) {
> -		if (t == cpu)
> -			continue;
> -		rq = cpu_rq(t);
> -		if (rq->core == rq) {
> -			core_rq = rq;
> -			break;
> -		}
> -	}
> +	t = cpumask_any_but(smt_mask, cpu);
> +	if (t >= nr_cpu_ids)
> +		goto unlock;
>  
> -	if (WARN_ON_ONCE(!core_rq)) /* whoopsie */
> +	rq = cpu_rq(t);
> +	if (WARN_ON_ONCE(rq->core != rq)) /* whoopsie */
>  		goto unlock;
>  
> +	core_rq = rq;
> +
>  	/* install and validate core_rq */
>  	for_each_cpu(t, smt_mask) {
>  		rq = cpu_rq(t);

I don't think this is equivalent. Imagine SMT4, with:

  rqN->core_rq = rq0

Now, further suppose smt0-2 are online and we're about to online smt3.
Then t above is free to be smt2, which then results in insta triggering:

+	if (WARN_ON_ONCE(rq->core != rq)) /* whoopsie */

You seem to have lost how the first loop searches for rq->core.

Please, be more careful. Also, all of this is super cold path don't
bother with optimizations. Much of the patches you have in this series
fall under that.

Re: [PATCH 17/22] sched/core: fix opencoded cpumask_any_but()

[Valentin Schneider]

On 10/05/22 18:37, Peter Zijlstra wrote:
> On Tue, May 10, 2022 at 08:47:45AM -0700, Yury Norov wrote:
>> sched_core_cpu_starting() and sched_core_cpu_deactivate() implement
>> opencoded cpumask_any_but(). Fix it.
>>
>> CC: Ben Segall <bsegall@google.com>
>> CC: Daniel Bristot de Oliveira <bristot@redhat.com>
>> CC: Dietmar Eggemann <dietmar.eggemann@arm.com>
>> CC: Ingo Molnar <mingo@redhat.com>
>> CC: Juri Lelli <juri.lelli@redhat.com>
>> CC: Mel Gorman <mgorman@suse.de>
>> CC: Peter Zijlstra <peterz@infradead.org>
>> CC: Steven Rostedt <rostedt@goodmis.org>
>> CC: Valentin Schneider <vschneid@redhat.com>
>> CC: Vincent Guittot <vincent.guittot@linaro.org>
>> CC: linux-kernel@vger.kernel.org
>> Signed-off-by: Yury Norov <yury.norov@gmail.com>
>> ---
>>  kernel/sched/core.c | 33 +++++++++++++--------------------
>>  1 file changed, 13 insertions(+), 20 deletions(-)
>>
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index f5ebc392493d..9700001948d0 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -6125,7 +6125,7 @@ static void queue_core_balance(struct rq *rq)
>>  static void sched_core_cpu_starting(unsigned int cpu)
>>  {
>>      const struct cpumask *smt_mask = cpu_smt_mask(cpu);
>> -	struct rq *rq = cpu_rq(cpu), *core_rq = NULL;
>> +	struct rq *rq = cpu_rq(cpu), *core_rq;
>>      unsigned long flags;
>>      int t;
>>
>> @@ -6138,19 +6138,16 @@ static void sched_core_cpu_starting(unsigned int cpu)
>>              goto unlock;
>>
>>      /* find the leader */
>> -	for_each_cpu(t, smt_mask) {
>> -		if (t == cpu)
>> -			continue;
>> -		rq = cpu_rq(t);
>> -		if (rq->core == rq) {
>> -			core_rq = rq;
>> -			break;
>> -		}
>> -	}
>> +	t = cpumask_any_but(smt_mask, cpu);
>> +	if (t >= nr_cpu_ids)
>> +		goto unlock;
>>
>> -	if (WARN_ON_ONCE(!core_rq)) /* whoopsie */
>> +	rq = cpu_rq(t);
>> +	if (WARN_ON_ONCE(rq->core != rq)) /* whoopsie */
>>              goto unlock;
>>
>> +	core_rq = rq;
>> +
>>      /* install and validate core_rq */
>>      for_each_cpu(t, smt_mask) {
>>              rq = cpu_rq(t);
>
> I don't think this is equivalent. Imagine SMT4, with:
>
>   rqN->core_rq = rq0
>
> Now, further suppose smt0-2 are online and we're about to online smt3.
> Then t above is free to be smt2, which then results in insta triggering:
>
> +	if (WARN_ON_ONCE(rq->core != rq)) /* whoopsie */
>
> You seem to have lost how the first loop searches for rq->core.
>

cpumask_any() is actually cpumask_first(), so t should be smt0 in that
case. However, if for some reason rq->core isn't the first online CPU in
smt_mask \ {cpu} (which I think can happen if you offline smt0-1 then
re-online smt0), then yes that splats.

> Please, be more careful. Also, all of this is super cold path don't
> bother with optimizations. Much of the patches you have in this series
> fall under that.

I tend to agree, I do like the cpumask_weight_eq() stuff because it's a low
hanging fruit and can even be autopatched with coccinelle, but the
open-coded stuff in cold paths isn't as relevant (nor as obvious as it may
look :)).

[PATCH 18/22] sched/core: remove unneeded cpumask_weight() in sched_core_cpu_{starting,deactivate}

[Yury Norov]

The following cpumask_any_but() implicitly calculates if number of smt cpus
is equal to 1, therefore we can drop the code that does it explicitly.

CC: Ben Segall <bsegall@google.com>
CC: Daniel Bristot de Oliveira <bristot@redhat.com>
CC: Dietmar Eggemann <dietmar.eggemann@arm.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: Juri Lelli <juri.lelli@redhat.com>
CC: Mel Gorman <mgorman@suse.de>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Valentin Schneider <vschneid@redhat.com>
CC: Vincent Guittot <vincent.guittot@linaro.org>
CC: linux-kernel@vger.kernel.org
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
This is RFC because it drops WARN_ON_ONCE(rq->core != rq). If this warinig
is important, I'll rework the patch.

 kernel/sched/core.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 9700001948d0..07b8f35cbe36 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6133,12 +6133,10 @@ static void sched_core_cpu_starting(unsigned int cpu)
 
 	WARN_ON_ONCE(rq->core != rq);
 
-	/* if we're the first, we'll be our own leader */
-	if (cpumask_weight(smt_mask) == 1)
-		goto unlock;
-
 	/* find the leader */
 	t = cpumask_any_but(smt_mask, cpu);
+
+	/* if we're the first, we'll be our own leader */
 	if (t >= nr_cpu_ids)
 		goto unlock;
 
@@ -6171,18 +6169,14 @@ static void sched_core_cpu_deactivate(unsigned int cpu)
 
 	sched_core_lock(cpu, &flags);
 
-	/* if we're the last man standing, nothing to do */
-	if (cpumask_weight(smt_mask) == 1) {
-		WARN_ON_ONCE(rq->core != rq);
-		goto unlock;
-	}
-
 	/* if we're not the leader, nothing to do */
 	if (rq->core != rq)
 		goto unlock;
 
 	/* find a new leader */
 	t = cpumask_any_but(smt_mask, cpu);
+
+	/* if we're the last man standing, nothing to do */
 	if (t >= nr_cpu_ids)
 		goto unlock;
 
-- 
2.32.0

[PATCH 19/22] sched/core: replace cpumask_weight() with cpumask_weight_eq() where appropriate

[Yury Norov]

cpumask_weight_eq() is better than cpumask_weight() because it may
return earlier depending on condition.

CC: Ben Segall <bsegall@google.com>
CC: Daniel Bristot de Oliveira <bristot@redhat.com>
CC: Dietmar Eggemann <dietmar.eggemann@arm.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: Juri Lelli <juri.lelli@redhat.com>
CC: Mel Gorman <mgorman@suse.de>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Valentin Schneider <vschneid@redhat.com>
CC: Vincent Guittot <vincent.guittot@linaro.org>
CC: linux-kernel@vger.kernel.org
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 kernel/sched/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 07b8f35cbe36..6bbd5e880984 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -9234,7 +9234,7 @@ int sched_cpu_activate(unsigned int cpu)
 	/*
 	 * When going up, increment the number of cores with SMT present.
 	 */
-	if (cpumask_weight(cpu_smt_mask(cpu)) == 2)
+	if (cpumask_weight_eq(cpu_smt_mask(cpu), 2))
 		static_branch_inc_cpuslocked(&sched_smt_present);
 #endif
 	set_cpu_active(cpu, true);
@@ -9310,7 +9310,7 @@ int sched_cpu_deactivate(unsigned int cpu)
 	/*
 	 * When going down, decrement the number of cores with SMT present.
 	 */
-	if (cpumask_weight(cpu_smt_mask(cpu)) == 2)
+	if (cpumask_weight_eq(cpu_smt_mask(cpu), 2))
 		static_branch_dec_cpuslocked(&sched_smt_present);
 
 	sched_core_cpu_deactivate(cpu);
-- 
2.32.0

[PATCH 20/22] sched/topology: replace cpumask_weight() with cpumask_weight_eq() where appropriate

[Yury Norov]

Replace cpumask_weight() with cpumask_weight_eq(..., 1) because it
may return earlier.

CC: Ben Segall <bsegall@google.com>
CC: Daniel Bristot de Oliveira <bristot@redhat.com>
CC: Dietmar Eggemann <dietmar.eggemann@arm.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: Juri Lelli <juri.lelli@redhat.com>
CC: Mel Gorman <mgorman@suse.de>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Valentin Schneider <vschneid@redhat.com>
CC: Vincent Guittot <vincent.guittot@linaro.org>
CC: linux-kernel@vger.kernel.org
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 kernel/sched/topology.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 05b6c2ad90b9..860137913b18 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -168,7 +168,7 @@ static const unsigned int SD_DEGENERATE_GROUPS_MASK =
 
 static int sd_degenerate(struct sched_domain *sd)
 {
-	if (cpumask_weight(sched_domain_span(sd)) == 1)
+	if (cpumask_weight_eq(sched_domain_span(sd), 1))
 		return 1;
 
 	/* Following flags need at least 2 groups */
@@ -1999,7 +1999,7 @@ void sched_update_numa(int cpu, bool online)
 	 * Scheduler NUMA topology is updated when the first CPU of a
 	 * node is onlined or the last CPU of a node is offlined.
 	 */
-	if (cpumask_weight(cpumask_of_node(node)) != 1)
+	if (!cpumask_weight_eq(cpumask_of_node(node), 1))
 		return;
 
 	sched_reset_numa();
-- 
2.32.0

[PATCH 21/22] cpufreq: use cpumask_weight_gt() in policy_is_shared()

[Yury Norov]

cpumask_weight_gt() is better than cpumask_weight() because it may
return earlier depending on condition.

CC: Rafael J. Wysocki <rafael@kernel.org>
CC: Viresh Kumar <viresh.kumar@linaro.org>
CC: linux-pm@vger.kernel.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 include/linux/cpufreq.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index d5595d57f4e5..865cc9e23518 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -214,7 +214,7 @@ static inline bool policy_is_inactive(struct cpufreq_policy *policy)
 
 static inline bool policy_is_shared(struct cpufreq_policy *policy)
 {
-	return cpumask_weight(policy->cpus) > 1;
+	return cpumask_weight_gt(policy->cpus, 1);
 }
 
 #ifdef CONFIG_CPU_FREQ
-- 
2.32.0

Re: [PATCH 21/22] cpufreq: use cpumask_weight_gt() in policy_is_shared()

[Viresh Kumar]

On 10-05-22, 08:47, Yury Norov wrote:
> cpumask_weight_gt() is better than cpumask_weight() because it may
> return earlier depending on condition.
> 
> CC: Rafael J. Wysocki <rafael@kernel.org>
> CC: Viresh Kumar <viresh.kumar@linaro.org>
> CC: linux-pm@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> Signed-off-by: Yury Norov <yury.norov@gmail.com>
> ---
>  include/linux/cpufreq.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index d5595d57f4e5..865cc9e23518 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -214,7 +214,7 @@ static inline bool policy_is_inactive(struct cpufreq_policy *policy)
>  
>  static inline bool policy_is_shared(struct cpufreq_policy *policy)
>  {
> -	return cpumask_weight(policy->cpus) > 1;
> +	return cpumask_weight_gt(policy->cpus, 1);
>  }
>  
>  #ifdef CONFIG_CPU_FREQ

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Though the patch to add cpumask_weight_gt() is still in linux-next and so this
patch should get merged after rc1 is out.

And it would have been nice to know of this dependency in the original mail
itself instead of me searching for it :)

-- 
viresh

[PATCH 22/22] clockevents: use cpumask_weight_eq() in tick_cleanup_dead_cpu()

[Yury Norov]

Replace cpumask_weight() with cpumask_weight_eq(..., 1) because the it may
return earlier.

CC: Thomas Gleixner <tglx@linutronix.de>
CC: linux-kernel@vger.kernel.org
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 kernel/time/clockevents.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index 5d85014d59b5..fa651263650f 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -648,7 +648,7 @@ void tick_cleanup_dead_cpu(int cpu)
 	 */
 	list_for_each_entry_safe(dev, tmp, &clockevent_devices, list) {
 		if (cpumask_test_cpu(cpu, dev->cpumask) &&
-		    cpumask_weight(dev->cpumask) == 1 &&
+		    cpumask_weight_eq(dev->cpumask, 1) &&
 		    !tick_is_broadcast_device(dev)) {
 			BUG_ON(!clockevent_state_detached(dev));
 			list_del(&dev->list);
-- 
2.32.0

Re: [PATCH 22/22] clockevents: use cpumask_weight_eq() in tick_cleanup_dead_cpu()

[Thomas Gleixner]

On Tue, May 10 2022 at 08:47, Yury Norov wrote:
> Replace cpumask_weight() with cpumask_weight_eq(..., 1) because the it may
> return earlier.

'because the it'?