From: Eric Dumazet <edumazet@google.com>
To: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>,
Linux Next Mailing List <linux-next@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: linux-next: build warning after merge of the rcu tree
Date: Wed, 11 Dec 2019 22:57:24 -0800 [thread overview]
Message-ID: <CANn89i+xomdo4HFqewrfNf_Z4Q5ayXuW6A4SjSkE46JXP9KuFw@mail.gmail.com> (raw)
In-Reply-To: <CANn89iKJhsMLUBNbkXSr1+t+38POFU8jWrP+tU3JWLjs__HuPw@mail.gmail.com>
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);
next prev parent reply other threads:[~2019-12-12 6:57 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-12 5:06 linux-next: build warning after merge of the rcu tree Stephen Rothwell
2019-12-12 6:02 ` Paul E. McKenney
2019-12-12 6:38 ` Eric Dumazet
2019-12-12 6:57 ` Eric Dumazet [this message]
2020-01-10 21:57 ` Paul E. McKenney
2020-01-15 16:42 ` Paul E. McKenney
2019-12-12 11:40 ` Stephen Rothwell
2019-12-13 1:31 ` Paul E. McKenney
2020-01-06 17:51 ` Olof Johansson
2020-01-06 18:10 ` Paul E. McKenney
2020-01-06 21:08 ` Olof Johansson
2020-01-06 21:43 ` Paul E. McKenney
-- strict thread matches above, loose matches on Subject: below --
2024-01-25 3:33 Stephen Rothwell
2024-01-25 13:18 ` Paul E. McKenney
2023-07-26 2:32 Stephen Rothwell
2023-07-26 3:33 ` Paul E. McKenney
2023-07-26 3:48 ` Paul E. McKenney
2023-07-26 6:37 ` Stephen Rothwell
2023-04-06 4:43 Stephen Rothwell
2023-04-06 14:15 ` Paul E. McKenney
2023-03-21 2:50 Stephen Rothwell
2023-03-21 3:01 ` Boqun Feng
2022-11-07 3:26 Stephen Rothwell
2022-11-07 5:02 ` Paul E. McKenney
2022-11-07 9:28 ` Akira Yokosawa
2022-06-15 5:38 Stephen Rothwell
2022-06-15 13:55 ` Paul E. McKenney
2021-03-04 1:41 Stephen Rothwell
2021-03-04 1:48 ` Paul E. McKenney
2020-12-07 8:20 Stephen Rothwell
2020-12-07 16:47 ` Paul E. McKenney
2020-12-07 17:48 ` Jonathan Corbet
2020-12-07 18:53 ` Paul E. McKenney
2020-03-10 2:27 Stephen Rothwell
2020-03-10 2:49 ` Paul E. McKenney
2017-06-21 6:48 Stephen Rothwell
2017-06-21 13:30 ` Paul E. McKenney
2011-08-24 4:23 Stephen Rothwell
2011-08-24 12:27 ` Paul E. McKenney
2011-08-25 0:46 ` Arnaud Lacombe
2011-08-25 5:04 ` Paul E. McKenney
2011-03-25 5:05 Stephen Rothwell
2011-03-25 19:23 ` Paul E. McKenney
2010-09-06 2:14 Stephen Rothwell
2010-09-06 2:32 ` Neil Brown
2010-09-06 3:02 ` Stephen Rothwell
2010-09-06 5:37 ` Paul E. McKenney
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CANn89i+xomdo4HFqewrfNf_Z4Q5ayXuW6A4SjSkE46JXP9KuFw@mail.gmail.com \
--to=edumazet@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-next@vger.kernel.org \
--cc=paulmck@kernel.org \
--cc=sfr@canb.auug.org.au \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).