Re: [PATCH 2/2] UTRACE_STOP: nesting engine management (updated)
On 03/12, Renzo Davoli wrote:
>
> I have update also the second patch. Please note that now this patch
> must be applied after the first one.
> This patch implements a consistent nesting model for utrace machines.
> (There is a full description in the messages I sent on Feb. 14 and Mar. 6)
This patch does 2 completely different things. I think you should
make separate patches.
Again, we need Roland's opinion, but could you explain why it would
be better to use _reverse in utrace_report_syscall_entry() ?
As for another change,
> --- linux-2.6.29-rc7-git5-utrace-p1/kernel/utrace.c 2009-03-12
> 11:05:50.0 +0100
> +++ linux-2.6.29-rc7-git5-utrace-p2/kernel/utrace.c 2009-03-12
> 13:37:27.0 +0100
> @@ -1405,6 +1405,7 @@
> static bool finish_callback(struct utrace *utrace,
> struct utrace_report *report,
> struct utrace_engine *engine,
> + struct task_struct *task,
> u32 ret)
> {
> enum utrace_resume_action action = utrace_resume_action(ret);
> @@ -1426,6 +1427,7 @@
> spin_lock(&utrace->lock);
> mark_engine_wants_stop(engine);
> spin_unlock(&utrace->lock);
> + utrace_stop(task, utrace);
I don't think this is safe. If we do utrace_stop() here, the next engine
can be detached before we return (UTRACE_DETACH assumes it it safe to
unlink the engine when the target is stopped). This means we can't
continue list_for_each_entry(engine, &utrace->attached, entry) after
return from finish_callback().
Oleg.
Re: [PATCH 2/2] UTRACE_STOP: nesting engine management (updated)
> Again, we need Roland's opinion, but could you explain why it would > be better to use _reverse in utrace_report_syscall_entry() ? I refer to this posting: http://www.mail-archive.com/utrace-devel@redhat.com/msg00579.html Item #4 explains why it is *needed* to reverse the order in utrace_report_syscall_entry to have a consistent implementation of nested virtualization. > I don't think this is safe. If we do utrace_stop() here, the next engine > can be detached before we return (UTRACE_DETACH assumes it it safe to > unlink the engine when the target is stopped). This means we can't > continue list_for_each_entry(engine, &utrace->attached, entry) after > return from finish_callback(). Maybe this is not the best patch, maybe we can solve the problem in a better way. The point is explained in #3 in the same posting cited above. When a report function of an engine returns UTRACE_STOP, it means (may mean) that it wants to change the status of the process before resuming it. VM monitors often change the status, sometimes debugger users want to set some variables too. IMHO, utrace should stop it *before* calling the report function of the next engine, otherwise we need to set up another structure to synchronize the engines (that may even be unknown one to the other). If there is a tracer/debugger among the engines, it is not even possible to know which snapshot it gets, after or before the modification created by the VM monitor? With these patches it is possible to run nested virtual machines based on utrace, it is also possbile to strace (use ptrace) on processes running inside a VM. renzo
Re: [PATCH 2/2] UTRACE_STOP: nesting engine management (updated)
> When a report function of an engine returns UTRACE_STOP, it means (may mean) > that it wants to change the status of the process before resuming it. > VM monitors often change the status, sometimes debugger users want to set > some variables too. Yes. In ideal cases, it can decide up front quickly what it wants to do, and change the user state right in the callback without stopping. But when it needs another agent to decide what to do, it uses UTRACE_STOP. > IMHO, utrace should stop it *before* calling the report function of the > next engine, No, we'll never want to do it this way. One engine doesn't get to arbitrarily delay the reporting to other engines of the thread's events. This is both an efficiency point and a robustness point. It's important to remember that utrace is about the primitive events: the user thread had an event ... the user thread is about to run again. The high-level notion of "what did the other engine do?" is built from examining the state at these events, and knowing about the delays that other engines are imposing via UTRACE_STOP. > otherwise we need to set up another structure to synchronize > the engines (that may even be unknown one to the other). > If there is a tracer/debugger among the engines, it is not even possible to > know > which snapshot it gets, after or before the modification created by the VM > monitor? This is where the broader discussion of callback order comes in. When a previous engine has decided to use UTRACE_STOP, your callback's @action argument reflects this. You know that another engine is going to do something asynchronous before it lets the user thread run. If your own engine doesn't especially want it stopped now but wants to see what it looks like when other engines are done fiddling with it, then you can use UTRACE_REPORT. That ensures that you'll get a report_quiesce callback after those other engines have done their thing. Thanks, Roland
