Re: [PATCH 0/3] UTRACE_DETACH fixes

2010-09-02 Thread Roland McGrath
 But probably we need utrace_detached_ops-report_reap() anyway.
 -report_death can return UTRACE_DETACH, and after that utrace_report_death()
 can skip utrace_reset() and do -report_reap().

Yeah, that's a good point.  There's no good reason to do a special case
check for detached_ops there rather than just having the no-op report_reap
hook.

  If an engine used UTRACE_INTERRUPT without having first set QUIESCE,
  then it has no rightful expectation of getting any callback.
 
 Hmm. This is certainly new to me. Could you confirm?

Well, I didn't say it precisely correctly.  UTRACE_INTERRUPT serves two
purposes.  First, it just interrupts things like a signal would.  You could
use that on its own without even tracing any events at all, just do achieve
fault injection or similar.  Second, it's like UTRACE_REPORT.  

If you do have some other event bits set, then you can expect/rely on
getting those normal events soon if they would otherwise have
happened--i.e., if you know it's blocked in a syscall, and you have
UTRACE_EVENT(SYSCALL_EXIT) set, then you can expect to get a
report_syscall_exit soon.  (But, it's always possible to race with the
syscall finishing on its own, or being interrupted by an outside signal, so
that exit might have come before your utrace_control call if you were not
otherwise synchronizing with your established report_syscall_exit callback.)

As with UTRACE_REPORT, after UTRACE_INTERRUPT you can expect to get some
report_quiesce callback even if there is no other event you were tracing
that happens.  For this to actually happen, you need to have
UTRACE_EVENT(QUIESCE) set.  

So, it is technically kosher enough to use UTRACE_INTERRUPT without
UTRACE_EVENT(QUIESCE) set, if you really know the situation and are sure
about all those caveats above.  But, it's only UTRACE_EVENT(QUIESCE) that
gives you any expectation of an extra callback for no event from either
UTRACE_REPORT or UTRACE_INTERRUPT.  (And since that is the sole possible
purpose of UTRACE_REPORT, we diagnose a utrace_control call like that.)


Thanks,
Roland



Re: gdbstub initial code, v7

2010-09-02 Thread Oleg Nesterov
Sorry for the delay, I was distracted. Trying to switch back to ugdb.

On 08/31, Jan Kratochvil wrote:

  ugdb should support qXfer:siginfo, currently accessible only via $_siginfo
  print/set, though.

 Still sure this feature should be also implemented one day.

Yes sure. This should be simple, although I didn't expect qXfer
needs remote_escape_output() and x86_siginfo_fixup(). I assume
that qXfer:siginfo:read always mean Hg thread. It is not clear
to me what should ugdb report if there is no a valid siginfo.
linux_xfer_siginfo() return E01, but gdbserver uses SIGSTOP to
stop the tracee, so it always has something to report. But ugdb
stop the tracee somewhere else, not in get_signal_to_deliver()
path.

Likewise, it is not clear what should ugdb do if gdb sends
$CSIG in this case. But this all is minor, I think.

I was going to send v8 which implements qXfer:siginfo:read and
continue with signal, but (oh, as always) hit the unexpected
problems. To the point, I had to add printk's to utrace.c to
understand what is wrong. Hopefully tomorrow.

Oleg.



Re: [PATCH 0/3] UTRACE_DETACH fixes

2010-09-02 Thread Oleg Nesterov
I was going to ping you ;) This is connected to the problem I hit today.
Despite the fact I already complained about utrace_get_signal()  QUIESCE,
I forgot about this and figured out it doesn't work in practice.

On 09/02, Roland McGrath wrote:

   If an engine used UTRACE_INTERRUPT without having first set QUIESCE,
   then it has no rightful expectation of getting any callback.
 
  Hmm. This is certainly new to me. Could you confirm?

 Well, I didn't say it precisely correctly.  UTRACE_INTERRUPT serves two
 purposes.  First, it just interrupts things like a signal would.  You could
 use that on its own without even tracing any events at all, just do achieve
 fault injection or similar.  Second, it's like UTRACE_REPORT.

Yes, this is clear.

 As with UTRACE_REPORT, after UTRACE_INTERRUPT you can expect to get some
 report_quiesce callback even if there is no other event you were tracing
 that happens.  For this to actually happen, you need to have
 UTRACE_EVENT(QUIESCE) set.

Hmm. I am not sure I understand. If -report_signal != NULL, then
you can't expect -report_quiesce() after utrace_control(INTERRUPT) ?

 So, it is technically kosher enough to use UTRACE_INTERRUPT without
 UTRACE_EVENT(QUIESCE) set, if you really know the situation and are sure
 about all those caveats above.

How? see below.

 But, it's only UTRACE_EVENT(QUIESCE) that
 gives you any expectation of an extra callback for no event from either
 UTRACE_REPORT or UTRACE_INTERRUPT.

Yes, this is clear too.


So. Now I am even more confused. First of all, ugdb (at least currently)
does not need report_quiesce() and UTRACE_EVENT(QUIESCE) at all. OK, this
is not 100% true due to multitracing/etc, lets ignore this. Anyway it
makes sense to clear QUIESCE after $c to avoid the unnecessary callbacks.

All it needs is -report_signal(). But, the problem is, it is not called
after utrace_control(INTERRUPT) ! You need QUIESCE to ensure report_signal()
will be called, and this looks at least strange to me.

Once again, I am not asking to change utrace now, but could you explain
what is wrong with the patch below?

Oleg.

--- x/kernel/utrace.c
+++ x/kernel/utrace.c
@@ -2029,7 +2029,8 @@ int utrace_get_signal(struct task_struct
report.spurious = false;
finish_resume_report(task, utrace, report);
return -1;
-   } else if (!(task-utrace_flags  UTRACE_EVENT(QUIESCE))) {
+   } else if (!(task-utrace_flags 
+   (UTRACE_EVENT(QUIESCE) | 
UTRACE_EVENT_SIGNAL_ALL))) {
/*
 * We only got here to clear utrace-signal_handler.
 */