Re: [PATCHSET ptrace] ptrace: implement PTRACE_SEIZE/INTERRUPT andgroup stop notification, take#4

[Oleg Nesterov]

Hi Tejun,

On 06/02, Tejun Heo wrote:
>
> I've tested threaded one and INTERRUPT immediate re-triggering and at
> least the apparent cases work fine.  I've re-generated the git tree on
> top of 3.0-rc1 with the updated patches.
>
>  git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git review-ptrace-seize

Everything looks fine to me.

I feel we can cleanup this code a little bit, but we can do this later.

Only one thing. I think it makes sense to discuss the idea from Denys,

> * Which signo to use in exit_code on STOP traps.

Yes. other pending issues can be solved later.

So,

	static void ptrace_do_notify(int exit_code, int why)
	{
		info.si_signo = SIGTRAP;
		info.si_code = __SI_TRAP | exit_code;

		if ((current->ptrace & PT_SEIZED) &&
		    (sig->group_stop_count || sig->flags & SIGNAL_STOP_STOPPED)) {
			info.si_pt_flags |= PTRACE_SI_STOPPED;
			info.si_signo = current->jobctl & JOBCTL_STOP_SIGMASK;
			WARN_ON_ONCE(!info.si_signo);
		}

	}

Can't we set si_pt_flags only if PTRACE_EVENT_STOP? Afaics, this
should be enough to support the jobctl tracking.

If yes, then can't we encode si_pt_flags in task->exit_code which
is "visible" to wait?

IOW, ptrace_do_notify(PTRACE_EVENT_STOP) path should use

	exit_code = signr | PTRACE_EVENT_STOP<<8;

and signr is roughly calculated as

	if (group_stop_count || SIGNAL_STOP_STOPPED)
		signr = jobctl & JOBCTL_STOP_SIGMASK;
	else if (JOBCTL_TRAP_NOTIFY)
		signr = SIGCONT;
	else
		signr =  SIGTRAP; // PTRACE_INTERRUPT

In this case we can avoid all siginfo changes. The tracer does wait(status)
anyway, it can see the state without GETSIGINFO. The only problem, the tracer
should be careful to avoid the confusion with ptrace_signal(), it should
check status & (PTRACE_EVENT_STOP << 16).

What do you think?

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/