* [PATCH] sched: remove unused cpumask variable in mm_cid_get()
@ 2025-10-09 19:48 Kriish Sharma
2025-10-14 9:56 ` Breno Leitao
2025-10-14 10:31 ` Peter Zijlstra
0 siblings, 2 replies; 8+ messages in thread
From: Kriish Sharma @ 2025-10-09 19:48 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider
Cc: linux-kernel, david.hunter.linux, skhan, Kriish Sharma
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] sched: remove unused cpumask variable in mm_cid_get() 2025-10-09 19:48 [PATCH] sched: remove unused cpumask variable in mm_cid_get() Kriish Sharma @ 2025-10-14 9:56 ` Breno Leitao 2025-10-14 10:13 ` Vlastimil Babka 2025-10-14 10:31 ` Peter Zijlstra 1 sibling, 1 reply; 8+ messages in thread From: Breno Leitao @ 2025-10-14 9:56 UTC (permalink / raw) To: Kriish Sharma Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider, linux-kernel, david.hunter.linux, skhan 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> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sched: remove unused cpumask variable in mm_cid_get() 2025-10-14 9:56 ` Breno Leitao @ 2025-10-14 10:13 ` Vlastimil Babka 2025-10-14 10:34 ` Peter Zijlstra 0 siblings, 1 reply; 8+ messages in thread From: Vlastimil Babka @ 2025-10-14 10:13 UTC (permalink / raw) To: Breno Leitao, Kriish Sharma Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider, linux-kernel, david.hunter.linux, skhan, Menglong Dong 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> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sched: remove unused cpumask variable in mm_cid_get() 2025-10-14 10:13 ` Vlastimil Babka @ 2025-10-14 10:34 ` Peter Zijlstra 2025-10-21 12:01 ` Uwe Kleine-König 0 siblings, 1 reply; 8+ messages in thread From: Peter Zijlstra @ 2025-10-14 10:34 UTC (permalink / raw) To: Vlastimil Babka Cc: Breno Leitao, Kriish Sharma, Ingo Molnar, Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider, linux-kernel, david.hunter.linux, skhan, Menglong Dong 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sched: remove unused cpumask variable in mm_cid_get() 2025-10-14 10:34 ` Peter Zijlstra @ 2025-10-21 12:01 ` Uwe Kleine-König 2025-10-21 13:30 ` Peter Zijlstra 0 siblings, 1 reply; 8+ messages in thread From: Uwe Kleine-König @ 2025-10-21 12:01 UTC (permalink / raw) To: Peter Zijlstra, Thomas Gleixner Cc: Vlastimil Babka, Breno Leitao, Kriish Sharma, Ingo Molnar, Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider, linux-kernel, david.hunter.linux, skhan, Menglong Dong, André Draszik, Andy Shevchenko, Aditya Gollamudi, Kevin Brodsky, Krzysztof Kozlowski, Dan Carpenter [-- Attachment #1: Type: text/plain, Size: 3215 bytes --] 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sched: remove unused cpumask variable in mm_cid_get() 2025-10-21 12:01 ` Uwe Kleine-König @ 2025-10-21 13:30 ` Peter Zijlstra 2025-10-21 20:02 ` Krzysztof Kozlowski 0 siblings, 1 reply; 8+ messages in thread From: Peter Zijlstra @ 2025-10-21 13:30 UTC (permalink / raw) To: Uwe Kleine-König Cc: Thomas Gleixner, Vlastimil Babka, Breno Leitao, Kriish Sharma, Ingo Molnar, Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider, linux-kernel, david.hunter.linux, skhan, Menglong Dong, André Draszik, Andy Shevchenko, Aditya Gollamudi, Kevin Brodsky, Krzysztof Kozlowski, Dan Carpenter [-- Attachment #1: Type: text/plain, Size: 3299 bytes --] 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sched: remove unused cpumask variable in mm_cid_get() 2025-10-21 13:30 ` Peter Zijlstra @ 2025-10-21 20:02 ` Krzysztof Kozlowski 0 siblings, 0 replies; 8+ messages in thread From: Krzysztof Kozlowski @ 2025-10-21 20:02 UTC (permalink / raw) To: Peter Zijlstra, Uwe Kleine-König Cc: Thomas Gleixner, Vlastimil Babka, Breno Leitao, Kriish Sharma, Ingo Molnar, Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider, linux-kernel, david.hunter.linux, skhan, Menglong Dong, André Draszik, Andy Shevchenko, Aditya Gollamudi, Kevin Brodsky, Dan Carpenter 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sched: remove unused cpumask variable in mm_cid_get() 2025-10-09 19:48 [PATCH] sched: remove unused cpumask variable in mm_cid_get() Kriish Sharma 2025-10-14 9:56 ` Breno Leitao @ 2025-10-14 10:31 ` Peter Zijlstra 1 sibling, 0 replies; 8+ messages in thread From: Peter Zijlstra @ 2025-10-14 10:31 UTC (permalink / raw) To: Kriish Sharma, Thomas Gleixner Cc: Ingo Molnar, Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider, linux-kernel, david.hunter.linux, skhan 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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-10-21 20:02 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-10-09 19:48 [PATCH] sched: remove unused cpumask variable in mm_cid_get() Kriish Sharma 2025-10-14 9:56 ` Breno Leitao 2025-10-14 10:13 ` Vlastimil Babka 2025-10-14 10:34 ` Peter Zijlstra 2025-10-21 12:01 ` Uwe Kleine-König 2025-10-21 13:30 ` Peter Zijlstra 2025-10-21 20:02 ` Krzysztof Kozlowski 2025-10-14 10:31 ` Peter Zijlstra
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®