Slow down stupid pounders

Slow down stupid pounders

[Borislav Petkov]

Hi,

so I'm getting sick'n'tired of all those bug reports of people pounding
cpu hotplug with stupid scripts.

* We know cpu hotplug is fragile/buggy/crap/needs proper rewrite.

* Stupid hotplugging script doesn't resemble any real use case - go use
a real benchmark/stress test to trigger bugs.

So if we can't make pounders stop jerking off, let's make it
uninterestingly slow. Stupid patch below, it might be completely idiotic
to do it this way but at least starts the discussion about this being a
really annoying issue which needs some sort of dealing with.

I dunno, we can make it configurable (which will probably defeat its
purpose partially), we can do some more fancy ratelimiting, per cpu,
whatever... we'll see.

Opinions, flames?

---
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 006b1bc5297d..615c7af767ed 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -40,6 +40,11 @@ static void change_cpu_under_node(struct cpu *cpu,
 	cpu->node_id = to_nid;
 }
 
+static void delay_hotplug(void)
+{
+	schedule_timeout_uninterruptible(msecs_to_jiffies(MSEC_PER_SEC));
+}
+
 static int __ref cpu_subsys_online(struct device *dev)
 {
 	struct cpu *cpu = container_of(dev, struct cpu, dev);
@@ -47,6 +52,8 @@ static int __ref cpu_subsys_online(struct device *dev)
 	int from_nid, to_nid;
 	int ret;
 
+	delay_hotplug();
+
 	from_nid = cpu_to_node(cpuid);
 	if (from_nid == NUMA_NO_NODE)
 		return -ENODEV;
@@ -65,6 +72,8 @@ static int __ref cpu_subsys_online(struct device *dev)
 
 static int cpu_subsys_offline(struct device *dev)
 {
+	delay_hotplug();
+
 	return cpu_down(dev->id);
 }
 

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: Slow down stupid pounders

[Steven Rostedt]

On Tue, 6 May 2014 12:29:24 +0200
Borislav Petkov <bp@alien8.de> wrote:

> Hi,
> 
> so I'm getting sick'n'tired of all those bug reports of people pounding
> cpu hotplug with stupid scripts.
> 
> * We know cpu hotplug is fragile/buggy/crap/needs proper rewrite.
> 
> * Stupid hotplugging script doesn't resemble any real use case - go use
> a real benchmark/stress test to trigger bugs.
> 
> So if we can't make pounders stop jerking off, let's make it
> uninterestingly slow. Stupid patch below, it might be completely idiotic
> to do it this way but at least starts the discussion about this being a
> really annoying issue which needs some sort of dealing with.

Hey, I'm one of those that jerk off to CPU hotplug stress test scripts!

> 
> I dunno, we can make it configurable (which will probably defeat its
> purpose partially), we can do some more fancy ratelimiting, per cpu,
> whatever... we'll see.
> 
> Opinions, flames?

We were just bitching about this yesterday, but -rt related. As
anything crap in mainline just becomes exponentially more crap in -rt.
The CPU hotplug case becomes a mountain of crap that we just dig
tunnels through to get by.

A while ago Thomas had a proof of concept patch I believe that was
suppose to pick up traction but unfortunately never went anywhere, even
after being told it would.

The real option is to rewrite cpu hotplug.

-- Steve


> 
> ---
> diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
> index 006b1bc5297d..615c7af767ed 100644
> --- a/drivers/base/cpu.c
> +++ b/drivers/base/cpu.c
> @@ -40,6 +40,11 @@ static void change_cpu_under_node(struct cpu *cpu,
>  	cpu->node_id = to_nid;
>  }
>  
> +static void delay_hotplug(void)
> +{
> +	schedule_timeout_uninterruptible(msecs_to_jiffies(MSEC_PER_SEC));
> +}
> +
>  static int __ref cpu_subsys_online(struct device *dev)
>  {
>  	struct cpu *cpu = container_of(dev, struct cpu, dev);
> @@ -47,6 +52,8 @@ static int __ref cpu_subsys_online(struct device *dev)
>  	int from_nid, to_nid;
>  	int ret;
>  
> +	delay_hotplug();
> +
>  	from_nid = cpu_to_node(cpuid);
>  	if (from_nid == NUMA_NO_NODE)
>  		return -ENODEV;
> @@ -65,6 +72,8 @@ static int __ref cpu_subsys_online(struct device *dev)
>  
>  static int cpu_subsys_offline(struct device *dev)
>  {
> +	delay_hotplug();
> +
>  	return cpu_down(dev->id);
>  }
>  
> 

Re: Slow down stupid pounders

[Borislav Petkov]

On Tue, May 06, 2014 at 07:08:54AM -0400, Steven Rostedt wrote:
> Hey, I'm one of those that jerk off to CPU hotplug stress test scripts!

I know you are, Mike gave me your script. Like you haven't done enough!

:-P

> We were just bitching about this yesterday, but -rt related. As
> anything crap in mainline just becomes exponentially more crap in -rt.
> The CPU hotplug case becomes a mountain of crap that we just dig
> tunnels through to get by.
> 
> A while ago Thomas had a proof of concept patch I believe that was
> suppose to pick up traction but unfortunately never went anywhere, even
> after being told it would.
> 
> The real option is to rewrite cpu hotplug.

I know, and we might get lucky, after all.

I'm proposing this as a temporary thing until we have the real deal in
place. I simply would like to shoo those pounding beaks in the interim.

:-)

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: Slow down stupid pounders

[Mike Galbraith]

On Tue, 2014-05-06 at 12:29 +0200, Borislav Petkov wrote: 
> Hi,
> 
> so I'm getting sick'n'tired of all those bug reports of people pounding
> cpu hotplug with stupid scripts.
> 
> * We know cpu hotplug is fragile/buggy/crap/needs proper rewrite.
> 
> * Stupid hotplugging script doesn't resemble any real use case - go use
> a real benchmark/stress test to trigger bugs.
> 
> So if we can't make pounders stop jerking off, let's make it
> uninterestingly slow. Stupid patch below, it might be completely idiotic
> to do it this way but at least starts the discussion about this being a
> really annoying issue which needs some sort of dealing with.
> 
> I dunno, we can make it configurable (which will probably defeat its
> purpose partially), we can do some more fancy ratelimiting, per cpu,
> whatever... we'll see.
> 
> Opinions, flames?

Damn.  I was gonna suggest we shorten hours in -rt kernels to achieve my
full day stress testing survival goal.

-Mike

Re: Slow down stupid pounders

[Mike Galbraith]

On Tue, 2014-05-06 at 12:29 +0200, Borislav Petkov wrote: 
> Hi,
> 
> so I'm getting sick'n'tired of all those bug reports of people pounding
> cpu hotplug with stupid scripts.
> 
> * We know cpu hotplug is fragile/buggy/crap/needs proper rewrite.
> 
> * Stupid hotplugging script doesn't resemble any real use case - go use
> a real benchmark/stress test to trigger bugs.
> 
> So if we can't make pounders stop jerking off, let's make it
> uninterestingly slow. Stupid patch below, it might be completely idiotic
> to do it this way but at least starts the discussion about this being a
> really annoying issue which needs some sort of dealing with.
> 
> I dunno, we can make it configurable (which will probably defeat its
> purpose partially), we can do some more fancy ratelimiting, per cpu,
> whatever... we'll see.
> 
> Opinions, flames?

Rather than ratelimit the interface perhaps send a ratelimited note
telling the user straight up that he's begging for trouble.  We can
ignore it when regression testing, and point it out to Joe Pounder
should he fail to notice.. or be somewhat dense.

-Mike

Re: Slow down stupid pounders

[Borislav Petkov]

On Tue, May 06, 2014 at 05:08:19PM +0200, Mike Galbraith wrote:
> Rather than ratelimit the interface perhaps send a ratelimited note
> telling the user straight up that he's begging for trouble. We can
> ignore it when regression testing, and point it out to Joe Pounder
> should he fail to notice.. or be somewhat dense.

Yeah, if those dudes don't stare at dmesg, that note doesn't bring us
whit.

Alternatively, we could do

	add_taint(TAINT_HOTPLUG_POUNDER)

when the rate is exceeded...

I'd rather like to slow it down for a second, though, if it doesn't hurt
any sensible use cases in doing so.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: Slow down stupid pounders

[Pavel Machek]

On Tue 2014-05-06 18:10:04, Borislav Petkov wrote:
> On Tue, May 06, 2014 at 05:08:19PM +0200, Mike Galbraith wrote:
> > Rather than ratelimit the interface perhaps send a ratelimited note
> > telling the user straight up that he's begging for trouble. We can
> > ignore it when regression testing, and point it out to Joe Pounder
> > should he fail to notice.. or be somewhat dense.
> 
> Yeah, if those dudes don't stare at dmesg, that note doesn't bring us
> whit.
> 
> Alternatively, we could do
> 
> 	add_taint(TAINT_HOTPLUG_POUNDER)
> 
> when the rate is exceeded...
> 
> I'd rather like to slow it down for a second, though, if it doesn't hurt
> any sensible use cases in doing so.

Actually, you should just taint as soon as someone tries to use cpu hotplug if 
it does not work. 

I wonder what happens on those multi-core android phones. They suspend multiple 
times a second. Do they do cpu hotplugs at that point?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html