[PATCH] sched: remove unused cpumask variable in mm_cid_get()

Kriish Sharma posted 1 patch 2 months, 1 week ago
kernel/sched/sched.h | 2 --
1 file changed, 2 deletions(-)
[PATCH] sched: remove unused cpumask variable in mm_cid_get()
Posted by Kriish Sharma 2 months, 1 week ago
The variable 'cpumask' in mm_cid_get() was assigned but never used,
causing the following build error with -Werror:

kernel/sched/sched.h: In function ‘mm_cid_get’:
kernel/sched/sched.h:3743:25: error: variable ‘cpumask’ set but not used [-Werror=unused-but-set-variable]
 3743 |         struct cpumask *cpumask;
      |                         ^~~~~~~

Removing the unused variable allows the kernel to compile without errors.

Signed-off-by: Kriish Sharma <kriish.sharma2006@gmail.com>
---
 kernel/sched/sched.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 1f5d07067f60..361f9101cef9 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -3740,11 +3740,9 @@ static inline int mm_cid_get(struct rq *rq, struct task_struct *t,
 			     struct mm_struct *mm)
 {
 	struct mm_cid __percpu *pcpu_cid = mm->pcpu_cid;
-	struct cpumask *cpumask;
 	int cid;
 
 	lockdep_assert_rq_held(rq);
-	cpumask = mm_cidmask(mm);
 	cid = __this_cpu_read(pcpu_cid->cid);
 	if (mm_cid_is_valid(cid)) {
 		mm_cid_snapshot_time(rq, mm);
-- 
2.34.1

Re: [PATCH] sched: remove unused cpumask variable in mm_cid_get()
Posted by Peter Zijlstra 2 months ago
On Thu, Oct 09, 2025 at 07:48:18PM +0000, Kriish Sharma wrote:
> The variable 'cpumask' in mm_cid_get() was assigned but never used,
> causing the following build error with -Werror:
> 
> kernel/sched/sched.h: In function ‘mm_cid_get’:
> kernel/sched/sched.h:3743:25: error: variable ‘cpumask’ set but not used [-Werror=unused-but-set-variable]
>  3743 |         struct cpumask *cpumask;
>       |                         ^~~~~~~
> 
> Removing the unused variable allows the kernel to compile without errors.

How come I don't see build errors myself?

Anyway, Thomas is in the process or rewriting all this, I'm not inclined
to bother with this unless the build robots actually complain.

Also, you're missing a Fixes tag or something, some commit must've
caused this.

> Signed-off-by: Kriish Sharma <kriish.sharma2006@gmail.com>
> ---
>  kernel/sched/sched.h | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 1f5d07067f60..361f9101cef9 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -3740,11 +3740,9 @@ static inline int mm_cid_get(struct rq *rq, struct task_struct *t,
>  			     struct mm_struct *mm)
>  {
>  	struct mm_cid __percpu *pcpu_cid = mm->pcpu_cid;
> -	struct cpumask *cpumask;
>  	int cid;
>  
>  	lockdep_assert_rq_held(rq);
> -	cpumask = mm_cidmask(mm);
>  	cid = __this_cpu_read(pcpu_cid->cid);
>  	if (mm_cid_is_valid(cid)) {
>  		mm_cid_snapshot_time(rq, mm);
> -- 
> 2.34.1
> 
Re: [PATCH] sched: remove unused cpumask variable in mm_cid_get()
Posted by Breno Leitao 2 months ago
On Thu, Oct 09, 2025 at 07:48:18PM +0000, Kriish Sharma wrote:
> The variable 'cpumask' in mm_cid_get() was assigned but never used,
> causing the following build error with -Werror:
> 
> kernel/sched/sched.h: In function ‘mm_cid_get’:
> kernel/sched/sched.h:3743:25: error: variable ‘cpumask’ set but not used [-Werror=unused-but-set-variable]
>  3743 |         struct cpumask *cpumask;
>       |                         ^~~~~~~

Thanks for the fix. I am hitting the same issue in my builds.

> Removing the unused variable allows the kernel to compile without errors.
> 
> Signed-off-by: Kriish Sharma <kriish.sharma2006@gmail.com>

Reviewed-by: Breno Leitao <leitao@debian.org>
Re: [PATCH] sched: remove unused cpumask variable in mm_cid_get()
Posted by Vlastimil Babka 2 months ago
On 10/14/25 11:56, Breno Leitao wrote:
> On Thu, Oct 09, 2025 at 07:48:18PM +0000, Kriish Sharma wrote:
>> The variable 'cpumask' in mm_cid_get() was assigned but never used,
>> causing the following build error with -Werror:
>> 
>> kernel/sched/sched.h: In function ‘mm_cid_get’:
>> kernel/sched/sched.h:3743:25: error: variable ‘cpumask’ set but not used [-Werror=unused-but-set-variable]
>>  3743 |         struct cpumask *cpumask;
>>       |                         ^~~~~~~
> 
> Thanks for the fix. I am hitting the same issue in my builds.

Let me add why this years old small issue became much more problematic in
6.18-rc1. When I want to test my own files I'm developing on with e.g. "make
W=1 mm/slub.o", the W=1 hits earlier in:

  CC      kernel/sched/rq-offsets.s
In file included from kernel/sched/rq-offsets.c:5:
kernel/sched/sched.h:3718:18: error: variable 'cpumask' set but not used
[-Werror,-Wunused-but-set-variable]
 3718 |         struct cpumask *cpumask;
      |                         ^
1 error generated.
make[2]: *** [scripts/Makefile.build:182: kernel/sched/rq-offsets.s] Error 1

So I can't get to the part where I test-compile my own code with W=1. So
fixing this ASAP in 6.18 would be appreciated, thanks!

FWIW I've bisected this to commit
378b7708194f ("sched: Make migrate_{en,dis}able() inline")

>> Removing the unused variable allows the kernel to compile without errors.
>> 
>> Signed-off-by: Kriish Sharma <kriish.sharma2006@gmail.com>
> 
> Reviewed-by: Breno Leitao <leitao@debian.org>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

Re: [PATCH] sched: remove unused cpumask variable in mm_cid_get()
Posted by Peter Zijlstra 2 months ago
On Tue, Oct 14, 2025 at 12:13:23PM +0200, Vlastimil Babka wrote:
> On 10/14/25 11:56, Breno Leitao wrote:
> > On Thu, Oct 09, 2025 at 07:48:18PM +0000, Kriish Sharma wrote:
> >> The variable 'cpumask' in mm_cid_get() was assigned but never used,
> >> causing the following build error with -Werror:
> >> 
> >> kernel/sched/sched.h: In function ‘mm_cid_get’:
> >> kernel/sched/sched.h:3743:25: error: variable ‘cpumask’ set but not used [-Werror=unused-but-set-variable]
> >>  3743 |         struct cpumask *cpumask;
> >>       |                         ^~~~~~~
> > 
> > Thanks for the fix. I am hitting the same issue in my builds.
> 
> Let me add why this years old small issue became much more problematic in
> 6.18-rc1. When I want to test my own files I'm developing on with e.g. "make
> W=1 mm/slub.o", the W=1 hits earlier in:
> 
>   CC      kernel/sched/rq-offsets.s
> In file included from kernel/sched/rq-offsets.c:5:
> kernel/sched/sched.h:3718:18: error: variable 'cpumask' set but not used
> [-Werror,-Wunused-but-set-variable]
>  3718 |         struct cpumask *cpumask;
>       |                         ^
> 1 error generated.
> make[2]: *** [scripts/Makefile.build:182: kernel/sched/rq-offsets.s] Error 1
> 
> So I can't get to the part where I test-compile my own code with W=1. So
> fixing this ASAP in 6.18 would be appreciated, thanks!
> 
> FWIW I've bisected this to commit
> 378b7708194f ("sched: Make migrate_{en,dis}able() inline")

People using W=1 and WERROR can keep the pieces. Anyway, this is a much
more coherent explanation that the original patch.
Re: [PATCH] sched: remove unused cpumask variable in mm_cid_get()
Posted by Uwe Kleine-König 1 month, 4 weeks ago
Hello Peter,

On Tue, Oct 14, 2025 at 12:34:39PM +0200, Peter Zijlstra wrote:
> On Tue, Oct 14, 2025 at 12:13:23PM +0200, Vlastimil Babka wrote:
> > On 10/14/25 11:56, Breno Leitao wrote:
> > > On Thu, Oct 09, 2025 at 07:48:18PM +0000, Kriish Sharma wrote:
> > >> The variable 'cpumask' in mm_cid_get() was assigned but never used,
> > >> causing the following build error with -Werror:
> > >> 
> > >> kernel/sched/sched.h: In function ‘mm_cid_get’:
> > >> kernel/sched/sched.h:3743:25: error: variable ‘cpumask’ set but not used [-Werror=unused-but-set-variable]
> > >>  3743 |         struct cpumask *cpumask;
> > >>       |                         ^~~~~~~
> > > 
> > > Thanks for the fix. I am hitting the same issue in my builds.
> > 
> > Let me add why this years old small issue became much more problematic in
> > 6.18-rc1. When I want to test my own files I'm developing on with e.g. "make
> > W=1 mm/slub.o", the W=1 hits earlier in:
> > 
> >   CC      kernel/sched/rq-offsets.s
> > In file included from kernel/sched/rq-offsets.c:5:
> > kernel/sched/sched.h:3718:18: error: variable 'cpumask' set but not used
> > [-Werror,-Wunused-but-set-variable]
> >  3718 |         struct cpumask *cpumask;
> >       |                         ^
> > 1 error generated.
> > make[2]: *** [scripts/Makefile.build:182: kernel/sched/rq-offsets.s] Error 1
> > 
> > So I can't get to the part where I test-compile my own code with W=1. So
> > fixing this ASAP in 6.18 would be appreciated, thanks!
> > 
> > FWIW I've bisected this to commit
> > 378b7708194f ("sched: Make migrate_{en,dis}able() inline")

There are several other submissions of the same patch with different
commit logs; I found:

https://lore.kernel.org/all/20251002-sched-w1-v1-1-a6fdf549d179@linaro.org/
https://lore.kernel.org/all/20251009194818.1587650-1-kriish.sharma2006@gmail.com/
https://lore.kernel.org/all/20251015091935.2977229-1-andriy.shevchenko@linux.intel.com/
https://lore.kernel.org/all/20251020221728.177983-1-adigollamudi@gmail.com/
https://lore.kernel.org/all/20251017073050.2411988-1-kevin.brodsky@arm.com/ 

Also Krzysztof's build bot is very unhappy:
https://krzk.eu/#/builders/135

> People using W=1 and WERROR can keep the pieces. Anyway, this is a much
> more coherent explanation that the original patch.

Can we please get this fixed even though you don't bother about W=1
builds? There seem to be others who do. And note that even

	make W=1 drivers/pwm/

is broken due to that, so it affects also maintainers who only want W=1
for their own subtree.

Regarding the Fixes line: Vlastimil Babka bisected it to 378b7708194f
("sched: Make migrate_{en,dis}able() inline"), but I think this is just
the commit that made the compiler notice that. IMHO Andy identified the
more plausible commit with:

Fixes: 223baf9d17f2 ("sched: Fix performance regression introduced by mm_cid")

.

Note there is a lkp report about André's patch (i.e. the first in my
list above) at
https://lore.kernel.org/all/202510041546.DvhFLp2x-lkp@intel.com/#t. I
don't understand the issue found there, but maybe someone should before
the patch is applied.

Best regards
Uwe
Re: [PATCH] sched: remove unused cpumask variable in mm_cid_get()
Posted by Peter Zijlstra 1 month, 4 weeks ago
On Tue, Oct 21, 2025 at 02:01:42PM +0200, Uwe Kleine-König wrote:

> There are several other submissions of the same patch with different
> commit logs; I found:
> 
> https://lore.kernel.org/all/20251002-sched-w1-v1-1-a6fdf549d179@linaro.org/
> https://lore.kernel.org/all/20251009194818.1587650-1-kriish.sharma2006@gmail.com/
> https://lore.kernel.org/all/20251015091935.2977229-1-andriy.shevchenko@linux.intel.com/
> https://lore.kernel.org/all/20251020221728.177983-1-adigollamudi@gmail.com/
> https://lore.kernel.org/all/20251017073050.2411988-1-kevin.brodsky@arm.com/ 

I know right, I seem to be getting at least one a day. If only people
were as good in testing -next I suppose. It also shows people can't be
arsed to search the archive :/

> Also Krzysztof's build bot is very unhappy:
> https://krzk.eu/#/builders/135

Not familiar with that thing.

> > People using W=1 and WERROR can keep the pieces. Anyway, this is a much
> > more coherent explanation that the original patch.
> 
> Can we please get this fixed even though you don't bother about W=1
> builds? There seem to be others who do. And note that even
> 
> 	make W=1 drivers/pwm/
> 
> is broken due to that, so it affects also maintainers who only want W=1
> for their own subtree.

Only if you have WERROR=y, which really you shouldn't have if you use
W>0.

> Regarding the Fixes line: Vlastimil Babka bisected it to 378b7708194f
> ("sched: Make migrate_{en,dis}able() inline"), but I think this is just
> the commit that made the compiler notice that. IMHO Andy identified the
> more plausible commit with:
> 
> Fixes: 223baf9d17f2 ("sched: Fix performance regression introduced by mm_cid")

Right, as said, Thomas is rewriting all that. His first patch is a
revert of that commit:

  https://lkml.kernel.org/r/20251015164952.694882104@linutronix.de

> Note there is a lkp report about André's patch (i.e. the first in my
> list above) at
> https://lore.kernel.org/all/202510041546.DvhFLp2x-lkp@intel.com/#t. I
> don't understand the issue found there, but maybe someone should before
> the patch is applied.

That's unrelated to the patch in question -- it is the robot
re-reporting a smatch thing because the code changed and the new report
no longer exactly matches the old report or something.

smatch wasn't able to discover the relation between next->mm and
next->mm_cid_active and warns us that next->mm can be NULL (per a
previous test for that) and that feeding said NULL into mm_cid_get() is
a problem -- it would be, except next->mm_cid_active cannot be set if
!next->mm.

*sigh*, it just means Thomas will have to rebase his series -- not the
end of the world I suppose but I really don't get this obsession with
W=1.

The problem is really that I'm now mandated to keep the scheduler W=1
clean, and I really, as in *really* don't care for W=1. A number of
warnings there are just not sane, like that ludicrous unused static
inline warning.

But sure -- send a patch for this, with a coherent changelog. I'll be a
bigger pain in the arse the moment the 'fix' really doesn't make sense.
I'll probably propose removing the warnings from W=1, like here:

  https://lkml.kernel.org/r/20250813152142.GP4067720@noisy.programming.kicks-ass.net
Re: [PATCH] sched: remove unused cpumask variable in mm_cid_get()
Posted by Krzysztof Kozlowski 1 month, 4 weeks ago
On 21/10/2025 15:30, Peter Zijlstra wrote:
>>> People using W=1 and WERROR can keep the pieces. Anyway, this is a much
>>> more coherent explanation that the original patch.
>>
>> Can we please get this fixed even though you don't bother about W=1
>> builds? There seem to be others who do. And note that even
>>
>> 	make W=1 drivers/pwm/
>>
>> is broken due to that, so it affects also maintainers who only want W=1
>> for their own subtree.
> 
> Only if you have WERROR=y, which really you shouldn't have if you use
> W>0.


That's not correct. I want my code to have zero W=1 errors and way I
achieved that is that I fixed all the warnings and now I build code I
maintain with allyesconfig (so werror) and W=1. That's way I am sure no
new warnings will slip into subsystem I maintain.

I recommend other maintainers also not to introduce W=1 errors and don't
accept patches who introduce them, even if that patch (like in this
case) just re-shuffled things and made some non important issue visible.

Why? Because we anyway do not want W=1 errors in RC release...

> 
>> Regarding the Fixes line: Vlastimil Babka bisected it to 378b7708194f
>> ("sched: Make migrate_{en,dis}able() inline"), but I think this is just
>> the commit that made the compiler notice that. IMHO Andy identified the
>> more plausible commit with:
>>
>> Fixes: 223baf9d17f2 ("sched: Fix performance regression introduced by mm_cid")
> 
> Right, as said, Thomas is rewriting all that. His first patch is a
> revert of that commit:
> 
>   https://lkml.kernel.org/r/20251015164952.694882104@linutronix.de
> 
>> Note there is a lkp report about André's patch (i.e. the first in my
>> list above) at
>> https://lore.kernel.org/all/202510041546.DvhFLp2x-lkp@intel.com/#t. I
>> don't understand the issue found there, but maybe someone should before
>> the patch is applied.
> 
> That's unrelated to the patch in question -- it is the robot
> re-reporting a smatch thing because the code changed and the new report
> no longer exactly matches the old report or something.
> 
> smatch wasn't able to discover the relation between next->mm and
> next->mm_cid_active and warns us that next->mm can be NULL (per a
> previous test for that) and that feeding said NULL into mm_cid_get() is
> a problem -- it would be, except next->mm_cid_active cannot be set if
> !next->mm.
> 
> *sigh*, it just means Thomas will have to rebase his series -- not the
> end of the world I suppose but I really don't get this obsession with
> W=1.
> 
> The problem is really that I'm now mandated to keep the scheduler W=1
> clean, and I really, as in *really* don't care for W=1. A number of

Great, so since your code is quite important and I cannot build my code
without that part, we are all stuck because you want W=0 compliance...

> warnings there are just not sane, like that ludicrous unused static
> inline warning.

That ludicrous warning could have been fixed when it hit next, because
it was not hiding from you. Building with W=1 is pretty standard thing
for new code. The truth is that that commit was applied shortly - few
days before merge window:
1. my earliest report comes from 26th of September
https://krzk.eu/#/builders/135/builds/180

2. Merge window opens Sep 28.

Two days in next, great!

> 
> But sure -- send a patch for this, with a coherent changelog. I'll be a
> bigger pain in the arse the moment the 'fix' really doesn't make sense.
> I'll probably propose removing the warnings from W=1, like here:
> 
>   https://lkml.kernel.org/r/20250813152142.GP4067720@noisy.programming.kicks-ass.net

That is fine as well. Your subsystem, your call, but unfortunately we
cannot build our stuff when yours fails.


Best regards,
Krzysztof