* [PATCH] seqlock: avoid -Wshadow warnings
@ 2020-10-26 16:50 Arnd Bergmann
2020-10-26 16:58 ` Peter Zijlstra
2020-12-03 10:35 ` [tip: locking/core] " tip-bot2 for Arnd Bergmann
0 siblings, 2 replies; 6+ messages in thread
From: Arnd Bergmann @ 2020-10-26 16:50 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Will Deacon, Ahmed S. Darwish
Cc: Arnd Bergmann, Paul E. McKenney, Marco Elver, linux-kernel
From: Arnd Bergmann <arnd@arndb.de>
When building with W=2, there is a flood of warnings about the seqlock
macros shadowing local variables:
19806 linux/seqlock.h:331:11: warning: declaration of 'seq' shadows a previous local [-Wshadow]
48 linux/seqlock.h:348:11: warning: declaration of 'seq' shadows a previous local [-Wshadow]
8 linux/seqlock.h:379:11: warning: declaration of 'seq' shadows a previous local [-Wshadow]
Prefix the local variables to make the warning useful elsewhere again.
Fixes: 52ac39e5db51 ("seqlock: seqcount_t: Implement all read APIs as statement expressions")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
include/linux/seqlock.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h
index cbfc78b92b65..8d8552474c64 100644
--- a/include/linux/seqlock.h
+++ b/include/linux/seqlock.h
@@ -328,13 +328,13 @@ SEQCOUNT_LOCKNAME(ww_mutex, struct ww_mutex, true, &s->lock->base, ww_mu
*/
#define __read_seqcount_begin(s) \
({ \
- unsigned seq; \
+ unsigned __seq; \
\
- while ((seq = __seqcount_sequence(s)) & 1) \
+ while ((__seq = __seqcount_sequence(s)) & 1) \
cpu_relax(); \
\
kcsan_atomic_next(KCSAN_SEQLOCK_REGION_MAX); \
- seq; \
+ __seq; \
})
/**
@@ -345,10 +345,10 @@ SEQCOUNT_LOCKNAME(ww_mutex, struct ww_mutex, true, &s->lock->base, ww_mu
*/
#define raw_read_seqcount_begin(s) \
({ \
- unsigned seq = __read_seqcount_begin(s); \
+ unsigned _seq = __read_seqcount_begin(s); \
\
smp_rmb(); \
- seq; \
+ _seq; \
})
/**
@@ -376,11 +376,11 @@ SEQCOUNT_LOCKNAME(ww_mutex, struct ww_mutex, true, &s->lock->base, ww_mu
*/
#define raw_read_seqcount(s) \
({ \
- unsigned seq = __seqcount_sequence(s); \
+ unsigned __seq = __seqcount_sequence(s); \
\
smp_rmb(); \
kcsan_atomic_next(KCSAN_SEQLOCK_REGION_MAX); \
- seq; \
+ __seq; \
})
/**
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] seqlock: avoid -Wshadow warnings
2020-10-26 16:50 [PATCH] seqlock: avoid -Wshadow warnings Arnd Bergmann
@ 2020-10-26 16:58 ` Peter Zijlstra
2020-10-27 23:34 ` Arnd Bergmann
2020-12-03 10:35 ` [tip: locking/core] " tip-bot2 for Arnd Bergmann
1 sibling, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2020-10-26 16:58 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Ingo Molnar, Will Deacon, Ahmed S. Darwish, Arnd Bergmann,
Paul E. McKenney, Marco Elver, linux-kernel
On Mon, Oct 26, 2020 at 05:50:38PM +0100, Arnd Bergmann wrote:
> - unsigned seq; \
> + unsigned __seq; \
> - unsigned seq = __read_seqcount_begin(s); \
> + unsigned _seq = __read_seqcount_begin(s); \
> - unsigned seq = __seqcount_sequence(s); \
> + unsigned __seq = __seqcount_sequence(s); \
Can we have a consistent number of leading '_' ?
Also, I suppose you're going to find the explicit shadow in
___wait_event(), that one's not going to be trivial to fix.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] seqlock: avoid -Wshadow warnings
2020-10-26 16:58 ` Peter Zijlstra
@ 2020-10-27 23:34 ` Arnd Bergmann
2020-10-28 7:51 ` Rasmus Villemoes
2020-10-28 8:51 ` Peter Zijlstra
0 siblings, 2 replies; 6+ messages in thread
From: Arnd Bergmann @ 2020-10-27 23:34 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Ingo Molnar, Will Deacon, Ahmed S. Darwish, Paul E. McKenney,
Marco Elver, linux-kernel@vger.kernel.org
On Mon, Oct 26, 2020 at 5:58 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Mon, Oct 26, 2020 at 05:50:38PM +0100, Arnd Bergmann wrote:
>
> > - unsigned seq; \
> > + unsigned __seq; \
>
> > - unsigned seq = __read_seqcount_begin(s); \
> > + unsigned _seq = __read_seqcount_begin(s); \
>
> > - unsigned seq = __seqcount_sequence(s); \
> > + unsigned __seq = __seqcount_sequence(s); \
>
> Can we have a consistent number of leading '_' ?
Not really ;-)
The warning comes from raw_read_seqcount_begin() calling
__read_seqcount_begin() and both using the same variable
name. I could rename one of them and use double-underscores
for both, but I haven't come up with a good alternative name
that wouldn't make it less consistent in the process.
> Also, I suppose you're going to find the explicit shadow in
> ___wait_event(), that one's not going to be trivial to fix.
I have this patch in my tree at the moment but did not send that yet
because that caused a regression on powerpc:
diff --git a/include/linux/wait.h b/include/linux/wait.h
index 57ccf26d3b96..5d00a6fb7154 100644
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -265,7 +265,11 @@ extern void init_wait_entry(struct
wait_queue_entry *wq_entry, int flags);
({
\
__label__ __out;
\
struct wait_queue_entry __wq_entry;
\
- long __ret = ret; /* explicit shadow */
\
+ __diag_push()
\
+ __diag_ignore(GCC, 8, "-Wshadow", "explicit shadow")
\
+ __diag_ignore(CLANG, 9, "-Wshadow", "explicit shadow")
\
+ long __ret = ret;
\
+ __diag_pop();
\
\
init_wait_entry(&__wq_entry, exclusive ? WQ_FLAG_EXCLUSIVE :
0); \
for (;;) {
\
Still looking at alternative approaches.
Arnd
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] seqlock: avoid -Wshadow warnings
2020-10-27 23:34 ` Arnd Bergmann
@ 2020-10-28 7:51 ` Rasmus Villemoes
2020-10-28 8:51 ` Peter Zijlstra
1 sibling, 0 replies; 6+ messages in thread
From: Rasmus Villemoes @ 2020-10-28 7:51 UTC (permalink / raw)
To: Arnd Bergmann, Peter Zijlstra
Cc: Ingo Molnar, Will Deacon, Ahmed S. Darwish, Paul E. McKenney,
Marco Elver, linux-kernel@vger.kernel.org
On 28/10/2020 00.34, Arnd Bergmann wrote:
> On Mon, Oct 26, 2020 at 5:58 PM Peter Zijlstra <peterz@infradead.org> wrote:
>>
>> On Mon, Oct 26, 2020 at 05:50:38PM +0100, Arnd Bergmann wrote:
>>
>>> - unsigned seq; \
>>> + unsigned __seq; \
>>
>>> - unsigned seq = __read_seqcount_begin(s); \
>>> + unsigned _seq = __read_seqcount_begin(s); \
>>
>>> - unsigned seq = __seqcount_sequence(s); \
>>> + unsigned __seq = __seqcount_sequence(s); \
>>
>> Can we have a consistent number of leading '_' ?
>
> Not really ;-)
>
> The warning comes from raw_read_seqcount_begin() calling
> __read_seqcount_begin() and both using the same variable
> name. I could rename one of them and use double-underscores
> for both, but I haven't come up with a good alternative name
> that wouldn't make it less consistent in the process.
At least x86's put_user and get_user use _pu/_gu suffixes on their local
variables, so perhaps that could be made a weak default convention?
__seq_rsb
__seq_rrsb
__seq_rrs
Hm, or perhaps not. But it's still better than triplicating each macro
to do a UNIQUE_ID dance.
Rasmus
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] seqlock: avoid -Wshadow warnings
2020-10-27 23:34 ` Arnd Bergmann
2020-10-28 7:51 ` Rasmus Villemoes
@ 2020-10-28 8:51 ` Peter Zijlstra
1 sibling, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2020-10-28 8:51 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Ingo Molnar, Will Deacon, Ahmed S. Darwish, Paul E. McKenney,
Marco Elver, linux-kernel@vger.kernel.org
On Wed, Oct 28, 2020 at 12:34:10AM +0100, Arnd Bergmann wrote:
> On Mon, Oct 26, 2020 at 5:58 PM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Mon, Oct 26, 2020 at 05:50:38PM +0100, Arnd Bergmann wrote:
> >
> > > - unsigned seq; \
> > > + unsigned __seq; \
> >
> > > - unsigned seq = __read_seqcount_begin(s); \
> > > + unsigned _seq = __read_seqcount_begin(s); \
> >
> > > - unsigned seq = __seqcount_sequence(s); \
> > > + unsigned __seq = __seqcount_sequence(s); \
> >
> > Can we have a consistent number of leading '_' ?
>
> Not really ;-)
Duh.. ok, I'll take it as is.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [tip: locking/core] seqlock: avoid -Wshadow warnings
2020-10-26 16:50 [PATCH] seqlock: avoid -Wshadow warnings Arnd Bergmann
2020-10-26 16:58 ` Peter Zijlstra
@ 2020-12-03 10:35 ` tip-bot2 for Arnd Bergmann
1 sibling, 0 replies; 6+ messages in thread
From: tip-bot2 for Arnd Bergmann @ 2020-12-03 10:35 UTC (permalink / raw)
To: linux-tip-commits
Cc: Arnd Bergmann, Peter Zijlstra (Intel), x86, linux-kernel
The following commit has been merged into the locking/core branch of tip:
Commit-ID: a07c45312f06e288417049208c344ad76074627d
Gitweb: https://git.kernel.org/tip/a07c45312f06e288417049208c344ad76074627d
Author: Arnd Bergmann <arnd@arndb.de>
AuthorDate: Mon, 26 Oct 2020 17:50:38 +01:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Thu, 03 Dec 2020 11:20:50 +01:00
seqlock: avoid -Wshadow warnings
When building with W=2, there is a flood of warnings about the seqlock
macros shadowing local variables:
19806 linux/seqlock.h:331:11: warning: declaration of 'seq' shadows a previous local [-Wshadow]
48 linux/seqlock.h:348:11: warning: declaration of 'seq' shadows a previous local [-Wshadow]
8 linux/seqlock.h:379:11: warning: declaration of 'seq' shadows a previous local [-Wshadow]
Prefix the local variables to make the warning useful elsewhere again.
Fixes: 52ac39e5db51 ("seqlock: seqcount_t: Implement all read APIs as statement expressions")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20201026165044.3722931-1-arnd@kernel.org
---
include/linux/seqlock.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h
index cbfc78b..8d85524 100644
--- a/include/linux/seqlock.h
+++ b/include/linux/seqlock.h
@@ -328,13 +328,13 @@ SEQCOUNT_LOCKNAME(ww_mutex, struct ww_mutex, true, &s->lock->base, ww_mu
*/
#define __read_seqcount_begin(s) \
({ \
- unsigned seq; \
+ unsigned __seq; \
\
- while ((seq = __seqcount_sequence(s)) & 1) \
+ while ((__seq = __seqcount_sequence(s)) & 1) \
cpu_relax(); \
\
kcsan_atomic_next(KCSAN_SEQLOCK_REGION_MAX); \
- seq; \
+ __seq; \
})
/**
@@ -345,10 +345,10 @@ SEQCOUNT_LOCKNAME(ww_mutex, struct ww_mutex, true, &s->lock->base, ww_mu
*/
#define raw_read_seqcount_begin(s) \
({ \
- unsigned seq = __read_seqcount_begin(s); \
+ unsigned _seq = __read_seqcount_begin(s); \
\
smp_rmb(); \
- seq; \
+ _seq; \
})
/**
@@ -376,11 +376,11 @@ SEQCOUNT_LOCKNAME(ww_mutex, struct ww_mutex, true, &s->lock->base, ww_mu
*/
#define raw_read_seqcount(s) \
({ \
- unsigned seq = __seqcount_sequence(s); \
+ unsigned __seq = __seqcount_sequence(s); \
\
smp_rmb(); \
kcsan_atomic_next(KCSAN_SEQLOCK_REGION_MAX); \
- seq; \
+ __seq; \
})
/**
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-03 10:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-26 16:50 [PATCH] seqlock: avoid -Wshadow warnings Arnd Bergmann
2020-10-26 16:58 ` Peter Zijlstra
2020-10-27 23:34 ` Arnd Bergmann
2020-10-28 7:51 ` Rasmus Villemoes
2020-10-28 8:51 ` Peter Zijlstra
2020-12-03 10:35 ` [tip: locking/core] " tip-bot2 for Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox