Re: nohz: Warn if the machine can not perform nohz_full

Re: nohz: Warn if the machine can not perform nohz_full

[Dave Jones]

 > Gitweb:     http://git.kernel.org/linus/;a=commit;h=e12d0271774fea9fddf1e2a7952a0bffb2ee8e8b
 > Commit:     e12d0271774fea9fddf1e2a7952a0bffb2ee8e8b
 > Parent:     7d132055814ef17a6c7b69f342244c410a5e000f
 > Author:     Steven Rostedt <rostedt@goodmis.org>
 > AuthorDate: Fri May 10 17:12:28 2013 -0400
 > Committer:  Frederic Weisbecker <fweisbec@gmail.com>
 > CommitDate: Thu Jun 20 01:15:51 2013 +0200
 > 
 >     nohz: Warn if the machine can not perform nohz_full
 >     
 >     If the user configures NO_HZ_FULL and defines nohz_full=XXX on the
 >     kernel command line, or enables NO_HZ_FULL_ALL, but nohz fails
 >     due to the machine having a unstable clock, warn about it.
 >     
 >     We do not want users thinking that they are getting the benefit
 >     of nohz when their machine can not support it.
 > ---
 >  kernel/time/tick-sched.c |    5 +++++
 >  1 files changed, 5 insertions(+), 0 deletions(-)
 > 
 > diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
 > index f420813..d87d22c 100644
 > --- a/kernel/time/tick-sched.c
 > +++ b/kernel/time/tick-sched.c
 > @@ -178,6 +178,11 @@ static bool can_stop_full_tick(void)
 >  	 */
 >  	if (!sched_clock_stable) {
 >  		trace_tick_stop(0, "unstable sched clock\n");
 > +		/*
 > +		 * Don't allow the user to think they can get
 > +		 * full NO_HZ with this machine.
 > +		 */
 > +		WARN_ONCE(1, "NO_HZ FULL will not work with unstable sched clock");
 >  		return false;
 
So I guess you guys never want this to be enabled on distro kernels ?
If that's the case, can you add something to that effect in Kconfig ?

	Dave

Re: nohz: Warn if the machine can not perform nohz_full

[Frederic Weisbecker]

On Mon, Jul 15, 2013 at 01:08:59PM -0400, Dave Jones wrote:
>  > Gitweb:     http://git.kernel.org/linus/;a=commit;h=e12d0271774fea9fddf1e2a7952a0bffb2ee8e8b
>  > Commit:     e12d0271774fea9fddf1e2a7952a0bffb2ee8e8b
>  > Parent:     7d132055814ef17a6c7b69f342244c410a5e000f
>  > Author:     Steven Rostedt <rostedt@goodmis.org>
>  > AuthorDate: Fri May 10 17:12:28 2013 -0400
>  > Committer:  Frederic Weisbecker <fweisbec@gmail.com>
>  > CommitDate: Thu Jun 20 01:15:51 2013 +0200
>  > 
>  >     nohz: Warn if the machine can not perform nohz_full
>  >     
>  >     If the user configures NO_HZ_FULL and defines nohz_full=XXX on the
>  >     kernel command line, or enables NO_HZ_FULL_ALL, but nohz fails
>  >     due to the machine having a unstable clock, warn about it.
>  >     
>  >     We do not want users thinking that they are getting the benefit
>  >     of nohz when their machine can not support it.
>  > ---
>  >  kernel/time/tick-sched.c |    5 +++++
>  >  1 files changed, 5 insertions(+), 0 deletions(-)
>  > 
>  > diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
>  > index f420813..d87d22c 100644
>  > --- a/kernel/time/tick-sched.c
>  > +++ b/kernel/time/tick-sched.c
>  > @@ -178,6 +178,11 @@ static bool can_stop_full_tick(void)
>  >  	 */
>  >  	if (!sched_clock_stable) {
>  >  		trace_tick_stop(0, "unstable sched clock\n");
>  > +		/*
>  > +		 * Don't allow the user to think they can get
>  > +		 * full NO_HZ with this machine.
>  > +		 */
>  > +		WARN_ONCE(1, "NO_HZ FULL will not work with unstable sched clock");
>  >  		return false;
>  
> So I guess you guys never want this to be enabled on distro kernels ?
> If that's the case, can you add something to that effect in Kconfig ?

I believe we want it to be enabled on distros in the long term. But right now it would
be a bad idea until the off case (nohz_full= parameter empty) is carefully optimized.
I'm currently working on that.

Now for the unstable tsc, which is what it's about on the above code block, we need
the tick to be there to leverage the sched clock madness. May be there could be some
other solution that could work along full dynticks but for now we chose the easy path.

Are broken TSCs that common?

Also what is the preffered way to tell the distros that they shouldn't enable that option
for now? Here is what we currently have in the tail of the related Kconfig help:

        This is implemented at the expense of some overhead in user <-> kernel
        transitions: syscalls, exceptions and interrupts. Even when it's
        dynamically off.

        Say N.

Re: nohz: Warn if the machine can not perform nohz_full

[Dave Jones]

On Mon, Jul 15, 2013 at 07:18:02PM +0200, Frederic Weisbecker wrote:
 
 > > So I guess you guys never want this to be enabled on distro kernels ?
 > > If that's the case, can you add something to that effect in Kconfig ?
 > 
 > I believe we want it to be enabled on distros in the long term. But right now it would
 > be a bad idea until the off case (nohz_full= parameter empty) is carefully optimized.
 > I'm currently working on that.
 > 
 > Now for the unstable tsc, which is what it's about on the above code block, we need
 > the tick to be there to leverage the sched clock madness. May be there could be some
 > other solution that could work along full dynticks but for now we chose the easy path.
 > 
 > Are broken TSCs that common?
 
I just hit one apparently.  http://paste.fedoraproject.org/25421/73907845/raw/
That's a fairly recent Atom board, so I suspect it's not uncommon on that platform.

 > Also what is the preffered way to tell the distros that they shouldn't enable that option
 > for now? Here is what we currently have in the tail of the related Kconfig help:
 > 
 >         This is implemented at the expense of some overhead in user <-> kernel
 >         transitions: syscalls, exceptions and interrupts. Even when it's
 >         dynamically off.

"This feature is not ready to be deployed" ?

"This will taint the kernel if it decides it can't work" ?

	Dave

Re: nohz: Warn if the machine can not perform nohz_full

[Dave Jones]

On Mon, Jul 15, 2013 at 01:24:23PM -0400, Dave Jones wrote:
 > On Mon, Jul 15, 2013 at 07:18:02PM +0200, Frederic Weisbecker wrote:
 >  
 >  > > So I guess you guys never want this to be enabled on distro kernels ?
 >  > > If that's the case, can you add something to that effect in Kconfig ?
 >  > 
 >  > I believe we want it to be enabled on distros in the long term. But right now it would
 >  > be a bad idea until the off case (nohz_full= parameter empty) is carefully optimized.
 >  > I'm currently working on that.
 >  > 
 >  > Now for the unstable tsc, which is what it's about on the above code block, we need
 >  > the tick to be there to leverage the sched clock madness. May be there could be some
 >  > other solution that could work along full dynticks but for now we chose the easy path.
 >  > 
 >  > Are broken TSCs that common?
 >  
 > I just hit one apparently.  http://paste.fedoraproject.org/25421/73907845/raw/
 > That's a fairly recent Atom board, so I suspect it's not uncommon on that platform.

And here's a Core Duo from circa 2008.
http://paste.fedoraproject.org/25429/13739098/raw

Two for two so far. I get the feeling you guys are going to get a ton of these reports.

	Dave

Re: nohz: Warn if the machine can not perform nohz_full

[Steven Rostedt]

On Mon, 2013-07-15 at 13:38 -0400, Dave Jones wrote:
> On Mon, Jul 15, 2013 at 01:24:23PM -0400, Dave Jones wrote:
>  > On Mon, Jul 15, 2013 at 07:18:02PM +0200, Frederic Weisbecker wrote:
>  >  
>  >  > > So I guess you guys never want this to be enabled on distro kernels ?
>  >  > > If that's the case, can you add something to that effect in Kconfig ?
>  >  > 
>  >  > I believe we want it to be enabled on distros in the long term. But right now it would
>  >  > be a bad idea until the off case (nohz_full= parameter empty) is carefully optimized.
>  >  > I'm currently working on that.
>  >  > 
>  >  > Now for the unstable tsc, which is what it's about on the above code block, we need
>  >  > the tick to be there to leverage the sched clock madness. May be there could be some
>  >  > other solution that could work along full dynticks but for now we chose the easy path.
>  >  > 
>  >  > Are broken TSCs that common?
>  >  
>  > I just hit one apparently.  http://paste.fedoraproject.org/25421/73907845/raw/
>  > That's a fairly recent Atom board, so I suspect it's not uncommon on that platform.
> 
> And here's a Core Duo from circa 2008.
> http://paste.fedoraproject.org/25429/13739098/raw
> 
> Two for two so far. I get the feeling you guys are going to get a ton of these reports.
> 

Hmm, we should only warn if the user tried to enable nohz-full via the
command line. But it looks like it warns even without enabling
nohz-full, which wasn't the desired effect.

I'll look at this, and send a patch to make sure the warning only
happens when the user tries to use nohz-full, and doesn't just compile
it in. The point of the patch is to not let the user think they have
nohz-full when they don't.

-- Steve

Re: nohz: Warn if the machine can not perform nohz_full

[Dave Jones]

On Mon, Jul 15, 2013 at 02:49:42PM -0400, Steven Rostedt wrote:
 
 > > And here's a Core Duo from circa 2008.
 > > http://paste.fedoraproject.org/25429/13739098/raw
 > > 
 > > Two for two so far. I get the feeling you guys are going to get a ton of these reports.
 > 
 > Hmm, we should only warn if the user tried to enable nohz-full via the
 > command line. But it looks like it warns even without enabling
 > nohz-full, which wasn't the desired effect.
 > 
 > I'll look at this, and send a patch to make sure the warning only
 > happens when the user tries to use nohz-full, and doesn't just compile
 > it in. The point of the patch is to not let the user think they have
 > nohz-full when they don't.

Is a printk not enough for that purpose ? Tainting the kernel is kinda anti-social.

	Dave

Re: nohz: Warn if the machine can not perform nohz_full

[Steven Rostedt]

On Mon, 2013-07-15 at 14:56 -0400, Dave Jones wrote:
> On Mon, Jul 15, 2013 at 02:49:42PM -0400, Steven Rostedt wrote:
>  
> Is a printk not enough for that purpose ? Tainting the kernel is kinda anti-social.
> 

printk's don't usually get peoples attention. Taints and warnings do. If
the hardware doesn't allow for nohz-full, and the only way to enable it
is via kernel command line, how do you scream to the user that it isn't
going to work.

We could do a large banner saying:

***************************************************************
***************************************************************
***************************************************************
***************************************************************
***                    UNSTABLE TSC                         ***
***                NO_HZ_FULL disabled                      ***
***************************************************************
***************************************************************
***************************************************************
***************************************************************


And not do the warning. Maybe that will get peoples attentions?

But then again, this could be lost in the boot up if the box prints a
lot of data.

-- Steve

Re: nohz: Warn if the machine can not perform nohz_full

[Frederic Weisbecker]

On Mon, Jul 15, 2013 at 03:30:20PM -0400, Steven Rostedt wrote:
> On Mon, 2013-07-15 at 14:56 -0400, Dave Jones wrote:
> > On Mon, Jul 15, 2013 at 02:49:42PM -0400, Steven Rostedt wrote:
> >  
> > Is a printk not enough for that purpose ? Tainting the kernel is kinda anti-social.
> > 
> 
> printk's don't usually get peoples attention. Taints and warnings do. If
> the hardware doesn't allow for nohz-full, and the only way to enable it
> is via kernel command line, how do you scream to the user that it isn't
> going to work.
> 
> We could do a large banner saying:
> 
> ***************************************************************
> ***************************************************************
> ***************************************************************
> ***************************************************************
> ***                    UNSTABLE TSC                         ***
> ***                NO_HZ_FULL disabled                      ***
> ***************************************************************
> ***************************************************************
> ***************************************************************
> ***************************************************************

May be we can try that. In fact I have a pending patch that converts the
stacktrace warning to a one line printk message, as Ingo reported that issue
to me. But the risk is that it can be indeed lost in the flow.

> 
> 
> And not do the warning. Maybe that will get peoples attentions?
> 
> But then again, this could be lost in the boot up if the box prints a
> lot of data.

Well the above example is unlikely to be missed. If it is, then I believe a
traditional warning would be lost as well.

> 
> -- Steve
> 
> 

Re: nohz: Warn if the machine can not perform nohz_full

[Frederic Weisbecker]

On Mon, Jul 15, 2013 at 02:49:42PM -0400, Steven Rostedt wrote:
> On Mon, 2013-07-15 at 13:38 -0400, Dave Jones wrote:
> > On Mon, Jul 15, 2013 at 01:24:23PM -0400, Dave Jones wrote:
> >  > On Mon, Jul 15, 2013 at 07:18:02PM +0200, Frederic Weisbecker wrote:
> >  >  
> >  >  > > So I guess you guys never want this to be enabled on distro kernels ?
> >  >  > > If that's the case, can you add something to that effect in Kconfig ?
> >  >  > 
> >  >  > I believe we want it to be enabled on distros in the long term. But right now it would
> >  >  > be a bad idea until the off case (nohz_full= parameter empty) is carefully optimized.
> >  >  > I'm currently working on that.
> >  >  > 
> >  >  > Now for the unstable tsc, which is what it's about on the above code block, we need
> >  >  > the tick to be there to leverage the sched clock madness. May be there could be some
> >  >  > other solution that could work along full dynticks but for now we chose the easy path.
> >  >  > 
> >  >  > Are broken TSCs that common?
> >  >  
> >  > I just hit one apparently.  http://paste.fedoraproject.org/25421/73907845/raw/
> >  > That's a fairly recent Atom board, so I suspect it's not uncommon on that platform.
> > 
> > And here's a Core Duo from circa 2008.
> > http://paste.fedoraproject.org/25429/13739098/raw
> > 
> > Two for two so far. I get the feeling you guys are going to get a ton of these reports.
> > 
> 
> Hmm, we should only warn if the user tried to enable nohz-full via the
> command line. But it looks like it warns even without enabling
> nohz-full, which wasn't the desired effect.
> 
> I'll look at this, and send a patch to make sure the warning only
> happens when the user tries to use nohz-full, and doesn't just compile
> it in. The point of the patch is to not let the user think they have
> nohz-full when they don't.

Great! I'll wait for your patch then.

[PATCH] nohz: Do not warn about unstable tsc unless user uses nohz_full

[Steven Rostedt]

If the user enables CONFIG_NO_HZ_FULL and runs the kernel on a machine
with an unstable TSC, it will produce a WARN_ON dump as well as taint
the kernel. This is a bit extreme for a kernel that just enables a
feature but doesn't use it.

The warning should only happen if the user tries to use the feature by
either adding nohz_full to the kernel command line, or by enabling
CONFIG_NO_HZ_FULL_ALL that makes nohz used on all CPUs at boot up. Note,
this second feature should not (yet) be used by distros or anyone that
doesn't care if NO_HZ is used or not.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 6960172..6f47049 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -182,7 +182,8 @@ static bool can_stop_full_tick(void)
 		 * Don't allow the user to think they can get
 		 * full NO_HZ with this machine.
 		 */
-		WARN_ONCE(1, "NO_HZ FULL will not work with unstable sched clock");
+		WARN_ONCE(have_nohz_full_mask,
+			  "NO_HZ FULL will not work with unstable sched clock");
 		return false;
 	}
 #endif

Re: [PATCH] nohz: Do not warn about unstable tsc unless user uses nohz_full

[Frederic Weisbecker]

On Tue, Jul 16, 2013 at 10:22:12AM -0400, Steven Rostedt wrote:
> If the user enables CONFIG_NO_HZ_FULL and runs the kernel on a machine
> with an unstable TSC, it will produce a WARN_ON dump as well as taint
> the kernel. This is a bit extreme for a kernel that just enables a
> feature but doesn't use it.
> 
> The warning should only happen if the user tries to use the feature by
> either adding nohz_full to the kernel command line, or by enabling
> CONFIG_NO_HZ_FULL_ALL that makes nohz used on all CPUs at boot up. Note,
> this second feature should not (yet) be used by distros or anyone that
> doesn't care if NO_HZ is used or not.
> 
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> 
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 6960172..6f47049 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -182,7 +182,8 @@ static bool can_stop_full_tick(void)
>  		 * Don't allow the user to think they can get
>  		 * full NO_HZ with this machine.
>  		 */
> -		WARN_ONCE(1, "NO_HZ FULL will not work with unstable sched clock");
> +		WARN_ONCE(have_nohz_full_mask,
> +			  "NO_HZ FULL will not work with unstable sched clock");

Seems good, indeed the warning and taint can be justified if the user filled the
nohz_full mask.

Ok I'll apply this and let the last word to Ingo in a pull request.

Thanks.

>  		return false;
>  	}
>  #endif
> 
> 

Re: nohz: Warn if the machine can not perform nohz_full

[Frederic Weisbecker]

On Mon, Jul 15, 2013 at 01:38:48PM -0400, Dave Jones wrote:
> On Mon, Jul 15, 2013 at 01:24:23PM -0400, Dave Jones wrote:
>  > On Mon, Jul 15, 2013 at 07:18:02PM +0200, Frederic Weisbecker wrote:
>  >  
>  >  > > So I guess you guys never want this to be enabled on distro kernels ?
>  >  > > If that's the case, can you add something to that effect in Kconfig ?
>  >  > 
>  >  > I believe we want it to be enabled on distros in the long term. But right now it would
>  >  > be a bad idea until the off case (nohz_full= parameter empty) is carefully optimized.
>  >  > I'm currently working on that.
>  >  > 
>  >  > Now for the unstable tsc, which is what it's about on the above code block, we need
>  >  > the tick to be there to leverage the sched clock madness. May be there could be some
>  >  > other solution that could work along full dynticks but for now we chose the easy path.
>  >  > 
>  >  > Are broken TSCs that common?
>  >  
>  > I just hit one apparently.  http://paste.fedoraproject.org/25421/73907845/raw/
>  > That's a fairly recent Atom board, so I suspect it's not uncommon on that platform.
> 
> And here's a Core Duo from circa 2008.
> http://paste.fedoraproject.org/25429/13739098/raw
> 
> Two for two so far. I get the feeling you guys are going to get a ton of these reports.

Definetly, so it comforts me on the fact we need to remove the warning, which may be
interpreted as the symptom of a bug to report while it's not.

We need to warn the user but through another way.

Re: nohz: Warn if the machine can not perform nohz_full

[Frederic Weisbecker]

On Mon, Jul 15, 2013 at 01:24:23PM -0400, Dave Jones wrote:
> On Mon, Jul 15, 2013 at 07:18:02PM +0200, Frederic Weisbecker wrote:
>  
>  > > So I guess you guys never want this to be enabled on distro kernels ?
>  > > If that's the case, can you add something to that effect in Kconfig ?
>  > 
>  > I believe we want it to be enabled on distros in the long term. But right now it would
>  > be a bad idea until the off case (nohz_full= parameter empty) is carefully optimized.
>  > I'm currently working on that.
>  > 
>  > Now for the unstable tsc, which is what it's about on the above code block, we need
>  > the tick to be there to leverage the sched clock madness. May be there could be some
>  > other solution that could work along full dynticks but for now we chose the easy path.
>  > 
>  > Are broken TSCs that common?
>  
> I just hit one apparently.  http://paste.fedoraproject.org/25421/73907845/raw/
> That's a fairly recent Atom board, so I suspect it's not uncommon on that platform.
> 
>  > Also what is the preffered way to tell the distros that they shouldn't enable that option
>  > for now? Here is what we currently have in the tail of the related Kconfig help:
>  > 
>  >         This is implemented at the expense of some overhead in user <-> kernel
>  >         transitions: syscalls, exceptions and interrupts. Even when it's
>  >         dynamically off.
> 
> "This feature is not ready to be deployed" ?

I can try this one. Or may be I should be more direct and put:

"This feature is not ready to be deployed on distros"

> 
> "This will taint the kernel if it decides it can't work" ?
> 
> 	Dave
>