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
bitmap_empty() is better than bitmap_weight() because it may return
earlier, and improves on readability.
CC: David S. Miller <[email protected]>
CC: Eric Dumazet <[email protected]>
CC: Jakub Kicinski <[email protected]>
CC: Jesse Brandeburg <[email protected]>
CC: Paolo Abeni <[email protected]>
CC: Tony Nguyen <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
Signed-off-by: Yury Norov <[email protected]>
---
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
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 <[email protected]>
CC: Boris Ostrovsky <[email protected]>
CC: Borislav Petkov <[email protected]>
CC: Dave Hansen <[email protected]>
CC: H. Peter Anvin <[email protected]>
CC: Huang Rui <[email protected]>
CC: Ingo Molnar <[email protected]>
CC: Paul E. McKenney <[email protected]>
CC: Peter Zijlstra <[email protected]>
CC: Rafael J. Wysocki <[email protected]>
CC: Sean Christopherson <[email protected]>
CC: Thomas Gleixner <[email protected]>
CC: Tim Chen <[email protected]>
CC: [email protected]
CC: [email protected]
Signed-off-by: Yury Norov <[email protected]>
---
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
remove_siblinginfo() initialises variable 'last', but never uses it.
Drop unneeded code.
CC: Ingo Molnar <[email protected]>
CC: Peter Zijlstra <[email protected]>
CC: Valentin Schneider <[email protected]>
CC: [email protected]
CC: [email protected]
Signed-off-by: Yury Norov <[email protected]>
---
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
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 <[email protected]>
CC: Dave Hansen <[email protected]>
CC: Feng Tang <[email protected]>
CC: H. Peter Anvin <[email protected]>
CC: Ingo Molnar <[email protected]>
CC: Thomas Gleixner <[email protected]>
CC: [email protected]
CC: [email protected]
Signed-off-by: Yury Norov <[email protected]>
---
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
bitmap_empty() is better than bitmap_weight() because it may return
earlier, and improves on readability.
CC: Albert Ou <[email protected]>
CC: Anup Patel <[email protected]>
CC: Atish Patra <[email protected]>
CC: Jisheng Zhang <[email protected]>
CC: Palmer Dabbelt <[email protected]>
CC: Paul Walmsley <[email protected]>
CC: Tsukasa OI <[email protected]>
CC: [email protected]
CC: [email protected]
Signed-off-by: Yury Norov <[email protected]>
---
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
Replace cpumask_weight() with cpumask_weight_eq(..., 1) because it
may return earlier.
CC: Ben Segall <[email protected]>
CC: Daniel Bristot de Oliveira <[email protected]>
CC: Dietmar Eggemann <[email protected]>
CC: Ingo Molnar <[email protected]>
CC: Juri Lelli <[email protected]>
CC: Mel Gorman <[email protected]>
CC: Peter Zijlstra <[email protected]>
CC: Steven Rostedt <[email protected]>
CC: Valentin Schneider <[email protected]>
CC: Vincent Guittot <[email protected]>
CC: [email protected]
Signed-off-by: Yury Norov <[email protected]>
---
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
bitmap_weight_{eq,le} is better than bitmap_weight because it
may return earlier.
CC: Jonathan Cameron <[email protected]>
CC: Lars-Peter Clausen <[email protected]>
CC: Michael Hennerich <[email protected]>
CC: [email protected]
CC: [email protected]
Signed-off-by: Yury Norov <[email protected]>
---
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
cpumask_weight_gt() is better than cpumask_weight() because it may
return earlier depending on condition.
CC: Rafael J. Wysocki <[email protected]>
CC: Viresh Kumar <[email protected]>
CC: [email protected]
CC: [email protected]
Signed-off-by: Yury Norov <[email protected]>
---
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
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 <[email protected]>
CC: Dave Hansen <[email protected]>
CC: H. Peter Anvin <[email protected]>
CC: Ingo Molnar <[email protected]>
CC: Jim Mattson <[email protected]>
CC: Joerg Roedel <[email protected]>
CC: Paolo Bonzini <[email protected]>
CC: Sean Christopherson <[email protected]>
CC: Thomas Gleixner <[email protected]>
CC: Vitaly Kuznetsov <[email protected]>
CC: Wanpeng Li <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
Signed-off-by: Yury Norov <[email protected]>
---
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
On Tue, 10 May 2022 08:47:40 -0700 Yury Norov <[email protected]> wrote:
> remove_siblinginfo() initialises variable 'last', but never uses it.
> Drop unneeded code.
ack. I'll assume you'll be merging this up.
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 <[email protected]>
> CC: Viresh Kumar <[email protected]>
> CC: [email protected]
> CC: [email protected]
> Signed-off-by: Yury Norov <[email protected]>
> ---
> 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 <[email protected]>
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
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
On Tue, 10 May 2022 08:47:33 -0700
Yury Norov <[email protected]> wrote:
> bitmap_weight_{eq,le} is better than bitmap_weight because it
> may return earlier.
>
> CC: Jonathan Cameron <[email protected]>
> CC: Lars-Peter Clausen <[email protected]>
> CC: Michael Hennerich <[email protected]>
> CC: [email protected]
> CC: [email protected]
> Signed-off-by: Yury Norov <[email protected]>
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);
>
On Sat, 2022-05-14 at 16:53 +0100, Jonathan Cameron wrote:
> On Tue, 10 May 2022 08:47:33 -0700
> Yury Norov <[email protected]> wrote:
>
> > bitmap_weight_{eq,le} is better than bitmap_weight because it
> > may return earlier.
> >
> > CC: Jonathan Cameron <[email protected]>
> > CC: Lars-Peter Clausen <[email protected]>
> > CC: Michael Hennerich <[email protected]>
> > CC: [email protected]
> > CC: [email protected]
> > Signed-off-by: Yury Norov <[email protected]>
> 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/[email protected]/
that gives you the entire thread.
Yury Norov <[email protected]> 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 <[email protected]>
> CC: Dave Hansen <[email protected]>
> CC: H. Peter Anvin <[email protected]>
> CC: Ingo Molnar <[email protected]>
> CC: Jim Mattson <[email protected]>
> CC: Joerg Roedel <[email protected]>
> CC: Paolo Bonzini <[email protected]>
> CC: Sean Christopherson <[email protected]>
> CC: Thomas Gleixner <[email protected]>
> CC: Vitaly Kuznetsov <[email protected]>
> CC: Wanpeng Li <[email protected]>
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> Signed-off-by: Yury Norov <[email protected]>
> ---
> 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 <[email protected]>
--
Vitaly
On Sat, 14 May 2022 09:31:23 -0700
Joe Perches <[email protected]> wrote:
> On Sat, 2022-05-14 at 16:53 +0100, Jonathan Cameron wrote:
> > On Tue, 10 May 2022 08:47:33 -0700
> > Yury Norov <[email protected]> wrote:
> >
> > > bitmap_weight_{eq,le} is better than bitmap_weight because it
> > > may return earlier.
> > >
> > > CC: Jonathan Cameron <[email protected]>
> > > CC: Lars-Peter Clausen <[email protected]>
> > > CC: Michael Hennerich <[email protected]>
> > > CC: [email protected]
> > > CC: [email protected]
> > > Signed-off-by: Yury Norov <[email protected]>
> > 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/[email protected]/
>
> that gives you the entire thread.
>
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 <[email protected]>
> CC: Dave Hansen <[email protected]>
> CC: H. Peter Anvin <[email protected]>
> CC: Ingo Molnar <[email protected]>
> CC: Jim Mattson <[email protected]>
> CC: Joerg Roedel <[email protected]>
> CC: Paolo Bonzini <[email protected]>
> CC: Sean Christopherson <[email protected]>
> CC: Thomas Gleixner <[email protected]>
> CC: Vitaly Kuznetsov <[email protected]>
> CC: Wanpeng Li <[email protected]>
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> Signed-off-by: Yury Norov <[email protected]>
> ---
> 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
>
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/[email protected]/T/
Thanks,
Yury