Re: [PATCH 7/9] lib/cpumask: add num_{possible,present,active}_cpus_{eq,gt,le}
On Sun, Nov 28, 2021 at 12:54:00PM -0500, Dennis Zhou wrote:
> Hello,
>
> On Sun, Nov 28, 2021 at 09:43:20AM -0800, Yury Norov wrote:
> > On Sun, Nov 28, 2021 at 09:07:52AM -0800, Joe Perches wrote:
> > > On Sat, 2021-11-27 at 19:57 -0800, Yury Norov wrote:
> > > > Add num_{possible,present,active}_cpus_{eq,gt,le} and replace
> > > > num_*_cpus()
> > > > with one of new functions where appropriate. This allows num_*_cpus_*()
> > > > to return earlier depending on the condition.
> > > []
> > > > diff --git a/arch/arc/kernel/smp.c b/arch/arc/kernel/smp.c
> > > []
> > > > @@ -103,7 +103,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
> > > > * if platform didn't set the present map already, do it now
> > > > * boot cpu is set to present already by init/main.c
> > > > */
> > > > - if (num_present_cpus() <= 1)
> > > > + if (num_present_cpus_le(2))
> > > > init_cpu_present(cpu_possible_mask);
> > >
> > > ? is this supposed to be 2 or 1
> >
> > X <= 1 is the equivalent of X < 2.
> >
> > > > diff --git a/drivers/cpufreq/pcc-cpufreq.c
> > > > b/drivers/cpufreq/pcc-cpufreq.c
> > > []
> > > > @@ -593,7 +593,7 @@ static int __init pcc_cpufreq_init(void)
> > > > return ret;
> > > > }
> > > >
> > > > - if (num_present_cpus() > 4) {
> > > > + if (num_present_cpus_gt(4)) {
> > > > pcc_cpufreq_driver.flags |=
> > > > CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING;
> > > > pr_err("%s: Too many CPUs, dynamic performance scaling
> > > > disabled\n",
> > > >__func__);
> > >
> > > It looks as if the present variants should be using the same values
> > > so the _le test above with 1 changed to 2 looks odd.
> >
>
> I think the confusion comes from le meaning less than rather than lt.
> Given the general convention of: lt (<), le (<=), eg (=), ge (>=),
> gt (>), I'd consider renaming your le to lt.
Ok, makes sense. I'll rename in v2 and add <= and >= versions.
Re: [PATCH 7/9] lib/cpumask: add num_{possible,present,active}_cpus_{eq,gt,le}
On Sun, 2021-11-28 at 09:43 -0800, Yury Norov wrote:
> On Sun, Nov 28, 2021 at 09:07:52AM -0800, Joe Perches wrote:
> > On Sat, 2021-11-27 at 19:57 -0800, Yury Norov wrote:
> > > Add num_{possible,present,active}_cpus_{eq,gt,le} and replace num_*_cpus()
> > > with one of new functions where appropriate. This allows num_*_cpus_*()
> > > to return earlier depending on the condition.
> > []
> > > diff --git a/arch/arc/kernel/smp.c b/arch/arc/kernel/smp.c
> > []
> > > @@ -103,7 +103,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
> > >* if platform didn't set the present map already, do it now
> > >* boot cpu is set to present already by init/main.c
> > >*/
> > > - if (num_present_cpus() <= 1)
> > > + if (num_present_cpus_le(2))
> > > init_cpu_present(cpu_possible_mask);
> >
> > ? is this supposed to be 2 or 1
>
> X <= 1 is the equivalent of X < 2.
True. The call though is _le not _lt
Re: [PATCH 7/9] lib/cpumask: add num_{possible, present, active}_cpus_{eq, gt, le}
On Sun, 28 Nov 2021 at 18:43, Yury Norov wrote:
> On Sun, Nov 28, 2021 at 09:07:52AM -0800, Joe Perches wrote:
> > On Sat, 2021-11-27 at 19:57 -0800, Yury Norov wrote:
> > > Add num_{possible,present,active}_cpus_{eq,gt,le} and replace num_*_cpus()
> > > with one of new functions where appropriate. This allows num_*_cpus_*()
> > > to return earlier depending on the condition.
> > []
> > > diff --git a/arch/arc/kernel/smp.c b/arch/arc/kernel/smp.c
> > []
> > > @@ -103,7 +103,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
> > > * if platform didn't set the present map already, do it now
> > > * boot cpu is set to present already by init/main.c
> > > */
> > > - if (num_present_cpus() <= 1)
> > > + if (num_present_cpus_le(2))
> > > init_cpu_present(cpu_possible_mask);
> >
> > ? is this supposed to be 2 or 1
>
> X <= 1 is the equivalent of X < 2.
Ah, then the function is confusing. Usually it's lt = less than and lt
= less than or equal. Same idea for gt vs ge.
Re: [PATCH 7/9] lib/cpumask: add num_{possible,present,active}_cpus_{eq,gt,le}
Hello,
On Sun, Nov 28, 2021 at 09:43:20AM -0800, Yury Norov wrote:
> On Sun, Nov 28, 2021 at 09:07:52AM -0800, Joe Perches wrote:
> > On Sat, 2021-11-27 at 19:57 -0800, Yury Norov wrote:
> > > Add num_{possible,present,active}_cpus_{eq,gt,le} and replace num_*_cpus()
> > > with one of new functions where appropriate. This allows num_*_cpus_*()
> > > to return earlier depending on the condition.
> > []
> > > diff --git a/arch/arc/kernel/smp.c b/arch/arc/kernel/smp.c
> > []
> > > @@ -103,7 +103,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
> > >* if platform didn't set the present map already, do it now
> > >* boot cpu is set to present already by init/main.c
> > >*/
> > > - if (num_present_cpus() <= 1)
> > > + if (num_present_cpus_le(2))
> > > init_cpu_present(cpu_possible_mask);
> >
> > ? is this supposed to be 2 or 1
>
> X <= 1 is the equivalent of X < 2.
>
> > > diff --git a/drivers/cpufreq/pcc-cpufreq.c b/drivers/cpufreq/pcc-cpufreq.c
> > []
> > > @@ -593,7 +593,7 @@ static int __init pcc_cpufreq_init(void)
> > > return ret;
> > > }
> > >
> > > - if (num_present_cpus() > 4) {
> > > + if (num_present_cpus_gt(4)) {
> > > pcc_cpufreq_driver.flags |= CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING;
> > > pr_err("%s: Too many CPUs, dynamic performance scaling
> > > disabled\n",
> > > __func__);
> >
> > It looks as if the present variants should be using the same values
> > so the _le test above with 1 changed to 2 looks odd.
>
I think the confusion comes from le meaning less than rather than lt.
Given the general convention of: lt (<), le (<=), eg (=), ge (>=),
gt (>), I'd consider renaming your le to lt.
Thanks,
Dennis
Re: [PATCH 7/9] lib/cpumask: add num_{possible,present,active}_cpus_{eq,gt,le}
On Sun, Nov 28, 2021 at 09:07:52AM -0800, Joe Perches wrote:
> On Sat, 2021-11-27 at 19:57 -0800, Yury Norov wrote:
> > Add num_{possible,present,active}_cpus_{eq,gt,le} and replace num_*_cpus()
> > with one of new functions where appropriate. This allows num_*_cpus_*()
> > to return earlier depending on the condition.
> []
> > diff --git a/arch/arc/kernel/smp.c b/arch/arc/kernel/smp.c
> []
> > @@ -103,7 +103,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
> > * if platform didn't set the present map already, do it now
> > * boot cpu is set to present already by init/main.c
> > */
> > - if (num_present_cpus() <= 1)
> > + if (num_present_cpus_le(2))
> > init_cpu_present(cpu_possible_mask);
>
> ? is this supposed to be 2 or 1
X <= 1 is the equivalent of X < 2.
> > diff --git a/drivers/cpufreq/pcc-cpufreq.c b/drivers/cpufreq/pcc-cpufreq.c
> []
> > @@ -593,7 +593,7 @@ static int __init pcc_cpufreq_init(void)
> > return ret;
> > }
> >
> > - if (num_present_cpus() > 4) {
> > + if (num_present_cpus_gt(4)) {
> > pcc_cpufreq_driver.flags |= CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING;
> > pr_err("%s: Too many CPUs, dynamic performance scaling
> > disabled\n",
> >__func__);
>
> It looks as if the present variants should be using the same values
> so the _le test above with 1 changed to 2 looks odd.
Re: [PATCH 7/9] lib/cpumask: add num_{possible,present,active}_cpus_{eq,gt,le}
On Sat, 2021-11-27 at 19:57 -0800, Yury Norov wrote:
> Add num_{possible,present,active}_cpus_{eq,gt,le} and replace num_*_cpus()
> with one of new functions where appropriate. This allows num_*_cpus_*()
> to return earlier depending on the condition.
[]
> diff --git a/arch/arc/kernel/smp.c b/arch/arc/kernel/smp.c
[]
> @@ -103,7 +103,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
>* if platform didn't set the present map already, do it now
>* boot cpu is set to present already by init/main.c
>*/
> - if (num_present_cpus() <= 1)
> + if (num_present_cpus_le(2))
> init_cpu_present(cpu_possible_mask);
? is this supposed to be 2 or 1
> diff --git a/drivers/cpufreq/pcc-cpufreq.c b/drivers/cpufreq/pcc-cpufreq.c
[]
> @@ -593,7 +593,7 @@ static int __init pcc_cpufreq_init(void)
> return ret;
> }
>
> - if (num_present_cpus() > 4) {
> + if (num_present_cpus_gt(4)) {
> pcc_cpufreq_driver.flags |= CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING;
> pr_err("%s: Too many CPUs, dynamic performance scaling
> disabled\n",
> __func__);
It looks as if the present variants should be using the same values
so the _le test above with 1 changed to 2 looks odd.
Re: [PATCH 7/9] lib/cpumask: add num_{possible,present,active}_cpus_{eq,gt,le}
(restore CC list)
On Sun, Nov 28, 2021 at 05:56:51AM +0100, Michał Mirosław wrote:
> On Sat, Nov 27, 2021 at 07:57:02PM -0800, Yury Norov wrote:
> > Add num_{possible,present,active}_cpus_{eq,gt,le} and replace num_*_cpus()
> > with one of new functions where appropriate. This allows num_*_cpus_*()
> > to return earlier depending on the condition.
> [...]
> > @@ -3193,7 +3193,7 @@ int __init pcpu_page_first_chunk(size_t reserved_size,
> >
> > /* allocate pages */
> > j = 0;
> > - for (unit = 0; unit < num_possible_cpus(); unit++) {
> > + for (unit = 0; num_possible_cpus_gt(unit); unit++) {
>
> This looks dubious.
Only this?
> The old version I could hope the compiler would call
> num_possible_cpus() only once if it's marked const or pure, but the
> alternative is going to count the bits every time making this a guaranteed
> O(n^2) even though the bitmap doesn't change.
num_possible_cpus() is not const neither pure. This is O(n^2) before and after.
[PATCH 7/9] lib/cpumask: add num_{possible, present, active}_cpus_{eq, gt, le}
Add num_{possible,present,active}_cpus_{eq,gt,le} and replace num_*_cpus()
with one of new functions where appropriate. This allows num_*_cpus_*()
to return earlier depending on the condition.
Signed-off-by: Yury Norov
---
arch/arc/kernel/smp.c | 2 +-
arch/arm/kernel/machine_kexec.c | 2 +-
arch/arm/mach-exynos/exynos.c | 2 +-
arch/arm/mm/cache-b15-rac.c | 2 +-
arch/arm64/kernel/smp.c | 2 +-
arch/arm64/mm/context.c | 2 +-
arch/csky/mm/asid.c | 2 +-
arch/csky/mm/context.c| 2 +-
arch/ia64/mm/tlb.c| 6 ++---
arch/mips/kernel/i8253.c | 2 +-
arch/mips/kernel/perf_event_mipsxx.c | 4 ++--
arch/mips/kernel/rtlx-cmp.c | 2 +-
arch/mips/kernel/smp.c| 4 ++--
arch/mips/kernel/vpe-cmp.c| 2 +-
.../loongson2ef/common/cs5536/cs5536_mfgpt.c | 2 +-
arch/mips/mm/context.c| 2 +-
arch/mips/mm/tlbex.c | 2 +-
arch/nios2/kernel/cpuinfo.c | 2 +-
arch/powerpc/platforms/85xx/smp.c | 2 +-
arch/powerpc/platforms/pseries/hotplug-cpu.c | 4 ++--
arch/powerpc/sysdev/mpic.c| 2 +-
arch/powerpc/xmon/xmon.c | 6 ++---
arch/riscv/kvm/vmid.c | 2 +-
arch/sparc/kernel/mdesc.c | 6 ++---
arch/x86/events/amd/core.c| 2 +-
arch/x86/kernel/alternative.c | 8 +++
arch/x86/kernel/apic/apic.c | 4 ++--
arch/x86/kernel/apic/apic_flat_64.c | 2 +-
arch/x86/kernel/apic/probe_32.c | 2 +-
arch/x86/kernel/cpu/mce/dev-mcelog.c | 2 +-
arch/x86/kernel/hpet.c| 2 +-
arch/x86/kernel/i8253.c | 2 +-
arch/x86/kernel/kvm.c | 2 +-
arch/x86/kernel/kvmclock.c| 2 +-
arch/x86/kernel/tsc.c | 2 +-
arch/x86/xen/smp_pv.c | 2 +-
arch/x86/xen/spinlock.c | 2 +-
drivers/clk/samsung/clk-exynos4.c | 2 +-
drivers/clocksource/ingenic-timer.c | 3 +--
drivers/cpufreq/pcc-cpufreq.c | 2 +-
drivers/dma/mv_xor.c | 5 ++--
drivers/gpu/drm/i810/i810_drv.c | 2 +-
drivers/irqchip/irq-gic.c | 2 +-
drivers/net/caif/caif_virtio.c| 2 +-
.../cavium/liquidio/cn23xx_vf_device.c| 2 +-
drivers/net/ethernet/hisilicon/hns/hns_enet.c | 2 +-
.../net/ethernet/marvell/mvpp2/mvpp2_main.c | 2 +-
drivers/net/wireless/ath/ath9k/hw.c | 2 +-
drivers/net/wireless/marvell/mwifiex/main.c | 4 ++--
drivers/net/wireless/st/cw1200/queue.c| 3 +--
drivers/nvdimm/region.c | 2 +-
drivers/nvme/host/pci.c | 2 +-
drivers/perf/arm_pmu.c| 2 +-
.../intel/speed_select_if/isst_if_common.c| 6 ++---
drivers/soc/bcm/brcmstb/biuctrl.c | 2 +-
drivers/soc/fsl/dpio/dpio-service.c | 4 ++--
drivers/spi/spi-dw-bt1.c | 2 +-
drivers/virt/acrn/hsm.c | 2 +-
fs/xfs/xfs_sysfs.c| 2 +-
include/linux/cpumask.h | 23 +++
include/linux/kdb.h | 2 +-
kernel/debug/kdb/kdb_bt.c | 2 +-
kernel/printk/printk.c| 2 +-
kernel/reboot.c | 4 ++--
kernel/time/clockevents.c | 2 +-
mm/percpu.c | 6 ++---
mm/slab.c | 2 +-
67 files changed, 110 insertions(+), 90 deletions(-)
diff --git a/arch/arc/kernel/smp.c b/arch/arc/kernel/smp.c
index 78e6d069b1c1..d4f2765755c9 100644
--- a/arch/arc/kernel/smp.c
+++ b/arch/arc/kernel/smp.c
@@ -103,7 +103,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
* if platform didn't set the present map already, do it now
* boot cpu is set to present already by init/main.c
*/
- if (num_present_cpus() <= 1)
+ if (num_present_cpus_le(2))
init_cpu_present(cpu_possible_mask);
}
diff --git a/arch/arm/kernel/machine_kexec.c b/arch/arm/kernel/machine_kexec.c
index f567032a09c0..8875e2ee0083 100644
--- a/arch/arm/kernel/machine_kexec.c
+++ b/arch/arm/kernel/machine_kexec.c
@@ -44,7 +44,7 @@ int machine_kexec_prepare(struct kimage *image)
* and implements CPU hotplug for the current HW. If not, we won't be
* able to kexec reliably, so fail the prepare operation.
*/
- if (num_possible_cpus() > 1 &&
