2009-03-23 15:07:49

by Miklos Szeredi

[permalink] [raw]
Subject: [patch] fix ptrace slowness

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 <[email protected]>

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 <[email protected]>
CC: [email protected]
---
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 {
/*


2009-03-23 15:22:25

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch] fix ptrace slowness


* Miklos Szeredi <[email protected]> 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;
}

2009-03-23 16:03:48

by Oleg Nesterov

[permalink] [raw]
Subject: Re: [patch] fix ptrace slowness

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.

2009-03-23 16:16:18

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch] fix ptrace slowness


* Oleg Nesterov <[email protected]> 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

2009-03-23 16:17:30

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [patch] fix ptrace slowness

On Mon, 23 Mar 2009, Ingo Molnar wrote:
> * Miklos Szeredi <[email protected]> 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;
> }
>
>

2009-03-23 16:40:33

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch] fix ptrace slowness


* Miklos Szeredi <[email protected]> wrote:

> On Mon, 23 Mar 2009, Ingo Molnar wrote:
> > * Miklos Szeredi <[email protected]> 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

2009-03-23 16:41:15

by Miklos Szeredi

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

Commit-ID: 84eef8ca758fa4a68c29f7d752376f6ca6872383
Gitweb: http://git.kernel.org/tip/84eef8ca758fa4a68c29f7d752376f6ca6872383
Author: Miklos Szeredi <[email protected]>
AuthorDate: Mon, 23 Mar 2009 16:07:24 +0100
Committer: Ingo Molnar <[email protected]>
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 <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>


---
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 {
/*

2009-03-23 17:16:11

by Ingo Molnar

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


* Miklos Szeredi <[email protected]> wrote:

> Commit-ID: 84eef8ca758fa4a68c29f7d752376f6ca6872383
> Gitweb: http://git.kernel.org/tip/84eef8ca758fa4a68c29f7d752376f6ca6872383
> Author: Miklos Szeredi <[email protected]>
> AuthorDate: Mon, 23 Mar 2009 16:07:24 +0100
> Committer: Ingo Molnar <[email protected]>
> 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

2009-03-23 19:08:40

by Michael Riepe

[permalink] [raw]
Subject: Re: [patch] fix ptrace slowness

Hi!

Miklos Szeredi wrote:
> On Mon, 23 Mar 2009, Ingo Molnar wrote:
>
>>* Miklos Szeredi <[email protected]> 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 <[email protected]>
X-Tired: Each morning I get up I die a little