Hi all,

After merging the bitmap tree, today's linux-next build (arm64 defconfig)
failed like this:

arch/arm64/kernel/topology.c: In function 'arch_freq_get_on_cpu':
arch/arm64/kernel/topology.c:270:43: error: too many arguments to function 'cpumask_next_wrap'
  270 |                                 ref_cpu = cpumask_next_wrap(ref_cpu, policy->cpus,
      |                                           ^~~~~~~~~~~~~~~~~
In file included from arch/arm64/include/asm/cpufeature.h:27,
                 from arch/arm64/include/asm/ptrace.h:11,
                 from arch/arm64/include/asm/irqflags.h:9,
                 from include/linux/irqflags.h:18,
                 from include/linux/spinlock.h:59,
                 from include/linux/mmzone.h:8,
                 from include/linux/gfp.h:7,
                 from include/linux/slab.h:16,
                 from include/linux/resource_ext.h:11,
                 from include/linux/acpi.h:13,
                 from arch/arm64/kernel/topology.c:14:
include/linux/cpumask.h:317:14: note: declared here
  317 | unsigned int cpumask_next_wrap(int n, const struct cpumask *src)
      |              ^~~~~~~~~~~~~~~~~

Caused by commits

  46ac1bec179d ("cpumask: deprecate cpumask_next_wrap()")
  43f7f920e14e ("cpumask: re-introduce cpumask_next{,_and}_wrap()")
  65b98ea8b278 ("cpumask: drop cpumask_next_wrap_old()")

interacting with commit

  dd871ac1237f ("arm64: Provide an AMU-based version of arch_freq_get_on_cpu")

from the arm64 tree.

I have applied the following patch for today (which may not be correct).

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 18 Feb 2025 15:44:06 +1100
Subject: [PATCH] fixup for "cpumask: drop cpumask_next_wrap_old()"

interacting with commit

  dd871ac1237f ("arm64: Provide an AMU-based version of arch_freq_get_on_cpu")

from the arm64 tree.

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 arch/arm64/kernel/topology.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 6f0cab8e746b..70db234c41a2 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -231,7 +231,6 @@ void arch_cpu_idle_enter(void)
 int arch_freq_get_on_cpu(int cpu)
 {
 	struct amu_cntr_sample *amu_sample;
-	unsigned int start_cpu = cpu;
 	unsigned long last_update;
 	unsigned int freq = 0;
 	u64 scale;
@@ -267,8 +266,7 @@ int arch_freq_get_on_cpu(int cpu)
 
 
 			do {
-				ref_cpu = cpumask_next_wrap(ref_cpu, policy->cpus,
-							    start_cpu, false);
+				ref_cpu = cpumask_next_wrap(ref_cpu, policy->cpus);
 
 			} while (ref_cpu < nr_cpu_ids && idle_cpu(ref_cpu));
 
-- 
2.45.2

-- 
Cheers,
Stephen Rothwell

Hi,
On Tue, Feb 18, 2025 at 04:07:42PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the bitmap tree, today's linux-next build (arm64 defconfig)
> failed like this:
> 
> arch/arm64/kernel/topology.c: In function 'arch_freq_get_on_cpu':
> arch/arm64/kernel/topology.c:270:43: error: too many arguments to function 'cpumask_next_wrap'
>   270 |                                 ref_cpu = cpumask_next_wrap(ref_cpu, policy->cpus,
>       |                                           ^~~~~~~~~~~~~~~~~
> In file included from arch/arm64/include/asm/cpufeature.h:27,
>                  from arch/arm64/include/asm/ptrace.h:11,
>                  from arch/arm64/include/asm/irqflags.h:9,
>                  from include/linux/irqflags.h:18,
>                  from include/linux/spinlock.h:59,
>                  from include/linux/mmzone.h:8,
>                  from include/linux/gfp.h:7,
>                  from include/linux/slab.h:16,
>                  from include/linux/resource_ext.h:11,
>                  from include/linux/acpi.h:13,
>                  from arch/arm64/kernel/topology.c:14:
> include/linux/cpumask.h:317:14: note: declared here
>   317 | unsigned int cpumask_next_wrap(int n, const struct cpumask *src)
>       |              ^~~~~~~~~~~~~~~~~
> 
> Caused by commits
> 
>   46ac1bec179d ("cpumask: deprecate cpumask_next_wrap()")
>   43f7f920e14e ("cpumask: re-introduce cpumask_next{,_and}_wrap()")
>   65b98ea8b278 ("cpumask: drop cpumask_next_wrap_old()")
> 
> interacting with commit
> 
>   dd871ac1237f ("arm64: Provide an AMU-based version of arch_freq_get_on_cpu")
> 
> from the arm64 tree.
> 
> I have applied the following patch for today (which may not be correct).
Thank you for that.

I'm currently testing a proper fix for that one.
Should I just send it over as a diff to apply or rather a proper 'fixes' patch?

---
BR
Beata
> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Tue, 18 Feb 2025 15:44:06 +1100
> Subject: [PATCH] fixup for "cpumask: drop cpumask_next_wrap_old()"
> 
> interacting with commit
> 
>   dd871ac1237f ("arm64: Provide an AMU-based version of arch_freq_get_on_cpu")
> 
> from the arm64 tree.
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  arch/arm64/kernel/topology.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> index 6f0cab8e746b..70db234c41a2 100644
> --- a/arch/arm64/kernel/topology.c
> +++ b/arch/arm64/kernel/topology.c
> @@ -231,7 +231,6 @@ void arch_cpu_idle_enter(void)
>  int arch_freq_get_on_cpu(int cpu)
>  {
>  	struct amu_cntr_sample *amu_sample;
> -	unsigned int start_cpu = cpu;
>  	unsigned long last_update;
>  	unsigned int freq = 0;
>  	u64 scale;
> @@ -267,8 +266,7 @@ int arch_freq_get_on_cpu(int cpu)
>  
>  
>  			do {
> -				ref_cpu = cpumask_next_wrap(ref_cpu, policy->cpus,
> -							    start_cpu, false);
> +				ref_cpu = cpumask_next_wrap(ref_cpu, policy->cpus);
>  
>  			} while (ref_cpu < nr_cpu_ids && idle_cpu(ref_cpu));
>  
> -- 
> 2.45.2
> 
> -- 
> Cheers,
> Stephen Rothwell

Hi Beata,

On Tue, 18 Feb 2025 11:35:02 +0100 Beata Michalska <beata.michalska@arm.com> wrote:
>
> I'm currently testing a proper fix for that one.
> Should I just send it over as a diff to apply or rather a proper 'fixes' patch?

Maybe a proper 'fixes' patch, please, if easy - otherwise a diff is
fine.

-- 
Cheers,
Stephen Rothwell

Hi Stephen,

On Wed, Feb 19, 2025 at 12:49:34AM +1100, Stephen Rothwell wrote:
> On Tue, 18 Feb 2025 11:35:02 +0100 Beata Michalska <beata.michalska@arm.com> wrote:
> > I'm currently testing a proper fix for that one.
> > Should I just send it over as a diff to apply or rather a proper 'fixes' patch?
> 
> Maybe a proper 'fixes' patch, please, if easy - otherwise a diff is
> fine.

I just talked to Beata off-list. I think she'll try to use the current
for_each_cpu_wrap() API and avoid conflicts with the cpumask_next_wrap()
API change.

If that doesn't work, you either carry a patch in -next until both
branches end up upstream or I merge a stable bitmap branch from Yury
with a fix on top.

Thanks.

-- 
Catalin

On Tue, Feb 18, 2025 at 02:10:25PM +0000, Catalin Marinas wrote:
> Hi Stephen,
> 
> On Wed, Feb 19, 2025 at 12:49:34AM +1100, Stephen Rothwell wrote:
> > On Tue, 18 Feb 2025 11:35:02 +0100 Beata Michalska <beata.michalska@arm.com> wrote:
> > > I'm currently testing a proper fix for that one.
> > > Should I just send it over as a diff to apply or rather a proper 'fixes' patch?
> > 
> > Maybe a proper 'fixes' patch, please, if easy - otherwise a diff is
> > fine.
> 
> I just talked to Beata off-list. I think she'll try to use the current
> for_each_cpu_wrap() API and avoid conflicts with the cpumask_next_wrap()
> API change.

Hi,

Yes, for_each() loops are always preferable over opencoded iterating.
Please feel free to CC me in case I can help.

Thanks,
Yury

On Tue, Feb 18, 2025 at 09:16:34AM -0500, Yury Norov wrote:
> On Tue, Feb 18, 2025 at 02:10:25PM +0000, Catalin Marinas wrote:
> > Hi Stephen,
> > 
> > On Wed, Feb 19, 2025 at 12:49:34AM +1100, Stephen Rothwell wrote:
> > > On Tue, 18 Feb 2025 11:35:02 +0100 Beata Michalska <beata.michalska@arm.com> wrote:
> > > > I'm currently testing a proper fix for that one.
> > > > Should I just send it over as a diff to apply or rather a proper 'fixes' patch?
> > > 
> > > Maybe a proper 'fixes' patch, please, if easy - otherwise a diff is
> > > fine.
> > 
> > I just talked to Beata off-list. I think she'll try to use the current
> > for_each_cpu_wrap() API and avoid conflicts with the cpumask_next_wrap()
> > API change.
> 
> Hi,
> 
> Yes, for_each() loops are always preferable over opencoded iterating.
> Please feel free to CC me in case I can help.

Beata is going to post the official fix but in the meantime, to avoid
breaking next, I'll add my temporary fix:

--------8<--------------------------------
From 1b12139107798128c183838c5f4a3f7ffcea1e44 Mon Sep 17 00:00:00 2001
From: Catalin Marinas <catalin.marinas@arm.com>
Date: Tue, 18 Feb 2025 18:20:46 +0000
Subject: [PATCH] arm64: Do not use the deprecated cpumask_next_wrap() in
 arch_freq_get_on_cpu()

cpumask_next_wrap() will soon disappear in its current form. Use
for_each_cpu_wrap() instead.

Fixes: 16d1e27475f6 ("arm64: Provide an AMU-based version of arch_freq_get_on_cpu")
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/kernel/topology.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index a09b0551ec59..1544d3648554 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -254,7 +254,7 @@ int arch_freq_get_on_cpu(int cpu)
 		if (!housekeeping_cpu(cpu, HK_TYPE_TICK) ||
 		    time_is_before_jiffies(last_update + msecs_to_jiffies(AMU_SAMPLE_EXP_MS))) {
 			struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
-			int ref_cpu = cpu;
+			int ref_cpu;

 			if (!policy)
 				return -EINVAL;
@@ -265,11 +265,10 @@ int arch_freq_get_on_cpu(int cpu)
 				return -EOPNOTSUPP;
 			}

-			do {
-				ref_cpu = cpumask_next_wrap(ref_cpu, policy->cpus,
-							    start_cpu, true);
-
-			} while (ref_cpu < nr_cpu_ids && idle_cpu(ref_cpu));
+			for_each_cpu_wrap(ref_cpu, policy->cpus, start_cpu) {
+				if (!idle_cpu(ref_cpu))
+					break;
+			}

 			cpufreq_cpu_put(policy);

On Tue, Feb 18, 2025 at 06:23:29PM +0000, Catalin Marinas wrote:
> On Tue, Feb 18, 2025 at 09:16:34AM -0500, Yury Norov wrote:
> > On Tue, Feb 18, 2025 at 02:10:25PM +0000, Catalin Marinas wrote:
> > > Hi Stephen,
> > > 
> > > On Wed, Feb 19, 2025 at 12:49:34AM +1100, Stephen Rothwell wrote:
> > > > On Tue, 18 Feb 2025 11:35:02 +0100 Beata Michalska <beata.michalska@arm.com> wrote:
> > > > > I'm currently testing a proper fix for that one.
> > > > > Should I just send it over as a diff to apply or rather a proper 'fixes' patch?
> > > > 
> > > > Maybe a proper 'fixes' patch, please, if easy - otherwise a diff is
> > > > fine.
> > > 
> > > I just talked to Beata off-list. I think she'll try to use the current
> > > for_each_cpu_wrap() API and avoid conflicts with the cpumask_next_wrap()
> > > API change.
> > 
> > Hi,
> > 
> > Yes, for_each() loops are always preferable over opencoded iterating.
> > Please feel free to CC me in case I can help.
> 
> Beata is going to post the official fix but in the meantime, to avoid
> breaking next, I'll add my temporary fix:
>
Just posted the fix [1].
Thank you all.

---
[1] https://lore.kernel.org/linux-next/20250218192412.2072619-1-beata.michalska@arm.com/T/#u

---
BR
Beata
> --------8<--------------------------------
> From 1b12139107798128c183838c5f4a3f7ffcea1e44 Mon Sep 17 00:00:00 2001
> From: Catalin Marinas <catalin.marinas@arm.com>
> Date: Tue, 18 Feb 2025 18:20:46 +0000
> Subject: [PATCH] arm64: Do not use the deprecated cpumask_next_wrap() in
>  arch_freq_get_on_cpu()
> 
> cpumask_next_wrap() will soon disappear in its current form. Use
> for_each_cpu_wrap() instead.
> 
> Fixes: 16d1e27475f6 ("arm64: Provide an AMU-based version of arch_freq_get_on_cpu")
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
>  arch/arm64/kernel/topology.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> index a09b0551ec59..1544d3648554 100644
> --- a/arch/arm64/kernel/topology.c
> +++ b/arch/arm64/kernel/topology.c
> @@ -254,7 +254,7 @@ int arch_freq_get_on_cpu(int cpu)
>  		if (!housekeeping_cpu(cpu, HK_TYPE_TICK) ||
>  		    time_is_before_jiffies(last_update + msecs_to_jiffies(AMU_SAMPLE_EXP_MS))) {
>  			struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
> -			int ref_cpu = cpu;
> +			int ref_cpu;
> 
>  			if (!policy)
>  				return -EINVAL;
> @@ -265,11 +265,10 @@ int arch_freq_get_on_cpu(int cpu)
>  				return -EOPNOTSUPP;
>  			}
> 
> -			do {
> -				ref_cpu = cpumask_next_wrap(ref_cpu, policy->cpus,
> -							    start_cpu, true);
> -
> -			} while (ref_cpu < nr_cpu_ids && idle_cpu(ref_cpu));
> +			for_each_cpu_wrap(ref_cpu, policy->cpus, start_cpu) {
> +				if (!idle_cpu(ref_cpu))
> +					break;
> +			}
> 
>  			cpufreq_cpu_put(policy);
> 
>

On Tue, Feb 18, 2025 at 08:28:56PM +0100, Beata Michalska wrote:
> On Tue, Feb 18, 2025 at 06:23:29PM +0000, Catalin Marinas wrote:
> > On Tue, Feb 18, 2025 at 09:16:34AM -0500, Yury Norov wrote:
> > > On Tue, Feb 18, 2025 at 02:10:25PM +0000, Catalin Marinas wrote:
> > > > Hi Stephen,
> > > > 
> > > > On Wed, Feb 19, 2025 at 12:49:34AM +1100, Stephen Rothwell wrote:
> > > > > On Tue, 18 Feb 2025 11:35:02 +0100 Beata Michalska <beata.michalska@arm.com> wrote:
> > > > > > I'm currently testing a proper fix for that one.
> > > > > > Should I just send it over as a diff to apply or rather a proper 'fixes' patch?
> > > > > 
> > > > > Maybe a proper 'fixes' patch, please, if easy - otherwise a diff is
> > > > > fine.
> > > > 
> > > > I just talked to Beata off-list. I think she'll try to use the current
> > > > for_each_cpu_wrap() API and avoid conflicts with the cpumask_next_wrap()
> > > > API change.
> > > 
> > > Hi,
> > > 
> > > Yes, for_each() loops are always preferable over opencoded iterating.
> > > Please feel free to CC me in case I can help.
> > 
> > Beata is going to post the official fix but in the meantime, to avoid
> > breaking next, I'll add my temporary fix:
> >
> Just posted the fix [1].
> Thank you all.
> 
> ---
> [1] https://lore.kernel.org/linux-next/20250218192412.2072619-1-beata.michalska@arm.com/T/#u

Great, thanks. I'll queue it tomorrow.

-- 
Catalin