Re: [PATCH] signal: Revert ptrace preempt magic

From: Eric W. Biederman
Date: Thu May 24 2018 - 11:32:50 EST


Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> writes:

> From: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>
> Upstream commit '53da1d9456fe7f8 fix ptrace slowness' is nothing more
> than a bandaid around the ptrace design trainwreck. It's not a
> correctness issue, it's merily a cosmetic bandaid.

This patch comes with no justification or reason to remove the
``cosmetic bandaid''. The description in 53da1d9456fe ("fix ptrace
slowness") is quite persuasive that there is a real world issue.

So while this may be a good idea to remove this. I don't see any
description of testing to indicate this won't cause uml to regresssion.
Nor do I see any compelling reason except code tidiness to remove this.

As such until adequate descriptions can be provideded.

Nacked-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>


> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> ---
> kernel/signal.c | 8 --------
> 1 file changed, 8 deletions(-)
>
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -1876,15 +1876,7 @@ static void ptrace_stop(int exit_code, i
> if (gstop_done && ptrace_reparented(current))
> do_notify_parent_cldstop(current, false, why);
>
> - /*
> - * 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();
> freezable_schedule();
> } else {
> /*