[lkml]   [2011]   [May]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH 06/11] ptrace: make group stop state visible via PTRACE_GETSIGINFO
On 05/11, Tejun Heo wrote:
> On Tue, May 10, 2011 at 06:55:45PM +0200, Oleg Nesterov wrote:
> > IOW, if the tracee reports via ptrace_notify*, the tracee can look at
> > si_pt_flags == stop-in-effect. If the tracer reports a signal, the
> > tracer obviously lacks this info, hmm.
> Which indicates tracee is in group stop trap.

What do you mean?

si_pt_flags doesn't "exist" when the tracee reports the signal or
CLD_STOPPED. This doesn't look clean.

> > Probably I need more time to get used to this... But at first glance
> > this looks a bit unnatural. Say, can't we simply implement
> > PTRACE_GET_GROUP_STOP_STATUS request which returns this (and probably
> > more) info?
> I don't know. PTRACE_GETSIGINFO seemed to already fit the bill and I
> want to avoid introducing a new request if at all possible. It sure
> is a bit quirky but doesn't compromisea functionality.

I am not sure too, but the new request is much simpler to use, and it
is more extensible. We can report more info. Say, the state of
JOBCTL_STOP_CONSUME or something else.

> > > if (!ptrace(PTRACE_GETSIGINFO, tracee, NULL, &si)) {
> > > if (si.si_code) {
> > > stopped = !!si.si_status;
> >
> > In this case this "si_code != 0" check is correct, but how can the
> > tracer detect this case in general?
> This was quick hack. Proper test would look like,
> si.si_code && (si.si_pt_flags & PTRACE_SI_STOPPED)

This doesn't look right too? How can we know we can trust si_pt_flags?
This needs some YES_YOU_CAN_CHECK_si_pt_flags(si_code), but I can't
understand what it should do right now...

> > > + /* report whether group stop is in effect w/ SI_STOPPED */
> > > + if (sig->group_stop_count || (sig->flags & SIGNAL_STOP_STOPPED))
> >
> > We have more and more "group_stop_count || SIGNAL_STOP_STOPPED" checks,
> > perhaps we should make a helper. Or at least invent the short name to
> > denote the group-stopped-or-in-progress to simplify the discussions ;)
> Yeah, how about group_stop_in_effect()?

Or may me signal_stop_stopped(struct signal_struct *sig), like
signal_group_exit/SIGNAL_GROUP_EXIT. But I am fine with
group_stop_in_effect, probably it is more explanatorily.

> > Still, this is strange. With this change ptrace_getsiginfo() reports
> > the extra "volatile" info which wasn't reported by the tracee itself.
> > If the tracer does PTRACE_SETSIGINFO twice in a row, it can see the
> > different si_pt_flags's.
> (answering to both get/setsiginfo concerns)
> * I think we better block PTRACE_SETSIGINFO for non signal delivery
> traps. It doesn't make any sense. Let's just fail that with

Oh I agree, it does not make any sense. Should we change the current
behaviour for PT_SEIZED? I don't really care, this looks minor.

> * I don't think PTRACE_GETSIGINFO returning volatile information to be
> problematic. The information is generated on the fly on trap
> anyway.

Yes. And I'd understand if si_pt_flags was filled by the tracee
during the trap (although I do not think this makes sense) to record
the state at the time of this trap.

But PTRACE_GETSIGINFO returns the dynamic info which reflects the
process-wide state at the time of syscall.


 \ /
  Last update: 2011-05-12 18:51    [W:0.096 / U:1.292 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site