On 06/22, Roland McGrath wrote: > > > OTOH, I am wondering if we can give more power to utrace_control(STOP). > > Currently debugger can't stop the tracee unless it cooperates. However, if > > the tracee doesn't have UTRACE_EVENT(QUIESCE), then utrace_control(STOP) > > works. This is strange. > > Why is it strange? If you don't have any callback, then all your engine > does is get stopped. If you have callbacks, then you always get callbacks > before the thread does anything (including stop). Since your callback's > return value always replaces any other resume action
OK. But then perhaps UTRACE_STICKY_STOP makes sense even without the races we discussed. It can simplify the cooperation in this case. > > Stupid question: what if we just remove the clear_engine_wants_stop() > > code in finish_callback_report() ? > > That would change the API so that a callback's resume action is always > overridden by a utrace_control(,,UTRACE_STOP). Today the practice from a > third-party debugger thread is to do: > > ret = utrace_control(task, engine, UTRACE_STOP); > if (ret == 0) { > // Already stopped, e.g. was in jctl or another engine's stop. > // This means we can do our asynchronous examination here. > } else if (ret == -EINPROGRESS) { > // it will get to report_quiesce soonish (modulo blocked in kernel) > } > > If the examination you wanted to do was some predetermined sequence of > fetching and fiddling before resuming, then the report_* callback can just > do all that itself. For example, fetch the registers for a sample and keep > going; insert some breakpoints and keep going; etc. OK, agreed. > With this > change, it would have to explicitly request not stopping by calling > utrace_control(current, engine, UTRACE_RESUME) inside the callback. Ah, indeed, this can't be good. Thanks. > > Btw, before I forgot about this. shouldn't utrace_control(UTRACE_RESUME) > > remove ENGINE_STOP from ->utrace_flags like DETACH does? > > Perhaps so. My memory is dim on the use of that bit in utrace_flags. (don't assume I was able to quickly recall why DETACH does this ;) > It's > to close certain races, but we'll have to remind each other of the details. Yes. Let's consider the concrete example. The tracee is going to stop and calls utrace_stop(). Before it takes utrace->lock and sets state = TASK_TRACED, the debugger does utrace_control(DETACH). In this case utrace_stop() shouldn't stop, otherwise nobody will ever wake it up. That is why we clear this bit in ->utrace_flags to provoke utrace_reset() which will check carefully the tracee has an engine with ENGINE_STOP. The original motivation for this patch was ptrace-utrace, the implicit DETACH if the tracer exits. UTRACE_RESUME should probably do the same, otherwise the tracee can stop right after utrace_control(RESUME). This case is less important compared to UTRACE_DETACH (and just in case, ptrace doesn't need this) but still. > > Hmm. Not sure... at least I don't immediately see something simple. > > > > Except, just add wait_queue_head into struct utrace. This way > > utrace_barrier() > > can wait longer than needed if it notices utrace->reporting == engine. Or we > > can add it into utrace_engine, in this case utrace_stop() needs > > list_for_each(engine, utrace->attached). > > You're talking about two different things here. Whatever implementation > details change in utrace_barrier() is not directly apropos. Yes, agreed. > For the new API idea, I was talking about something like a utrace_wake_up() > call. Now I don't understand you again... The 1st question: should the new API help us to kill ptrace_notify_stop() in utrace_stop() ? > What I'm most interested in right now is thinking through the utility of > particular new API details for engine writers. Hmm... After the previous email I thought that this utrace_wake_up() or whatever shouldn't be visible outside of utrace.c. Ignoring the ptrace_notify_stop() problem, could you give us any example of how it can be used? Or. Do you literally mean something like utrace_wait_for_event(); // for debugger utrace_wake_up(); // for tracee which should (at least) cover all races with exit/detach/etc ? Oleg.