linux-next: build warning after merge of the rcu tree

linux-next: build warning after merge of the rcu tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 591 bytes --]

Hi all,

After merging the rcu (I think) tree, today's linux-next build (x86_64
allnoconfig) produced this warning:

kernel/time/timer.c: In function 'schedule_timeout':
kernel/time/timer.c:969:20: warning: 'timer.expires' may be used uninitialized in this function [-Wmaybe-uninitialized]
  969 |   long diff = timer->expires - expires;
      |               ~~~~~^~~~~~~~~

Introduced by (bisected to) commit

  c4127fce1d02 ("timer: Use hlist_unhashed_lockless() in timer_pending()")

x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build warning after merge of the rcu tree

[Paul E. McKenney]

On Thu, Dec 12, 2019 at 04:06:22PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the rcu (I think) tree, today's linux-next build (x86_64
> allnoconfig) produced this warning:
> 
> kernel/time/timer.c: In function 'schedule_timeout':
> kernel/time/timer.c:969:20: warning: 'timer.expires' may be used uninitialized in this function [-Wmaybe-uninitialized]
>   969 |   long diff = timer->expires - expires;
>       |               ~~~~~^~~~~~~~~
> 
> Introduced by (bisected to) commit
> 
>   c4127fce1d02 ("timer: Use hlist_unhashed_lockless() in timer_pending()")
> 
> x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130

Well, if the timer is pending, then ->expires has to have been
initialized, but off where the compiler cannot see it, such as during a
previous call to __mod_timer().  And the change may have made it harder
for the compiler to see all of these relationships, but...

I don't see this warning with gcc version 7.4.0.  Just out of curiosity,
what are you running, Stephen?

Eric, any thoughts for properly educating the compiler on this one?

							Thanx, Paul

Re: linux-next: build warning after merge of the rcu tree

[Eric Dumazet]

On Wed, Dec 11, 2019 at 10:02 PM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Thu, Dec 12, 2019 at 04:06:22PM +1100, Stephen Rothwell wrote:
> > Hi all,
> >
> > After merging the rcu (I think) tree, today's linux-next build (x86_64
> > allnoconfig) produced this warning:
> >
> > kernel/time/timer.c: In function 'schedule_timeout':
> > kernel/time/timer.c:969:20: warning: 'timer.expires' may be used uninitialized in this function [-Wmaybe-uninitialized]
> >   969 |   long diff = timer->expires - expires;
> >       |               ~~~~~^~~~~~~~~
> >
> > Introduced by (bisected to) commit
> >
> >   c4127fce1d02 ("timer: Use hlist_unhashed_lockless() in timer_pending()")
> >
> > x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130
>
> Well, if the timer is pending, then ->expires has to have been
> initialized, but off where the compiler cannot see it, such as during a
> previous call to __mod_timer().  And the change may have made it harder
> for the compiler to see all of these relationships, but...
>
> I don't see this warning with gcc version 7.4.0.  Just out of curiosity,
> what are you running, Stephen?
>
> Eric, any thoughts for properly educating the compiler on this one?

Ah... the READ_ONCE() apparently turns off the compiler ability to
infer that this branch should not be taken.

Since __mod_timer() is inlined we could perhaps add a new option

diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 4820823515e9..8bbce552568b 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -944,6 +944,7 @@ static struct timer_base *lock_timer_base(struct
timer_list *timer,

 #define MOD_TIMER_PENDING_ONLY         0x01
 #define MOD_TIMER_REDUCE               0x02
+#define MOD_TIMER_NOTPENDING           0x04

 static inline int
 __mod_timer(struct timer_list *timer, unsigned long expires, unsigned
int options)
@@ -960,7 +961,7 @@ __mod_timer(struct timer_list *timer, unsigned
long expires, unsigned int option
         * the timer is re-modified to have the same timeout or ends up in the
         * same array bucket then just return:
         */
-       if (timer_pending(timer)) {
+       if (!(options & MOD_TIMER_NOTPENDING) && timer_pending(timer)) {
                /*
                 * The downside of this optimization is that it can result in
                 * larger granularity than you would get from adding a new
@@ -1891,7 +1892,7 @@ signed long __sched schedule_timeout(signed long timeout)

        timer.task = current;
        timer_setup_on_stack(&timer.timer, process_timeout, 0);
-       __mod_timer(&timer.timer, expire, 0);
+       __mod_timer(&timer.timer, expire, MOD_TIMER_NOTPENDING);
        schedule();
        del_singleshot_timer_sync(&timer.timer);

Re: linux-next: build warning after merge of the rcu tree

[Eric Dumazet]

On Wed, Dec 11, 2019 at 10:38 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Wed, Dec 11, 2019 at 10:02 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Thu, Dec 12, 2019 at 04:06:22PM +1100, Stephen Rothwell wrote:
> > > Hi all,
> > >
> > > After merging the rcu (I think) tree, today's linux-next build (x86_64
> > > allnoconfig) produced this warning:
> > >
> > > kernel/time/timer.c: In function 'schedule_timeout':
> > > kernel/time/timer.c:969:20: warning: 'timer.expires' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > >   969 |   long diff = timer->expires - expires;
> > >       |               ~~~~~^~~~~~~~~
> > >
> > > Introduced by (bisected to) commit
> > >
> > >   c4127fce1d02 ("timer: Use hlist_unhashed_lockless() in timer_pending()")
> > >
> > > x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130
> >
> > Well, if the timer is pending, then ->expires has to have been
> > initialized, but off where the compiler cannot see it, such as during a
> > previous call to __mod_timer().  And the change may have made it harder
> > for the compiler to see all of these relationships, but...
> >
> > I don't see this warning with gcc version 7.4.0.  Just out of curiosity,
> > what are you running, Stephen?
> >
> > Eric, any thoughts for properly educating the compiler on this one?
>
> Ah... the READ_ONCE() apparently turns off the compiler ability to
> infer that this branch should not be taken.
>
> Since __mod_timer() is inlined we could perhaps add a new option
>
> diff --git a/kernel/time/timer.c b/kernel/time/timer.c
> index 4820823515e9..8bbce552568b 100644
> --- a/kernel/time/timer.c
> +++ b/kernel/time/timer.c
> @@ -944,6 +944,7 @@ static struct timer_base *lock_timer_base(struct
> timer_list *timer,
>
>  #define MOD_TIMER_PENDING_ONLY         0x01
>  #define MOD_TIMER_REDUCE               0x02
> +#define MOD_TIMER_NOTPENDING           0x04
>
>  static inline int
>  __mod_timer(struct timer_list *timer, unsigned long expires, unsigned
> int options)
> @@ -960,7 +961,7 @@ __mod_timer(struct timer_list *timer, unsigned
> long expires, unsigned int option
>          * the timer is re-modified to have the same timeout or ends up in the
>          * same array bucket then just return:
>          */
> -       if (timer_pending(timer)) {
> +       if (!(options & MOD_TIMER_NOTPENDING) && timer_pending(timer)) {
>                 /*
>                  * The downside of this optimization is that it can result in
>                  * larger granularity than you would get from adding a new
> @@ -1891,7 +1892,7 @@ signed long __sched schedule_timeout(signed long timeout)
>
>         timer.task = current;
>         timer_setup_on_stack(&timer.timer, process_timeout, 0);
> -       __mod_timer(&timer.timer, expire, 0);
> +       __mod_timer(&timer.timer, expire, MOD_TIMER_NOTPENDING);
>         schedule();
>         del_singleshot_timer_sync(&timer.timer);


Also add_timer() can benefit from the same hint, since it seems inlined as well.

(untested patch)

diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 4820823515e9..568564ae3597 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -944,6 +944,7 @@ static struct timer_base *lock_timer_base(struct
timer_list *timer,

 #define MOD_TIMER_PENDING_ONLY         0x01
 #define MOD_TIMER_REDUCE               0x02
+#define MOD_TIMER_NOTPENDING           0x04

 static inline int
 __mod_timer(struct timer_list *timer, unsigned long expires, unsigned
int options)
@@ -960,7 +961,7 @@ __mod_timer(struct timer_list *timer, unsigned
long expires, unsigned int option
         * the timer is re-modified to have the same timeout or ends up in the
         * same array bucket then just return:
         */
-       if (timer_pending(timer)) {
+       if (!(options & MOD_TIMER_NOTPENDING) && timer_pending(timer)) {
                /*
                 * The downside of this optimization is that it can result in
                 * larger granularity than you would get from adding a new
@@ -1133,7 +1134,7 @@ EXPORT_SYMBOL(timer_reduce);
 void add_timer(struct timer_list *timer)
 {
        BUG_ON(timer_pending(timer));
-       mod_timer(timer, timer->expires);
+       __mod_timer(timer, timer->expires, MOD_TIMER_NOTPENDING);
 }
 EXPORT_SYMBOL(add_timer);

@@ -1891,7 +1892,7 @@ signed long __sched schedule_timeout(signed long timeout)

        timer.task = current;
        timer_setup_on_stack(&timer.timer, process_timeout, 0);
-       __mod_timer(&timer.timer, expire, 0);
+       __mod_timer(&timer.timer, expire, MOD_TIMER_NOTPENDING);
        schedule();
        del_singleshot_timer_sync(&timer.timer);

Re: linux-next: build warning after merge of the rcu tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 408 bytes --]

Hi Paul,

On Wed, 11 Dec 2019 22:02:00 -0800 "Paul E. McKenney" <paulmck@kernel.org> wrote:
>
> On Thu, Dec 12, 2019 at 04:06:22PM +1100, Stephen Rothwell wrote:
> > 
> > x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130  
> 
> I don't see this warning with gcc version 7.4.0.  Just out of curiosity,
> what are you running, Stephen?

See above (9.2.1). ;-)

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build warning after merge of the rcu tree

[Paul E. McKenney]

On Thu, Dec 12, 2019 at 10:40:50PM +1100, Stephen Rothwell wrote:
> Hi Paul,
> 
> On Wed, 11 Dec 2019 22:02:00 -0800 "Paul E. McKenney" <paulmck@kernel.org> wrote:
> >
> > On Thu, Dec 12, 2019 at 04:06:22PM +1100, Stephen Rothwell wrote:
> > > 
> > > x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130  
> > 
> > I don't see this warning with gcc version 7.4.0.  Just out of curiosity,
> > what are you running, Stephen?
> 
> See above (9.2.1). ;-)

Color me blind.  :-/

							Thanx, Paul

Re: linux-next: build warning after merge of the rcu tree

[Olof Johansson]

Hi,

On Wed, Dec 11, 2019 at 9:06 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi all,
>
> After merging the rcu (I think) tree, today's linux-next build (x86_64
> allnoconfig) produced this warning:
>
> kernel/time/timer.c: In function 'schedule_timeout':
> kernel/time/timer.c:969:20: warning: 'timer.expires' may be used uninitialized in this function [-Wmaybe-uninitialized]
>   969 |   long diff = timer->expires - expires;
>       |               ~~~~~^~~~~~~~~
>
> Introduced by (bisected to) commit
>
>   c4127fce1d02 ("timer: Use hlist_unhashed_lockless() in timer_pending()")
>
> x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130

This is still there as of last night's -next. Any update on getting a
fix queued together with the offending patch?


Thanks!

-Olof

Re: linux-next: build warning after merge of the rcu tree

[Paul E. McKenney]

On Mon, Jan 06, 2020 at 09:51:47AM -0800, Olof Johansson wrote:
> Hi,
> 
> On Wed, Dec 11, 2019 at 9:06 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > Hi all,
> >
> > After merging the rcu (I think) tree, today's linux-next build (x86_64
> > allnoconfig) produced this warning:
> >
> > kernel/time/timer.c: In function 'schedule_timeout':
> > kernel/time/timer.c:969:20: warning: 'timer.expires' may be used uninitialized in this function [-Wmaybe-uninitialized]
> >   969 |   long diff = timer->expires - expires;
> >       |               ~~~~~^~~~~~~~~
> >
> > Introduced by (bisected to) commit
> >
> >   c4127fce1d02 ("timer: Use hlist_unhashed_lockless() in timer_pending()")
> >
> > x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130
> 
> This is still there as of last night's -next. Any update on getting a
> fix queued together with the offending patch?

Hello, Olof,

Thank you, I had indeed lost track of this one.  :-/

Does Eric's patch fix things for you?

https://lore.kernel.org/lkml/CANn89i+xomdo4HFqewrfNf_Z4Q5ayXuW6A4SjSkE46JXP9KuFw@mail.gmail.com/

							Thanx, Paul

Re: linux-next: build warning after merge of the rcu tree

[Olof Johansson]

On Mon, Jan 6, 2020 at 10:10 AM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Mon, Jan 06, 2020 at 09:51:47AM -0800, Olof Johansson wrote:
> > Hi,
> >
> > On Wed, Dec 11, 2019 at 9:06 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > >
> > > Hi all,
> > >
> > > After merging the rcu (I think) tree, today's linux-next build (x86_64
> > > allnoconfig) produced this warning:
> > >
> > > kernel/time/timer.c: In function 'schedule_timeout':
> > > kernel/time/timer.c:969:20: warning: 'timer.expires' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > >   969 |   long diff = timer->expires - expires;
> > >       |               ~~~~~^~~~~~~~~
> > >
> > > Introduced by (bisected to) commit
> > >
> > >   c4127fce1d02 ("timer: Use hlist_unhashed_lockless() in timer_pending()")
> > >
> > > x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130
> >
> > This is still there as of last night's -next. Any update on getting a
> > fix queued together with the offending patch?
>
> Hello, Olof,
>
> Thank you, I had indeed lost track of this one.  :-/
>
> Does Eric's patch fix things for you?
>
> https://lore.kernel.org/lkml/CANn89i+xomdo4HFqewrfNf_Z4Q5ayXuW6A4SjSkE46JXP9KuFw@mail.gmail.com/

It does, but it's not a proper patch (whitespace damage, no S-o-b, etc).


-Olof

Re: linux-next: build warning after merge of the rcu tree

[Paul E. McKenney]

On Mon, Jan 06, 2020 at 01:08:37PM -0800, Olof Johansson wrote:
> On Mon, Jan 6, 2020 at 10:10 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Mon, Jan 06, 2020 at 09:51:47AM -0800, Olof Johansson wrote:
> > > Hi,
> > >
> > > On Wed, Dec 11, 2019 at 9:06 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > > >
> > > > Hi all,
> > > >
> > > > After merging the rcu (I think) tree, today's linux-next build (x86_64
> > > > allnoconfig) produced this warning:
> > > >
> > > > kernel/time/timer.c: In function 'schedule_timeout':
> > > > kernel/time/timer.c:969:20: warning: 'timer.expires' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > > >   969 |   long diff = timer->expires - expires;
> > > >       |               ~~~~~^~~~~~~~~
> > > >
> > > > Introduced by (bisected to) commit
> > > >
> > > >   c4127fce1d02 ("timer: Use hlist_unhashed_lockless() in timer_pending()")
> > > >
> > > > x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130
> > >
> > > This is still there as of last night's -next. Any update on getting a
> > > fix queued together with the offending patch?
> >
> > Hello, Olof,
> >
> > Thank you, I had indeed lost track of this one.  :-/
> >
> > Does Eric's patch fix things for you?
> >
> > https://lore.kernel.org/lkml/CANn89i+xomdo4HFqewrfNf_Z4Q5ayXuW6A4SjSkE46JXP9KuFw@mail.gmail.com/
> 
> It does, but it's not a proper patch (whitespace damage, no S-o-b, etc).

Understood, and thank you for checking!

Eric, would you be willing to turn this into a something that can be
sent upstream?

							Thanx, Paul

Re: linux-next: build warning after merge of the rcu tree

[Paul E. McKenney]

Please accept my apologies for losing track of this one, and for
top-posting to any of you who might be sticklers for that sort of thing.
I must pull this commit out of my set for the next merge window, apply
it to the group for the next merge window, and try out Eric's suggested
changes.  Might still make the next merge window, but clearly not in
its current condition.

If it has taken some other path in the meantime, please do let me know!

							Thanx, Paul

On Wed, Dec 11, 2019 at 10:57:24PM -0800, Eric Dumazet wrote:
> On Wed, Dec 11, 2019 at 10:38 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Wed, Dec 11, 2019 at 10:02 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > >
> > > On Thu, Dec 12, 2019 at 04:06:22PM +1100, Stephen Rothwell wrote:
> > > > Hi all,
> > > >
> > > > After merging the rcu (I think) tree, today's linux-next build (x86_64
> > > > allnoconfig) produced this warning:
> > > >
> > > > kernel/time/timer.c: In function 'schedule_timeout':
> > > > kernel/time/timer.c:969:20: warning: 'timer.expires' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > > >   969 |   long diff = timer->expires - expires;
> > > >       |               ~~~~~^~~~~~~~~
> > > >
> > > > Introduced by (bisected to) commit
> > > >
> > > >   c4127fce1d02 ("timer: Use hlist_unhashed_lockless() in timer_pending()")
> > > >
> > > > x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130
> > >
> > > Well, if the timer is pending, then ->expires has to have been
> > > initialized, but off where the compiler cannot see it, such as during a
> > > previous call to __mod_timer().  And the change may have made it harder
> > > for the compiler to see all of these relationships, but...
> > >
> > > I don't see this warning with gcc version 7.4.0.  Just out of curiosity,
> > > what are you running, Stephen?
> > >
> > > Eric, any thoughts for properly educating the compiler on this one?
> >
> > Ah... the READ_ONCE() apparently turns off the compiler ability to
> > infer that this branch should not be taken.
> >
> > Since __mod_timer() is inlined we could perhaps add a new option
> >
> > diff --git a/kernel/time/timer.c b/kernel/time/timer.c
> > index 4820823515e9..8bbce552568b 100644
> > --- a/kernel/time/timer.c
> > +++ b/kernel/time/timer.c
> > @@ -944,6 +944,7 @@ static struct timer_base *lock_timer_base(struct
> > timer_list *timer,
> >
> >  #define MOD_TIMER_PENDING_ONLY         0x01
> >  #define MOD_TIMER_REDUCE               0x02
> > +#define MOD_TIMER_NOTPENDING           0x04
> >
> >  static inline int
> >  __mod_timer(struct timer_list *timer, unsigned long expires, unsigned
> > int options)
> > @@ -960,7 +961,7 @@ __mod_timer(struct timer_list *timer, unsigned
> > long expires, unsigned int option
> >          * the timer is re-modified to have the same timeout or ends up in the
> >          * same array bucket then just return:
> >          */
> > -       if (timer_pending(timer)) {
> > +       if (!(options & MOD_TIMER_NOTPENDING) && timer_pending(timer)) {
> >                 /*
> >                  * The downside of this optimization is that it can result in
> >                  * larger granularity than you would get from adding a new
> > @@ -1891,7 +1892,7 @@ signed long __sched schedule_timeout(signed long timeout)
> >
> >         timer.task = current;
> >         timer_setup_on_stack(&timer.timer, process_timeout, 0);
> > -       __mod_timer(&timer.timer, expire, 0);
> > +       __mod_timer(&timer.timer, expire, MOD_TIMER_NOTPENDING);
> >         schedule();
> >         del_singleshot_timer_sync(&timer.timer);
> 
> 
> Also add_timer() can benefit from the same hint, since it seems inlined as well.
> 
> (untested patch)
> 
> diff --git a/kernel/time/timer.c b/kernel/time/timer.c
> index 4820823515e9..568564ae3597 100644
> --- a/kernel/time/timer.c
> +++ b/kernel/time/timer.c
> @@ -944,6 +944,7 @@ static struct timer_base *lock_timer_base(struct
> timer_list *timer,
> 
>  #define MOD_TIMER_PENDING_ONLY         0x01
>  #define MOD_TIMER_REDUCE               0x02
> +#define MOD_TIMER_NOTPENDING           0x04
> 
>  static inline int
>  __mod_timer(struct timer_list *timer, unsigned long expires, unsigned
> int options)
> @@ -960,7 +961,7 @@ __mod_timer(struct timer_list *timer, unsigned
> long expires, unsigned int option
>          * the timer is re-modified to have the same timeout or ends up in the
>          * same array bucket then just return:
>          */
> -       if (timer_pending(timer)) {
> +       if (!(options & MOD_TIMER_NOTPENDING) && timer_pending(timer)) {
>                 /*
>                  * The downside of this optimization is that it can result in
>                  * larger granularity than you would get from adding a new
> @@ -1133,7 +1134,7 @@ EXPORT_SYMBOL(timer_reduce);
>  void add_timer(struct timer_list *timer)
>  {
>         BUG_ON(timer_pending(timer));
> -       mod_timer(timer, timer->expires);
> +       __mod_timer(timer, timer->expires, MOD_TIMER_NOTPENDING);
>  }
>  EXPORT_SYMBOL(add_timer);
> 
> @@ -1891,7 +1892,7 @@ signed long __sched schedule_timeout(signed long timeout)
> 
>         timer.task = current;
>         timer_setup_on_stack(&timer.timer, process_timeout, 0);
> -       __mod_timer(&timer.timer, expire, 0);
> +       __mod_timer(&timer.timer, expire, MOD_TIMER_NOTPENDING);
>         schedule();
>         del_singleshot_timer_sync(&timer.timer);

Re: linux-next: build warning after merge of the rcu tree

[Paul E. McKenney]

On Wed, Dec 11, 2019 at 10:57:24PM -0800, Eric Dumazet wrote:
> On Wed, Dec 11, 2019 at 10:38 PM Eric Dumazet <edumazet@google.com> wrote:
> > On Wed, Dec 11, 2019 at 10:02 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > On Thu, Dec 12, 2019 at 04:06:22PM +1100, Stephen Rothwell wrote:
> > > > Hi all,
> > > >
> > > > After merging the rcu (I think) tree, today's linux-next build (x86_64
> > > > allnoconfig) produced this warning:
> > > >
> > > > kernel/time/timer.c: In function 'schedule_timeout':
> > > > kernel/time/timer.c:969:20: warning: 'timer.expires' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > > >   969 |   long diff = timer->expires - expires;
> > > >       |               ~~~~~^~~~~~~~~
> > > >
> > > > Introduced by (bisected to) commit
> > > >
> > > >   c4127fce1d02 ("timer: Use hlist_unhashed_lockless() in timer_pending()")
> > > >
> > > > x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130
> > >
> > > Well, if the timer is pending, then ->expires has to have been
> > > initialized, but off where the compiler cannot see it, such as during a
> > > previous call to __mod_timer().  And the change may have made it harder
> > > for the compiler to see all of these relationships, but...
> > >
> > > I don't see this warning with gcc version 7.4.0.  Just out of curiosity,
> > > what are you running, Stephen?
> > >
> > > Eric, any thoughts for properly educating the compiler on this one?
> >
> > Ah... the READ_ONCE() apparently turns off the compiler ability to
> > infer that this branch should not be taken.
> >
> > Since __mod_timer() is inlined we could perhaps add a new option
> >
> > diff --git a/kernel/time/timer.c b/kernel/time/timer.c
> > index 4820823515e9..8bbce552568b 100644
> > --- a/kernel/time/timer.c
> > +++ b/kernel/time/timer.c
> > @@ -944,6 +944,7 @@ static struct timer_base *lock_timer_base(struct
> > timer_list *timer,
> >
> >  #define MOD_TIMER_PENDING_ONLY         0x01
> >  #define MOD_TIMER_REDUCE               0x02
> > +#define MOD_TIMER_NOTPENDING           0x04
> >
> >  static inline int
> >  __mod_timer(struct timer_list *timer, unsigned long expires, unsigned
> > int options)
> > @@ -960,7 +961,7 @@ __mod_timer(struct timer_list *timer, unsigned
> > long expires, unsigned int option
> >          * the timer is re-modified to have the same timeout or ends up in the
> >          * same array bucket then just return:
> >          */
> > -       if (timer_pending(timer)) {
> > +       if (!(options & MOD_TIMER_NOTPENDING) && timer_pending(timer)) {
> >                 /*
> >                  * The downside of this optimization is that it can result in
> >                  * larger granularity than you would get from adding a new
> > @@ -1891,7 +1892,7 @@ signed long __sched schedule_timeout(signed long timeout)
> >
> >         timer.task = current;
> >         timer_setup_on_stack(&timer.timer, process_timeout, 0);
> > -       __mod_timer(&timer.timer, expire, 0);
> > +       __mod_timer(&timer.timer, expire, MOD_TIMER_NOTPENDING);
> >         schedule();
> >         del_singleshot_timer_sync(&timer.timer);
> 
> 
> Also add_timer() can benefit from the same hint, since it seems inlined as well.
> 
> (untested patch)

Apologies for the delay, fat fingers and holidays...  :-/

I folded this into your earlier patch, resulting in the patch at the
end.  Could you please let me know whether this matches your intent?

							Thanx, Paul

> diff --git a/kernel/time/timer.c b/kernel/time/timer.c
> index 4820823515e9..568564ae3597 100644
> --- a/kernel/time/timer.c
> +++ b/kernel/time/timer.c
> @@ -944,6 +944,7 @@ static struct timer_base *lock_timer_base(struct
> timer_list *timer,
> 
>  #define MOD_TIMER_PENDING_ONLY         0x01
>  #define MOD_TIMER_REDUCE               0x02
> +#define MOD_TIMER_NOTPENDING           0x04
> 
>  static inline int
>  __mod_timer(struct timer_list *timer, unsigned long expires, unsigned
> int options)
> @@ -960,7 +961,7 @@ __mod_timer(struct timer_list *timer, unsigned
> long expires, unsigned int option
>          * the timer is re-modified to have the same timeout or ends up in the
>          * same array bucket then just return:
>          */
> -       if (timer_pending(timer)) {
> +       if (!(options & MOD_TIMER_NOTPENDING) && timer_pending(timer)) {
>                 /*
>                  * The downside of this optimization is that it can result in
>                  * larger granularity than you would get from adding a new
> @@ -1133,7 +1134,7 @@ EXPORT_SYMBOL(timer_reduce);
>  void add_timer(struct timer_list *timer)
>  {
>         BUG_ON(timer_pending(timer));
> -       mod_timer(timer, timer->expires);
> +       __mod_timer(timer, timer->expires, MOD_TIMER_NOTPENDING);
>  }
>  EXPORT_SYMBOL(add_timer);
> 
> @@ -1891,7 +1892,7 @@ signed long __sched schedule_timeout(signed long timeout)
> 
>         timer.task = current;
>         timer_setup_on_stack(&timer.timer, process_timeout, 0);
> -       __mod_timer(&timer.timer, expire, 0);
> +       __mod_timer(&timer.timer, expire, MOD_TIMER_NOTPENDING);
>         schedule();
>         del_singleshot_timer_sync(&timer.timer);
> 
------------------------------------------------------------------------

commit 704c46852c8f8c15cc0ecb45b19f8d3cd983faa6
Author: Eric Dumazet <edumazet@google.com>
Date:   Thu Nov 7 11:37:38 2019 -0800

    timer: Use hlist_unhashed_lockless() in timer_pending()
    
    The timer_pending() function is mostly used in lockless contexts, so
    Without proper annotations, KCSAN might detect a data-race [1].
    
    Using hlist_unhashed_lockless() instead of hand-coding it seems
    appropriate (as suggested by Paul E. McKenney).
    
    [1]
    
    BUG: KCSAN: data-race in del_timer / detach_if_pending
    
    write to 0xffff88808697d870 of 8 bytes by task 10 on cpu 0:
     __hlist_del include/linux/list.h:764 [inline]
     detach_timer kernel/time/timer.c:815 [inline]
     detach_if_pending+0xcd/0x2d0 kernel/time/timer.c:832
     try_to_del_timer_sync+0x60/0xb0 kernel/time/timer.c:1226
     del_timer_sync+0x6b/0xa0 kernel/time/timer.c:1365
     schedule_timeout+0x2d2/0x6e0 kernel/time/timer.c:1896
     rcu_gp_fqs_loop+0x37c/0x580 kernel/rcu/tree.c:1639
     rcu_gp_kthread+0x143/0x230 kernel/rcu/tree.c:1799
     kthread+0x1d4/0x200 drivers/block/aoe/aoecmd.c:1253
     ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:352
    
    read to 0xffff88808697d870 of 8 bytes by task 12060 on cpu 1:
     del_timer+0x3b/0xb0 kernel/time/timer.c:1198
     sk_stop_timer+0x25/0x60 net/core/sock.c:2845
     inet_csk_clear_xmit_timers+0x69/0xa0 net/ipv4/inet_connection_sock.c:523
     tcp_clear_xmit_timers include/net/tcp.h:606 [inline]
     tcp_v4_destroy_sock+0xa3/0x3f0 net/ipv4/tcp_ipv4.c:2096
     inet_csk_destroy_sock+0xf4/0x250 net/ipv4/inet_connection_sock.c:836
     tcp_close+0x6f3/0x970 net/ipv4/tcp.c:2497
     inet_release+0x86/0x100 net/ipv4/af_inet.c:427
     __sock_release+0x85/0x160 net/socket.c:590
     sock_close+0x24/0x30 net/socket.c:1268
     __fput+0x1e1/0x520 fs/file_table.c:280
     ____fput+0x1f/0x30 fs/file_table.c:313
     task_work_run+0xf6/0x130 kernel/task_work.c:113
     tracehook_notify_resume include/linux/tracehook.h:188 [inline]
     exit_to_usermode_loop+0x2b4/0x2c0 arch/x86/entry/common.c:163
    
    Reported by Kernel Concurrency Sanitizer on:
    CPU: 1 PID: 12060 Comm: syz-executor.5 Not tainted 5.4.0-rc3+ #0
    Hardware name: Google Google Compute Engine/Google Compute Engine,
    
    Signed-off-by: Eric Dumazet <edumazet@google.com>
    Cc: Thomas Gleixner <tglx@linutronix.de>
    [ paulmck: Pulled in Eric's later amendments. ]
    Signed-off-by: Paul E. McKenney <paulmck@kernel.org>

diff --git a/include/linux/timer.h b/include/linux/timer.h
index 1e6650e..0dc19a8 100644
--- a/include/linux/timer.h
+++ b/include/linux/timer.h
@@ -164,7 +164,7 @@ static inline void destroy_timer_on_stack(struct timer_list *timer) { }
  */
 static inline int timer_pending(const struct timer_list * timer)
 {
-	return timer->entry.pprev != NULL;
+	return !hlist_unhashed_lockless(&timer->entry);
 }
 
 extern void add_timer_on(struct timer_list *timer, int cpu);
diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 4820823..568564a 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -944,6 +944,7 @@ static struct timer_base *lock_timer_base(struct timer_list *timer,
 
 #define MOD_TIMER_PENDING_ONLY		0x01
 #define MOD_TIMER_REDUCE		0x02
+#define MOD_TIMER_NOTPENDING		0x04
 
 static inline int
 __mod_timer(struct timer_list *timer, unsigned long expires, unsigned int options)
@@ -960,7 +961,7 @@ __mod_timer(struct timer_list *timer, unsigned long expires, unsigned int option
 	 * the timer is re-modified to have the same timeout or ends up in the
 	 * same array bucket then just return:
 	 */
-	if (timer_pending(timer)) {
+	if (!(options & MOD_TIMER_NOTPENDING) && timer_pending(timer)) {
 		/*
 		 * The downside of this optimization is that it can result in
 		 * larger granularity than you would get from adding a new
@@ -1133,7 +1134,7 @@ EXPORT_SYMBOL(timer_reduce);
 void add_timer(struct timer_list *timer)
 {
 	BUG_ON(timer_pending(timer));
-	mod_timer(timer, timer->expires);
+	__mod_timer(timer, timer->expires, MOD_TIMER_NOTPENDING);
 }
 EXPORT_SYMBOL(add_timer);
 
@@ -1891,7 +1892,7 @@ signed long __sched schedule_timeout(signed long timeout)
 
 	timer.task = current;
 	timer_setup_on_stack(&timer.timer, process_timeout, 0);
-	__mod_timer(&timer.timer, expire, 0);
+	__mod_timer(&timer.timer, expire, MOD_TIMER_NOTPENDING);
 	schedule();
 	del_singleshot_timer_sync(&timer.timer);
 

Re: linux-next: build warning after merge of the rcu tree

[Paul E. McKenney]

On Thu, Jan 25, 2024 at 02:33:32PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the rcu tree, today's linux-next build (htmldocs) produced
> this warning:
> 
> include/linux/hrtimer.h:199: warning: Function parameter or struct member 'online' not described in 'hrtimer_cpu_base'
> 
> Introduced by commit
> 
>   7525a3cbb106 ("hrtimer: Report offline hrtimer enqueue")

Apologies for the hassle, and please see the proposed fixup patch below.
Unless I hear otherwise, I will merge this into the original before the
likely start of your day.

							Thanx, Paul

------------------------------------------------------------------------

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index 4f2cf7309486..991c83e929b4 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -151,6 +151,7 @@ enum  hrtimer_base_type {
  * @hang_detected:	The last hrtimer interrupt detected a hang
  * @softirq_activated:	displays, if the softirq is raised - update of softirq
  *			related settings is not required then.
+ * @online:		CPU is online from an hrtimers viewpoint
  * @nr_events:		Total number of hrtimer interrupt events
  * @nr_retries:		Total number of hrtimer interrupt retries
  * @nr_hangs:		Total number of hrtimer interrupt hangs

linux-next: build warning after merge of the rcu tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 342 bytes --]

Hi all,

After merging the rcu tree, today's linux-next build (htmldocs) produced
this warning:

include/linux/hrtimer.h:199: warning: Function parameter or struct member 'online' not described in 'hrtimer_cpu_base'

Introduced by commit

  7525a3cbb106 ("hrtimer: Report offline hrtimer enqueue")

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build warning after merge of the rcu tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2189 bytes --]

Hi Paul,

On Tue, 25 Jul 2023 20:48:13 -0700 "Paul E. McKenney" <paulmck@kernel.org> wrote:
>
> Does the following incremental diff (to be squashed into the original) help?
> 
> 							Thanx, Paul
> 
> ------------------------------------------------------------------------
> 
> diff --git a/init/main.c b/init/main.c
> index c946ab87783a..981170da0b1c 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state);
>  void (*__initdata late_time_init)(void);
>  
>  /* Untouched command line saved by arch-specific code. */
> -char __initdata boot_command_line[COMMAND_LINE_SIZE];
> +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init;
>  /* Untouched saved command line (eg. for /proc) */
>  char *saved_command_line __ro_after_init;
>  unsigned int saved_command_line_len __ro_after_init;

I needed the following (only tested x86_64 allmodconfig build):

diff --git a/include/linux/init.h b/include/linux/init.h
index 9a5973324072..e3ce68988e1b 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -112,6 +112,9 @@
 #define __REFCONST       .section       ".ref.rodata", "a"
 
 #ifndef __ASSEMBLY__
+
+#include <linux/cache.h>
+
 /*
  * Used for initialization calls..
  */
@@ -143,7 +146,7 @@ struct file_system_type;
 
 /* Defined in init/main.c */
 extern int do_one_initcall(initcall_t fn);
-extern char __initdata boot_command_line[];
+extern char boot_command_line[] __ro_after_init;
 extern char *saved_command_line;
 extern unsigned int saved_command_line_len;
 extern unsigned int reset_devices;
diff --git a/init/main.c b/init/main.c
index c946ab87783a..981170da0b1c 100644
--- a/init/main.c
+++ b/init/main.c
@@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state);
 void (*__initdata late_time_init)(void);
 
 /* Untouched command line saved by arch-specific code. */
-char __initdata boot_command_line[COMMAND_LINE_SIZE];
+char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init;
 /* Untouched saved command line (eg. for /proc) */
 char *saved_command_line __ro_after_init;
 unsigned int saved_command_line_len __ro_after_init;

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build warning after merge of the rcu tree

[Paul E. McKenney]

On Tue, Jul 25, 2023 at 08:33:06PM -0700, Paul E. McKenney wrote:
> On Wed, Jul 26, 2023 at 12:32:30PM +1000, Stephen Rothwell wrote:
> > Hi all,
> > 
> > After merging the rcu tree, today's linux-next build (x86_64 allmodconfig)
> > produced this warning:
> > 
> > WARNING: modpost: vmlinux: section mismatch in reference: cmdline_load_proc_show+0x22 (section: .text) -> boot_command_line (section: .init.data)
> > 
> > Introduced by commit
> > 
> >   cf9eca90a339 ("fs/proc: Add /proc/cmdline_load for boot loader arguments")
> 
> That __initdata needs to be __ro_after_init, doesn't it?  Will fix,
> thank you!

Sigh.  Hit "send" too soon.  :-/

Does the following incremental diff (to be squashed into the original) help?

							Thanx, Paul

------------------------------------------------------------------------

diff --git a/init/main.c b/init/main.c
index c946ab87783a..981170da0b1c 100644
--- a/init/main.c
+++ b/init/main.c
@@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state);
 void (*__initdata late_time_init)(void);
 
 /* Untouched command line saved by arch-specific code. */
-char __initdata boot_command_line[COMMAND_LINE_SIZE];
+char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init;
 /* Untouched saved command line (eg. for /proc) */
 char *saved_command_line __ro_after_init;
 unsigned int saved_command_line_len __ro_after_init;

Re: linux-next: build warning after merge of the rcu tree

[Paul E. McKenney]

On Wed, Jul 26, 2023 at 12:32:30PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the rcu tree, today's linux-next build (x86_64 allmodconfig)
> produced this warning:
> 
> WARNING: modpost: vmlinux: section mismatch in reference: cmdline_load_proc_show+0x22 (section: .text) -> boot_command_line (section: .init.data)
> 
> Introduced by commit
> 
>   cf9eca90a339 ("fs/proc: Add /proc/cmdline_load for boot loader arguments")

That __initdata needs to be __ro_after_init, doesn't it?  Will fix,
thank you!

							Thanx, Paul

linux-next: build warning after merge of the rcu tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 398 bytes --]

Hi all,

After merging the rcu tree, today's linux-next build (x86_64 allmodconfig)
produced this warning:

WARNING: modpost: vmlinux: section mismatch in reference: cmdline_load_proc_show+0x22 (section: .text) -> boot_command_line (section: .init.data)

Introduced by commit

  cf9eca90a339 ("fs/proc: Add /proc/cmdline_load for boot loader arguments")

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build warning after merge of the rcu tree

[Paul E. McKenney]

On Thu, Apr 06, 2023 at 02:43:46PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the rcu tree, today's linux-next build (htmldocs) produced
> this warning:
> 
> kernel/rcu/tree.c:2808: warning: Function parameter or member 'head_free_gp_snap' not described in 'kfree_rcu_cpu_work'
> 
> Introduced by commit
> 
>   07030d20c8ec ("rcu/kvfree: Add debug check for GP complete for kfree_rcu_cpu list")

Will fix, thank you!

							Thanx, Paul

linux-next: build warning after merge of the rcu tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 370 bytes --]

Hi all,

After merging the rcu tree, today's linux-next build (htmldocs) produced
this warning:

kernel/rcu/tree.c:2808: warning: Function parameter or member 'head_free_gp_snap' not described in 'kfree_rcu_cpu_work'

Introduced by commit

  07030d20c8ec ("rcu/kvfree: Add debug check for GP complete for kfree_rcu_cpu list")

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build warning after merge of the rcu tree

[Boqun Feng]

On Tue, Mar 21, 2023 at 01:50:47PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the rcu tree, today's linux-next build (htmldocs) produced
> this warning:
> 
> include/linux/srcu.h:106: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
>  * Annotations provide deadlock detection for SRCU.

Thanks for reporting. Fix it locally.

Regards,
Boqun

> 
> Introduced by commit
> 
>   57e98b73317b ("rcu: Annotate SRCU's update-side lockdep dependencies")
> 
> -- 
> Cheers,
> Stephen Rothwell

linux-next: build warning after merge of the rcu tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 434 bytes --]

Hi all,

After merging the rcu tree, today's linux-next build (htmldocs) produced
this warning:

include/linux/srcu.h:106: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
 * Annotations provide deadlock detection for SRCU.

Introduced by commit

  57e98b73317b ("rcu: Annotate SRCU's update-side lockdep dependencies")

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build warning after merge of the rcu tree

[Paul E. McKenney]

On Mon, Nov 07, 2022 at 02:26:41PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the rcu tree, today's linux-next build (htmldocs)
> produced this warning:
> 
> Documentation/RCU/rcubarrier.rst:205: WARNING: Literal block ends without a blank line; unexpected unindent.
> 
> Introduced by commit
> 
>   21c2e3909721 ("doc: Update rcubarrier.rst")

Huh.  I guess that numbered code samples are not supposed to have more
than nine lines?  Ah well, easy to fix by going back to left-justified
numbers.  I was wondering about that!

Apologies for the hassle!

							Thanx, Paul

linux-next: build warning after merge of the rcu tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 319 bytes --]

Hi all,

After merging the rcu tree, today's linux-next build (htmldocs)
produced this warning:

Documentation/RCU/rcubarrier.rst:205: WARNING: Literal block ends without a blank line; unexpected unindent.

Introduced by commit

  21c2e3909721 ("doc: Update rcubarrier.rst")

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build warning after merge of the rcu tree

[Paul E. McKenney]

On Wed, Jun 15, 2022 at 03:38:02PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the rcu tree, today's linux-next build (htmldocs) produced
> this warning:
> 
> Documentation/RCU/Design/Requirements/Requirements.rst:2220: WARNING: Malformed table.
> 
> +-----------------------------------------------------------------------+
> | **Quick Quiz**:                                                       |
> +-----------------------------------------------------------------------+
> | But what if my driver has a hardware interrupt handler that can run   |
> | for many seconds? I cannot invoke schedule() from an hardware         |
> | interrupt handler, after all!                                         |
> +-----------------------------------------------------------------------+
> | **Answer**:                                                           |
> +-----------------------------------------------------------------------+
> | One approach is to do ``ct_irq_exit();ct_irq_enter();`` every so    |
> | often. But given that long-running interrupt handlers can cause other |
> | problems, not least for response time, shouldn't you work to keep     |
> | your interrupt handler's runtime within reasonable bounds?            |
> +-----------------------------------------------------------------------+
> 
> Introduced by commit
> 
>   6c5218715286 ("context_tracking: Take IRQ eqs entrypoints over RCU")

Apologies and thank you, will fix.

							Thanx, Paul

linux-next: build warning after merge of the rcu tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1372 bytes --]

Hi all,

After merging the rcu tree, today's linux-next build (htmldocs) produced
this warning:

Documentation/RCU/Design/Requirements/Requirements.rst:2220: WARNING: Malformed table.

+-----------------------------------------------------------------------+
| **Quick Quiz**:                                                       |
+-----------------------------------------------------------------------+
| But what if my driver has a hardware interrupt handler that can run   |
| for many seconds? I cannot invoke schedule() from an hardware         |
| interrupt handler, after all!                                         |
+-----------------------------------------------------------------------+
| **Answer**:                                                           |
+-----------------------------------------------------------------------+
| One approach is to do ``ct_irq_exit();ct_irq_enter();`` every so    |
| often. But given that long-running interrupt handlers can cause other |
| problems, not least for response time, shouldn't you work to keep     |
| your interrupt handler's runtime within reasonable bounds?            |
+-----------------------------------------------------------------------+

Introduced by commit

  6c5218715286 ("context_tracking: Take IRQ eqs entrypoints over RCU")

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build warning after merge of the rcu tree

[Paul E. McKenney]

On Thu, Mar 04, 2021 at 12:41:05PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the rcu tree, today's linux-next build (htmldocs) produced
> this warning:
> 
> kernel/rcu/tree.c:3824: warning: expecting prototype for start_poll_state_synchronize_rcu(). Prototype was for start_poll_synchronize_rcu() instead
> 
> Introduced by commit
> 
>   7f9a26bbfff2 ("rcu: Provide polling interfaces for Tree RCU grace periods")

Apologies and thank you, will fix!

							Thanx, Paul

linux-next: build warning after merge of the rcu tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 390 bytes --]

Hi all,

After merging the rcu tree, today's linux-next build (htmldocs) produced
this warning:

kernel/rcu/tree.c:3824: warning: expecting prototype for start_poll_state_synchronize_rcu(). Prototype was for start_poll_synchronize_rcu() instead

Introduced by commit

  7f9a26bbfff2 ("rcu: Provide polling interfaces for Tree RCU grace periods")

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build warning after merge of the rcu tree

[Paul E. McKenney]

On Mon, Dec 07, 2020 at 10:48:51AM -0700, Jonathan Corbet wrote:
> On Mon, 7 Dec 2020 08:47:04 -0800
> "Paul E. McKenney" <paulmck@kernel.org> wrote:
> 
> > I freely confess that I have absolutely no idea what it doesn't like.
> > It is complaining about this header comment, correct?
> > 
> > /**
> >  * kmem_last_alloc_stack - Get return address and stack for last allocation
> >  * @object: object for which to find last-allocation return address.
> >  * @stackp: %NULL or pointer to location to place return-address stack.
> >  * @nstackp: maximum number of return addresses that may be stored.
> >  *
> >  * If the pointer references a slab-allocated object and if sufficient
> >  * debugging is enabled, return the return address for the corresponding
> >  * allocation.  If stackp is non-%NULL in %CONFIG_STACKTRACE kernels running
> >  * the slub allocator, also copy the return-address stack into @stackp,
> >  * limited by @nstackp.  Otherwise, return %NULL or an appropriate error
> >  * code using %ERR_PTR().
> >  *
> >  * Return: return address from last allocation, %NULL or negative error code.
> >  */
> 
> The problem is the %ERR_PTR().  I'm honestly not quite sure why, Sphinx is
> being a little weird there.  But in any case the % notation is supposed to
> mark a constant, which is not the case here.  I'd just take the % signs
> out.

Thank you, will do!

							Thanx, Paul

Re: linux-next: build warning after merge of the rcu tree

[Jonathan Corbet]

On Mon, 7 Dec 2020 08:47:04 -0800
"Paul E. McKenney" <paulmck@kernel.org> wrote:

> I freely confess that I have absolutely no idea what it doesn't like.
> It is complaining about this header comment, correct?
> 
> /**
>  * kmem_last_alloc_stack - Get return address and stack for last allocation
>  * @object: object for which to find last-allocation return address.
>  * @stackp: %NULL or pointer to location to place return-address stack.
>  * @nstackp: maximum number of return addresses that may be stored.
>  *
>  * If the pointer references a slab-allocated object and if sufficient
>  * debugging is enabled, return the return address for the corresponding
>  * allocation.  If stackp is non-%NULL in %CONFIG_STACKTRACE kernels running
>  * the slub allocator, also copy the return-address stack into @stackp,
>  * limited by @nstackp.  Otherwise, return %NULL or an appropriate error
>  * code using %ERR_PTR().
>  *
>  * Return: return address from last allocation, %NULL or negative error code.
>  */

The problem is the %ERR_PTR().  I'm honestly not quite sure why, Sphinx is
being a little weird there.  But in any case the % notation is supposed to
mark a constant, which is not the case here.  I'd just take the % signs
out.

jon

Re: linux-next: build warning after merge of the rcu tree

[Paul E. McKenney]

On Mon, Dec 07, 2020 at 07:20:28PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the rcu tree, today's linux-next build (htmldocs) produced
> this warning:
> 
> Documentation/core-api/mm-api:49: mm/slab_common.c:569: WARNING: Inline literal start-string without end-string.
> Documentation/core-api/mm-api:49: mm/slab_common.c:595: WARNING: Inline literal start-string without end-string.
> 
> Maybe introduced by commit
> 
>   f7c3fb4fc476 ("mm: Add kmem_last_alloc() to return last allocation for memory block")
> 
> (or one of the following ones).

I freely confess that I have absolutely no idea what it doesn't like.
It is complaining about this header comment, correct?

/**
 * kmem_last_alloc_stack - Get return address and stack for last allocation
 * @object: object for which to find last-allocation return address.
 * @stackp: %NULL or pointer to location to place return-address stack.
 * @nstackp: maximum number of return addresses that may be stored.
 *
 * If the pointer references a slab-allocated object and if sufficient
 * debugging is enabled, return the return address for the corresponding
 * allocation.  If stackp is non-%NULL in %CONFIG_STACKTRACE kernels running
 * the slub allocator, also copy the return-address stack into @stackp,
 * limited by @nstackp.  Otherwise, return %NULL or an appropriate error
 * code using %ERR_PTR().
 *
 * Return: return address from last allocation, %NULL or negative error code.
 */

							Thanx, Paul

linux-next: build warning after merge of the rcu tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 520 bytes --]

Hi all,

After merging the rcu tree, today's linux-next build (htmldocs) produced
this warning:

Documentation/core-api/mm-api:49: mm/slab_common.c:569: WARNING: Inline literal start-string without end-string.
Documentation/core-api/mm-api:49: mm/slab_common.c:595: WARNING: Inline literal start-string without end-string.

Maybe introduced by commit

  f7c3fb4fc476 ("mm: Add kmem_last_alloc() to return last allocation for memory block")

(or one of the following ones).

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build warning after merge of the rcu tree

[Paul E. McKenney]

On Tue, Mar 10, 2020 at 01:27:09PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the rcu tree, today's linux-next build (x86_64 allmodconfig)
> produced this warning:
> 
> WARNING: "rcu_tasks_rude_wait_gp" [vmlinux] is a static EXPORT_SYMBOL_GPL

Good catch, will fix, thank you!

> Introduced by commit
> 
>   cbd703932774 ("rcu-tasks: Add an RCU-tasks rude variant")
> 
> Why is all that code in a header file?  Do you intend to have those
> functions defined in each C file that includes kernel/rcu/tasks.h?

Same setup as for tree_plugin.h, tree_exp.h, tree_stall.h, and so in
in that same directory.  Plugins of various sorts.

							Thanx, Paul

linux-next: build warning after merge of the rcu tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 448 bytes --]

Hi all,

After merging the rcu tree, today's linux-next build (x86_64 allmodconfig)
produced this warning:

WARNING: "rcu_tasks_rude_wait_gp" [vmlinux] is a static EXPORT_SYMBOL_GPL

Introduced by commit

  cbd703932774 ("rcu-tasks: Add an RCU-tasks rude variant")

Why is all that code in a header file?  Do you intend to have those
functions defined in each C file that includes kernel/rcu/tasks.h?

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build warning after merge of the rcu tree

[Paul E. McKenney]

On Wed, Jun 21, 2017 at 04:48:06PM +1000, Stephen Rothwell wrote:
> Hi Paul,
> 
> After merging the rcu tree, today's linux-next build (x86_64 allnoconfig)
> produced this warning:
> 
> kernel/cpu.c: In function 'boot_cpu_state_init':
> kernel/cpu.c:1778:6: warning: unused variable 'cpu' [-Wunused-variable]
>   int cpu;
>       ^
> 
> Introduced by commit
> 
>   faeb334286b7 ("rcu: Migrate callbacks earlier in the CPU-offline timeline")

Hello, Stephen,

Apologies, should be fixed now -- added a __maybe_unused.

But I didn't realize you did SMP=n builds!  ;-)

							Thanx, Paul

linux-next: build warning after merge of the rcu tree

[Stephen Rothwell]

Hi Paul,

After merging the rcu tree, today's linux-next build (x86_64 allnoconfig)
produced this warning:

kernel/cpu.c: In function 'boot_cpu_state_init':
kernel/cpu.c:1778:6: warning: unused variable 'cpu' [-Wunused-variable]
  int cpu;
      ^

Introduced by commit

  faeb334286b7 ("rcu: Migrate callbacks earlier in the CPU-offline timeline")

-- 
Cheers,
Stephen Rothwell

Re: linux-next: build warning after merge of the rcu tree

[Paul E. McKenney]

On Wed, Aug 24, 2011 at 08:46:15PM -0400, Arnaud Lacombe wrote:
> Hi,
> 
> On Wed, Aug 24, 2011 at 8:27 AM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> > On Wed, Aug 24, 2011 at 02:23:37PM +1000, Stephen Rothwell wrote:
> >> Hi Paul,
> >>
> >> After merging the rcu tree, today's linux-next build (powerpc
> >> ppc64_defconfig) produced this warning:
> >>
> >> kernel/rtmutex.c: In function '__rt_mutex_slowlock':
> >> kernel/rtmutex.c:605:3: warning: suggest parentheses around assignment used as truth value
> >
> > There actually already are parentheses around it, and the first pass
> > through the loop it is uninitialized at that point.  But hey, that
> > is gcc for you!  Does the patch below cure it?
> >
> Yes.

Thank you!

							Thanx, Paul

>  - Arnaud
> 
> >> Introduced by commit 83841f021d4b ("rcu: Permit rt_mutex_unlock() with
> >> irqs disabled").
> >
> > If it does, then I will fold into that commit.
> >
> >                                                        Thanx, Paul
> >
> > ------------------------------------------------------------------------
> >
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> >
> > diff --git a/kernel/rtmutex.c b/kernel/rtmutex.c
> > index 0222e34..2548f44 100644
> > --- a/kernel/rtmutex.c
> > +++ b/kernel/rtmutex.c
> > @@ -602,7 +602,8 @@ __rt_mutex_slowlock(struct rt_mutex *lock, int state,
> >
> >                raw_spin_unlock(&lock->wait_lock);
> >
> > -               if (was_disabled = irqs_disabled())
> > +               was_disabled = irqs_disabled();
> > +               if (was_disabled)
> >                        local_irq_enable();
> >
> >                debug_rt_mutex_print_deadlock(waiter);
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> >

Re: linux-next: build warning after merge of the rcu tree

[Arnaud Lacombe]

Hi,

On Wed, Aug 24, 2011 at 8:27 AM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> On Wed, Aug 24, 2011 at 02:23:37PM +1000, Stephen Rothwell wrote:
>> Hi Paul,
>>
>> After merging the rcu tree, today's linux-next build (powerpc
>> ppc64_defconfig) produced this warning:
>>
>> kernel/rtmutex.c: In function '__rt_mutex_slowlock':
>> kernel/rtmutex.c:605:3: warning: suggest parentheses around assignment used as truth value
>
> There actually already are parentheses around it, and the first pass
> through the loop it is uninitialized at that point.  But hey, that
> is gcc for you!  Does the patch below cure it?
>
Yes.

 - Arnaud

>> Introduced by commit 83841f021d4b ("rcu: Permit rt_mutex_unlock() with
>> irqs disabled").
>
> If it does, then I will fold into that commit.
>
>                                                        Thanx, Paul
>
> ------------------------------------------------------------------------
>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
>
> diff --git a/kernel/rtmutex.c b/kernel/rtmutex.c
> index 0222e34..2548f44 100644
> --- a/kernel/rtmutex.c
> +++ b/kernel/rtmutex.c
> @@ -602,7 +602,8 @@ __rt_mutex_slowlock(struct rt_mutex *lock, int state,
>
>                raw_spin_unlock(&lock->wait_lock);
>
> -               if (was_disabled = irqs_disabled())
> +               was_disabled = irqs_disabled();
> +               if (was_disabled)
>                        local_irq_enable();
>
>                debug_rt_mutex_print_deadlock(waiter);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: linux-next: build warning after merge of the rcu tree

[Paul E. McKenney]

On Wed, Aug 24, 2011 at 02:23:37PM +1000, Stephen Rothwell wrote:
> Hi Paul,
> 
> After merging the rcu tree, today's linux-next build (powerpc
> ppc64_defconfig) produced this warning:
> 
> kernel/rtmutex.c: In function '__rt_mutex_slowlock':
> kernel/rtmutex.c:605:3: warning: suggest parentheses around assignment used as truth value

There actually already are parentheses around it, and the first pass
through the loop it is uninitialized at that point.  But hey, that 
is gcc for you!  Does the patch below cure it?

> Introduced by commit 83841f021d4b ("rcu: Permit rt_mutex_unlock() with
> irqs disabled").

If it does, then I will fold into that commit.

							Thanx, Paul

------------------------------------------------------------------------

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

diff --git a/kernel/rtmutex.c b/kernel/rtmutex.c
index 0222e34..2548f44 100644
--- a/kernel/rtmutex.c
+++ b/kernel/rtmutex.c
@@ -602,7 +602,8 @@ __rt_mutex_slowlock(struct rt_mutex *lock, int state,
 
 		raw_spin_unlock(&lock->wait_lock);
 
-		if (was_disabled = irqs_disabled())
+		was_disabled = irqs_disabled();
+		if (was_disabled)
 			local_irq_enable();
 
 		debug_rt_mutex_print_deadlock(waiter);

linux-next: build warning after merge of the rcu tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 465 bytes --]

Hi Paul,

After merging the rcu tree, today's linux-next build (powerpc
ppc64_defconfig) produced this warning:

kernel/rtmutex.c: In function '__rt_mutex_slowlock':
kernel/rtmutex.c:605:3: warning: suggest parentheses around assignment used as truth value

Introduced by commit 83841f021d4b ("rcu: Permit rt_mutex_unlock() with
irqs disabled").

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: linux-next: build warning after merge of the rcu tree

[Paul E. McKenney]

On Fri, Mar 25, 2011 at 04:05:39PM +1100, Stephen Rothwell wrote:
> Hi Paul,
> 
> After merging the rcu tree, today's linux-next build (powerpc
> ppc64_defconfig) produced this warning:
> 
> kernel/softirq.c:62: warning: excess elements in array initializer
> kernel/softirq.c:62: warning: (near initialization for 'softirq_to_name')
> 
> Introduced by commit 0ac7e739ad17 ("rcu: move TREE_RCU from softirq to
> kthread").

I must confess that it didn't occur to me to check for more than one
array of softirq names, and I have no idea why I didn't see this warning
in my builds.  In any case, I have fixed this and updated -rcu.

I just now found the mention of RCU in the proc.txt documentation file,
and have a fix for that queued as well, and will export to -rcu on the
next round.

							Thanx, Paul

linux-next: build warning after merge of the rcu tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 454 bytes --]

Hi Paul,

After merging the rcu tree, today's linux-next build (powerpc
ppc64_defconfig) produced this warning:

kernel/softirq.c:62: warning: excess elements in array initializer
kernel/softirq.c:62: warning: (near initialization for 'softirq_to_name')

Introduced by commit 0ac7e739ad17 ("rcu: move TREE_RCU from softirq to
kthread").
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: linux-next: build warning after merge of the rcu tree

[Paul E. McKenney]

On Mon, Sep 06, 2010 at 12:32:55PM +1000, Neil Brown wrote:
> On Mon, 6 Sep 2010 12:14:08 +1000
> Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> 
> > Hi Paul,
> > 
> > After merging the rcu tree, today's linux-next build
> > (powerpc ppc64_defconfig) produced this warning:
> > 
> > drivers/md/raid1.c: In function 'read_balance':
> > drivers/md/raid1.c:445: warning: operation on 'new_disk' may be undefined
> > 
> > I am picking on the rcu tree because the line above has not changed since
> > 2005.  The line is:
> > 
> >                 for (rdev = rcu_dereference(conf->mirrors[new_disk].rdev);
> >                      r1_bio->bios[new_disk] == IO_BLOCKED ||
> >                      !rdev || !test_bit(In_sync, &rdev->flags)
> >                              || test_bit(WriteMostly, &rdev->flags);
> >                      rdev = rcu_dereference(conf->mirrors[++new_disk].rdev)) {
> > 
> > Where new_disk is being updated in the parameter to the last
> > rcu_dereference.
> > 
> 
> Hi Stephen,
> 
>  There is a patch in linux-kernel
>     "PATCH] md: do not use ++ in rcu_dereference() argument"
> 
>  that addresses this - it is a problem in md: I guess rcu as become more
>  helpful in finding these problems.

Indeed, the sparse-based checking is a bit more picky about things,
even when you are not using sparse...

							Thanx, Paul

>  I'll probably fix it a little differently, but I'll have something in my
>  for-next for tomorrow.
> 
> Thanks,
> NeilBrown

Re: linux-next: build warning after merge of the rcu tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 543 bytes --]

Hi Neil,


On Mon, 6 Sep 2010 12:32:55 +1000 Neil Brown <neilb@suse.de> wrote:
>
>  There is a patch in linux-kernel
>     "PATCH] md: do not use ++ in rcu_dereference() argument"
> 
>  that addresses this - it is a problem in md: I guess rcu as become more
>  helpful in finding these problems.
> 
>  I'll probably fix it a little differently, but I'll have something in my
>  for-next for tomorrow.

Ah, ok.  Thanks.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: linux-next: build warning after merge of the rcu tree

[Neil Brown]

On Mon, 6 Sep 2010 12:14:08 +1000
Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi Paul,
> 
> After merging the rcu tree, today's linux-next build
> (powerpc ppc64_defconfig) produced this warning:
> 
> drivers/md/raid1.c: In function 'read_balance':
> drivers/md/raid1.c:445: warning: operation on 'new_disk' may be undefined
> 
> I am picking on the rcu tree because the line above has not changed since
> 2005.  The line is:
> 
>                 for (rdev = rcu_dereference(conf->mirrors[new_disk].rdev);
>                      r1_bio->bios[new_disk] == IO_BLOCKED ||
>                      !rdev || !test_bit(In_sync, &rdev->flags)
>                              || test_bit(WriteMostly, &rdev->flags);
>                      rdev = rcu_dereference(conf->mirrors[++new_disk].rdev)) {
> 
> Where new_disk is being updated in the parameter to the last
> rcu_dereference.
> 

Hi Stephen,

 There is a patch in linux-kernel
    "PATCH] md: do not use ++ in rcu_dereference() argument"

 that addresses this - it is a problem in md: I guess rcu as become more
 helpful in finding these problems.

 I'll probably fix it a little differently, but I'll have something in my
 for-next for tomorrow.

Thanks,
NeilBrown

linux-next: build warning after merge of the rcu tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 885 bytes --]

Hi Paul,

After merging the rcu tree, today's linux-next build
(powerpc ppc64_defconfig) produced this warning:

drivers/md/raid1.c: In function 'read_balance':
drivers/md/raid1.c:445: warning: operation on 'new_disk' may be undefined

I am picking on the rcu tree because the line above has not changed since
2005.  The line is:

                for (rdev = rcu_dereference(conf->mirrors[new_disk].rdev);
                     r1_bio->bios[new_disk] == IO_BLOCKED ||
                     !rdev || !test_bit(In_sync, &rdev->flags)
                             || test_bit(WriteMostly, &rdev->flags);
                     rdev = rcu_dereference(conf->mirrors[++new_disk].rdev)) {

Where new_disk is being updated in the parameter to the last
rcu_dereference.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]