Messages in this thread | | | Date | Thu, 12 May 2011 18:47:45 +0200 | From | Oleg Nesterov <> | Subject | Re: [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 > -EINVAL if PT_SEIZED.
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.
Oleg.
| |