[patch] fix ptrace slowness

[patch] fix ptrace slowness

[Miklos Szeredi]

This one incorporates comments from Oleg and Ingo.  Please apply to
2.6.29 and 2.6.2[78]-stable trees.

Thanks,
Miklos
----

From: Miklos Szeredi <mszeredi@suse.cz>

This patch fixes bug #12208:

  Bug-Entry       : http://bugzilla.kernel.org/show_bug.cgi?id=12208
  Subject         : uml is very slow on 2.6.28 host

This turned out to be not a scheduler regression, but an already
existing problem in ptrace being triggered by subtle scheduler
changes.

The problem is this:

 - task A is ptracing task B
 - task B stops on a trace event
 - task A is woken up and preempts task B
 - task A calls ptrace on task B, which does ptrace_check_attach()
 - this calls wait_task_inactive(), which sees that task B is still on the runq
 - task A goes to sleep for a jiffy
 - ...

Since UML does lots of the above sequences, those jiffies quickly add
up to make it slow as hell.

This patch solves this by not rescheduling in read_unlock() after
ptrace_stop() has woken up the tracer.

Thanks to Oleg Nesterov and Ingo Molnar for the feedback.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
CC: stable@kernel.org
---
 kernel/signal.c |    8 ++++++++
 1 file changed, 8 insertions(+)

