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.
On Mon, Oct 26, 2020 at 5:58 PM Peter Zijlstra <[email protected]> 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
On 28/10/2020 00.34, Arnd Bergmann wrote:
> On Mon, Oct 26, 2020 at 5:58 PM Peter Zijlstra <[email protected]> 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
On Wed, Oct 28, 2020 at 12:34:10AM +0100, Arnd Bergmann wrote:
> On Mon, Oct 26, 2020 at 5:58 PM Peter Zijlstra <[email protected]> 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.