> Ah, no. I never added mine sob's in these long series, because I thought > it doesn't matter.
It doesn't. I just need to stay clear on what tree state each day's patches are relative to, and what is "the current code" when we're discussing the details. > Yes, as usual the changes were chaotic. I tend to send the changes even if > I know they should be redone/cleanuped later, to get your early review and > to discuss asap. It's always good to post early and often. Last week I had to be out of the office a fair bit and so it wound up taking me until the weekend to review them all anyway. > But I think it would be much better to move this re-initialization into > ptrace_attach_task(), in this case we can't do memset(0) + restore siginfo. Ok. > I thought about this too. Currently I don't see how to do this cleanly. > This all needs some obvious cleanups, then we will see if we can cleanup > the code more and do some consolidation. Ok. > No, the return calue from utrace_barrier() does not matter. And, if the > tracee is killed we don't care. The race is different. The utrace_barrier return value should cover exactly the case you describe, and that is the clean way to do it. It will fail if the tracee is either dead or detached. Indeed, you don't really care either way when it's dead. A success return tells you positively that it was not detached. (If it's interrupted you want to just back off and return -ERESTARTNOINTR anyway.) > Agreed, this all is very subjective. I hate the idea to attach another > engine here, but can't prove it is really bad. Firstly, I dislike it > because it is very unlikely another tracer will attach before the tracee > handles this signal, a bit ugly we should arrange the new engine "just in > case". And in theory kmem_cache_alloc() can fail, not good. I agree with those concerns. I'm just also concerned with how hairy the code winds up. > But mostly I dislike the fact we must teach old and new engines to cooperate. We have some version of that regardless. > And. probably I missed something, but it is not trivial to implement. > A lot of problems to solve. Just for example, when we attach the new > engine, its ->flag has no UTRACE_EVENT(SIGNAL) yet. If we steal ->siginfo > we can "lose" this signal. If we set UTRACE_EVENT(SIGNAL) first, the new > and old engine can race. This should be taken care of when you detach the old engine. If utrace_control(task, old, UTRACE_DETACH) returns zero, then the old engine's final callback has not run yet. (If it had run, it would have detached already.) If it fails, then the old engine's callback has run or is starting to run, so that callback cycle will finish before the new engine's first callback could run. Hmm. I guess there is a race or three to contend with anyway. > Yes I thought about this, but afaics we have to fight with multiple annoying > races. Again, again, I do not pretend I am sure this is really true. I'm not sure either. If we are keeping all options in mind as we look for the cleanest final form of the code, I am satisfied with whichever approach you take. Thanks, Roland
