Re: utrace unwanted traps

[Oleg Nesterov]

On 10/14, Roland McGrath wrote:

  In short: I no longer understand why utrace-resume can be UTRACE_*STEP
  when the tracee is stopped, and in fact I think this is not right.

 I think we didn't have that originally.

Yes.

 The rationale that I remember is
 the idea that you should be able to implement a debugger interface feature
 like PTRACE_SINGLESTEP without noticeably more overhead than it had in
 pre-utrace ptrace.  That is, in vanilla ptrace, the tracer thread calls
 user_enable_single_step itself and then just lets the tracee resume
 directly to user mode.  We wanted to avoid always having an additional trip
 through tracehook_notify_resume and the utrace reporting loop and then back
 through the return-to-user assembly path a second time, between the tracee
 waking up and actually going to user mode.

Probably, I am not sure. I can't recall the previous discussions. IIRC,
the main problem with user_enable_single_step() was: we wanted to move
the callsite from tracer to tracee by many reasons. I can't recall if
avoid another reporting loop was the target, and additional trip
through tracehook_notify_resume() is not avoidable anyway.

Afaics, this was introduced by 26fefca955cc7c3c83014be2e10b773209d32eea
utrace: sticky resume action. Before that patch the stopped tracee
could only remember the pending UTRACE_REPORT or UTRACE_INTERRUPT.

 Right.  Said another way, any time an engine that previously used
 UTRACE_*STEP now uses UTRACE_RESUME, we must degrade utrace-resume back to
 UTRACE_REPORT at most.  Since we don't keep track of which engine it was
 that put UTRACE_*STEP into utrace-resume before, we'd have to just always
 degrade it any time anybody uses UTRACE_RESUME.

Agreed.

 That has almost the whole effect of punting utrace-resume back to just a
 utrace-report flag.

(see below)

 But that's how it used to be, and then we changed it.
 So it merits digging up our old discussion threads that led to doing that
 and see what all we had in mind then.

I tried to grep my mail archives and google, but failed to find anything
related.

_Perhaps_ this was not intended actually. Before the commit above, another
problem was fixed by 8ad60bbd4c665a11be179a0bff41483cca3b3560
utrace_stop: preserve report/interrupt requests across stop/resume, until
this one it was possible to lose UTRACE_INTERRUPT request if it races
with UTRACE_STOP from another engine. So, perhaps cleanup was the main
reason for 26fefca9.

See also http://www.mail-archive.com/utrace-devel@redhat.com/msg01647.html

Oleg.

Re: utrace unwanted traps

[Roland McGrath]

 Probably, I am not sure. I can't recall the previous discussions. 

Me either (except hazily the one notion I mentioned), but that's why
we have mailing list archives.

 Afaics, this was introduced by 26fefca955cc7c3c83014be2e10b773209d32eea
 utrace: sticky resume action. Before that patch the stopped tracee
 could only remember the pending UTRACE_REPORT or UTRACE_INTERRUPT.

Correct.

  But that's how it used to be, and then we changed it.
  So it merits digging up our old discussion threads that led to doing that
  and see what all we had in mind then.
 
 I tried to grep my mail archives and google, but failed to find anything
 related.

Based on the date of the commit you just mentioned, I browsed in:
https://www.redhat.com/archives/utrace-devel/2009-October/thread.html
and saw:
https://www.redhat.com/archives/utrace-devel/2009-October/msg00287.html
which is the day before that commit.  I didn't read through all that to
refamiliarize myself with all the details.


Thanks,
Roland

Re: [PATCH] utrace_barrier(detached_engine) must not sping without checking -reporting

[Roland McGrath]

 Yes. But, with this patch, in this case
 utrace_barrier()-get_utrace_lock(attached = false) returns success
 and then we check utrace-reporting == engine.

I guess that's OK if -reporting == engine is always set when any kind of
callback might be in progress.

 (Hmm. Probably utrace-reap = T case needs more attention,
  utrace_maybe_reap() doesn't set utrace-reporting).

That would be a problem.

 But, unfortunately, this signal_pending() check assumes the caller can
 always handle this ERESTARTSYS. But this is not true, one example is
 the exiting debugger doing exit_ptrace().

I understand it's a problem.  Perhaps we need to have a flag argument or
(probably better) a new utrace_barrier_uninterruptible call.  But, for
this particular situation with exit_ptrace, it could be kludged around.
The caller is never going to dequeue another signal once it's as far
into death as exit_ptrace anyway.  So, it could do a loop of:

ret = utrace_barrier(task, engine);
if (ret == -ERESTARTSYS) {
clear_thread_flag(TIF_SIGPENDING);
continue;
}

Not that I'm suggesting this isn't a dismal kludge.  But it seems like
it would close the hole we have today in practice.


Thanks,
Roland