Index: linux.git/kernel/signal.c
===================================================================
--- linux.git.orig/kernel/signal.c	2009-03-20 09:41:04.000000000 +0100
+++ linux.git/kernel/signal.c	2009-03-23 15:40:57.000000000 +0100
@@ -1575,7 +1575,15 @@ static void ptrace_stop(int exit_code, i
 	read_lock(&tasklist_lock);
 	if (may_ptrace_stop()) {
 		do_notify_parent_cldstop(current, CLD_TRAPPED);
+		/*
+		 * Don't want to allow preemption here, because
+		 * sys_ptrace() needs this task to be inactive.
+		 *
+		 * XXX: implement read_unlock_no_resched().
+		 */
+		preempt_disable();
 		read_unlock(&tasklist_lock);
+		preempt_enable_no_resched();
 		schedule();
 	} else {
 		/*

Re: [patch] fix ptrace slowness

[Ingo Molnar]

* Miklos Szeredi <miklos@szeredi.hu> wrote:

> This one incorporates comments from Oleg and Ingo.  Please apply 
> to 2.6.29 and 2.6.2[78]-stable trees.

The fix first needs to go upstream. There's an alternative patch 
below. Would you mind to give it a test? Chances are that it will 
make UML even faster than your fix.

	Ingo

diff --git a/kernel/sched.c b/kernel/sched.c
index 3e827b8..2d60f23 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -2119,7 +2119,8 @@ unsigned long wait_task_inactive(struct task_struct *p, long match_state)
 		 * yield - it could be a while.
 		 */
 		if (unlikely(on_rq)) {
-			schedule_timeout_uninterruptible(1);
+			cpu_relax();
+			cond_resched();
 			continue;
 		}
 

Re: [patch] fix ptrace slowness

[Oleg Nesterov]

On 03/23, Ingo Molnar wrote:
>
> There's an alternative patch
> below. Would you mind to give it a test? Chances are that it will
> make UML even faster than your fix.
>
> 	Ingo
>
> diff --git a/kernel/sched.c b/kernel/sched.c
> index 3e827b8..2d60f23 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -2119,7 +2119,8 @@ unsigned long wait_task_inactive(struct task_struct *p, long match_state)
>  		 * yield - it could be a while.
>  		 */
>  		if (unlikely(on_rq)) {
> -			schedule_timeout_uninterruptible(1);
> +			cpu_relax();
> +			cond_resched();

What if the caller is a realtime task? We can spin "forever", no?

Oleg.

Re: [patch] fix ptrace slowness

[Ingo Molnar]

* Oleg Nesterov <oleg@redhat.com> wrote:

> On 03/23, Ingo Molnar wrote:
> >
> > There's an alternative patch
> > below. Would you mind to give it a test? Chances are that it will
> > make UML even faster than your fix.
> >
> > 	Ingo
> >
> > diff --git a/kernel/sched.c b/kernel/sched.c
> > index 3e827b8..2d60f23 100644
> > --- a/kernel/sched.c
> > +++ b/kernel/sched.c
> > @@ -2119,7 +2119,8 @@ unsigned long wait_task_inactive(struct task_struct *p, long match_state)
> >  		 * yield - it could be a while.
> >  		 */
> >  		if (unlikely(on_rq)) {
> > -			schedule_timeout_uninterruptible(1);
> > +			cpu_relax();
> > +			cond_resched();
> 
> What if the caller is a realtime task? We can spin "forever", no?

hm, yes. I sure should have noticed _that_ ;-)

	Ingo

Re: [patch] fix ptrace slowness

[Miklos Szeredi]

On Mon, 23 Mar 2009, Ingo Molnar wrote:
> * Miklos Szeredi <miklos@szeredi.hu> wrote:
> 
> > This one incorporates comments from Oleg and Ingo.  Please apply 
> > to 2.6.29 and 2.6.2[78]-stable trees.
> 
> The fix first needs to go upstream. There's an alternative patch 
> below. Would you mind to give it a test? Chances are that it will 
> make UML even faster than your fix.

Just the opposite.  With my patch a UML image boots in 17 seconnds,
with your patch it boots in 33 seconds.  Without either patch it boots
in about 5 minutes.

Thanks,
Miklos

> 
> 	Ingo
> 
> diff --git a/kernel/sched.c b/kernel/sched.c
> index 3e827b8..2d60f23 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -2119,7 +2119,8 @@ unsigned long wait_task_inactive(struct task_struct *p, long match_state)
>  		 * yield - it could be a while.
>  		 */
>  		if (unlikely(on_rq)) {
> -			schedule_timeout_uninterruptible(1);
> +			cpu_relax();
> +			cond_resched();
>  			continue;
>  		}
>  
> 

Re: [patch] fix ptrace slowness

[Ingo Molnar]

* Miklos Szeredi <miklos@szeredi.hu> wrote:

> On Mon, 23 Mar 2009, Ingo Molnar wrote:
> > * Miklos Szeredi <miklos@szeredi.hu> wrote:
> > 
> > > This one incorporates comments from Oleg and Ingo.  Please apply 
> > > to 2.6.29 and 2.6.2[78]-stable trees.
> > 
> > The fix first needs to go upstream. There's an alternative patch 
> > below. Would you mind to give it a test? Chances are that it 
> > will make UML even faster than your fix.
> 
> Just the opposite.  With my patch a UML image boots in 17 
> seconnds, with your patch it boots in 33 seconds.  Without either 
> patch it boots in about 5 minutes.

okay - i've queued up your fix.

	Ingo

[tip:sched/urgent] sched, ptrace: fix UML and ptrace slowness

[Miklos Szeredi]

Commit-ID:  84eef8ca758fa4a68c29f7d752376f6ca6872383
Gitweb:     http://git.kernel.org/tip/84eef8ca758fa4a68c29f7d752376f6ca6872383
Author:     Miklos Szeredi <miklos@szeredi.hu>
AuthorDate: Mon, 23 Mar 2009 16:07:24 +0100
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 23 Mar 2009 17:37:38 +0100

sched, ptrace: fix UML and ptrace slowness

This patch fixes bug #12208:

  Bug-Entry       : http://bugzilla.kernel.org/show_bug.cgi?id=12208
  Subject         : uml is very slow on 2.6.28 host

This turned out to be not a scheduler regression, but an already
existing problem in ptrace being triggered by subtle scheduler
changes.

The problem is this:

 - task A is ptracing task B
 - task B stops on a trace event
 - task A is woken up and preempts task B
 - task A calls ptrace on task B, which does ptrace_check_attach()
 - this calls wait_task_inactive(), which sees that task B is still on the runq
 - task A goes to sleep for a jiffy
 - ...

Since UML does lots of the above sequences, those jiffies quickly add
up to make it slow as hell.

This patch solves this by not rescheduling in read_unlock() after
ptrace_stop() has woken up the tracer.

Thanks to Oleg Nesterov and Ingo Molnar for the feedback.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
Cc: oleg@redhat.com
Cc: peterz@infradead.org
Cc: roland@redhat.com
Cc: efault@gmx.de
Cc: rjw@sisk.pl
Cc: jdike@addtoit.com
Cc: user-mode-linux-devel@lists.sourceforge.net
Cc: torvalds@linux-foundation.org
Cc: akpm@linux-foundation.org
Cc: <stable@kernel.org>
LKML-Reference: <E1LllkK-0000zF-1h@pomaz-ex.szeredi.hu>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 kernel/signal.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 2a74fe8..1c88144 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1575,7 +1575,15 @@ static void ptrace_stop(int exit_code, int clear_code, siginfo_t *info)
 	read_lock(&tasklist_lock);
 	if (may_ptrace_stop()) {
 		do_notify_parent_cldstop(current, CLD_TRAPPED);
+		/*
+		 * Don't want to allow preemption here, because
+		 * sys_ptrace() needs this task to be inactive.
+		 *
+		 * XXX: implement read_unlock_no_resched().
+		 */
+		preempt_disable();
 		read_unlock(&tasklist_lock);
+		preempt_enable_no_resched();
 		schedule();
 	} else {
 		/*

Re: [tip:sched/urgent] sched, ptrace: fix UML and ptrace slowness

[Ingo Molnar]

* Miklos Szeredi <miklos@szeredi.hu> wrote:

> Commit-ID:  84eef8ca758fa4a68c29f7d752376f6ca6872383
> Gitweb:     http://git.kernel.org/tip/84eef8ca758fa4a68c29f7d752376f6ca6872383
> Author:     Miklos Szeredi <miklos@szeredi.hu>
> AuthorDate: Mon, 23 Mar 2009 16:07:24 +0100
> Committer:  Ingo Molnar <mingo@elte.hu>
> CommitDate: Mon, 23 Mar 2009 17:37:38 +0100
> 
> sched, ptrace: fix UML and ptrace slowness
> 
> This patch fixes bug #12208:
> 
>   Bug-Entry       : http://bugzilla.kernel.org/show_bug.cgi?id=12208
>   Subject         : uml is very slow on 2.6.28 host

Note, i zapped this commit as Linus took it in parallel so it's 
available upstream:

  53da1d9: fix ptrace slowness

	Ingo

Re: [patch] fix ptrace slowness

[Michael Riepe]

Hi!

Miklos Szeredi wrote:
> On Mon, 23 Mar 2009, Ingo Molnar wrote:
> 
>>* Miklos Szeredi <miklos@szeredi.hu> wrote:
>>
>>
>>>This one incorporates comments from Oleg and Ingo.  Please apply 
>>>to 2.6.29 and 2.6.2[78]-stable trees.
>>
>>The fix first needs to go upstream. There's an alternative patch 
>>below. Would you mind to give it a test? Chances are that it will 
>>make UML even faster than your fix.
> 
> 
> Just the opposite.  With my patch a UML image boots in 17 seconnds,
> with your patch it boots in 33 seconds.  Without either patch it boots
> in about 5 minutes.

My strace test case runs 60% faster with this patch (compared to Ingo's).

-- 
Michael "Tired" Riepe <michael.riepe@googlemail.com>
X-Tired: Each morning I get up I die a little