Re: [PATCH 0/1] Was: utrace-cleanup branch

2009-11-13 Thread Oleg Nesterov
On 11/12, Oleg Nesterov wrote: On 11/12, Roland McGrath wrote: I did some tweaks that I think address the several things you've raised. But I didn't try to reply point by point. I've merged everything up now, so the utrace-cleanup branch is gone. Please review the current code

Re: [PATCH 0/1] Was: utrace-cleanup branch

2009-11-12 Thread Roland McGrath
I did some tweaks that I think address the several things you've raised. But I didn't try to reply point by point. I've merged everything up now, so the utrace-cleanup branch is gone. Please review the current code and post about anything we still need to fix. I merged into utrace-ptrace

Re: [PATCH 0/1] Was: utrace-cleanup branch

2009-11-12 Thread Oleg Nesterov
On 11/12, Roland McGrath wrote: I did some tweaks that I think address the several things you've raised. But I didn't try to reply point by point. I've merged everything up now, so the utrace-cleanup branch is gone. Please review the current code and post about anything we still need to fix

Re: [PATCH 0/1] Was: utrace-cleanup branch

2009-11-04 Thread Roland McGrath
3. Now that we have utrace-resume, can't we kill report-resume_action ? I thought this initially when making the change and then decided against it. I don't recall exactly what was in my mind at the time. It would take some more thought now to be sure whether there is a semantic problem. But

Re: [PATCH 0/1] Was: utrace-cleanup branch

2009-11-03 Thread Roland McGrath
8. Completely off-topic, but utrace_control() has a very strange comment under case UTRACE_INTERRUPT, * When it's stopped, we know it's always going * through utrace_get_signal() and will recalculate. can't recall if it were ever true, but surely it is not now? I

Re: [PATCH 0/1] Was: utrace-cleanup branch

2009-11-03 Thread Roland McGrath
4. One of the changes in utrace_get_signal() doesn't look exactly right. if (utrace-resume UTRACE_RESUME || utrace-signal_handler) { ... if (resume UTRACE_REPORT) { report.action = resume;

Re: [PATCH 0/1] Was: utrace-cleanup branch

2009-11-03 Thread Roland McGrath
7. utrace_attach_task() has an implicit wmb() between -utrace = new_utrace and -utrace_flags = REAP, this is good. But, for example, tracehook_force_sigpending() does not have rmb(), this means utrace_interrupt_pending() can OOPS in theory. Ok. Please send a patch. Off hand it

Re: [PATCH 0/1] Was: utrace-cleanup branch

2009-11-03 Thread Roland McGrath
2. Cosmetic, but the usage of utrace_task_alloc() looks a bit strange. Why it returns bool, not struct utrace * ? The pointer it would return is always target-utrace or it's NULL. So the bool just says which of those it would be instead. Either way I imagine it should be inlined so the

[PATCH 0/1] Was: utrace-cleanup branch

2009-11-02 Thread Oleg Nesterov
On 10/28, Roland McGrath wrote: I've made a new branch, utrace-cleanup. This forks from utrace-indirect and has: 26fefca utrace: sticky resume action 28b2774 utrace: remove -stopped field I am not sure I understand the new code in details - too much changes. Anyway, I can never understand

Re: utrace-cleanup branch

2009-10-29 Thread Oleg Nesterov
Btw, why does utrace_set_events() check utrace-stopped? If a tracee is stopped then -reporting == engine is not possible, -reporting must be NULL. To optimize out other checks + mb() in the likely stopped case? Oleg.

Re: utrace-cleanup branch

2009-10-29 Thread Roland McGrath
To optimize out other checks + mb() in the likely stopped case? Yes. Thanks, Roland

Re: utrace-cleanup branch

2009-10-29 Thread Roland McGrath
Can't comment right now, need to read the code. Such gentlemanly restraint. Afaics, we can't just remove utrace_finish_jctl() and the similar code in utrace_stop(). We need void utrace_finish_jctl(void) { struct utrace *utrace = task_utrace_struct(current);

Re: utrace-cleanup branch

2009-10-29 Thread Oleg Nesterov
On 10/29, Roland McGrath wrote: void utrace_finish_jctl(void) { struct utrace *utrace = task_utrace_struct(current); /* * While in TASK_STOPPED, we can be considered safely stopped by * utrace_do_stop(). Make sure we can do

utrace-cleanup branch

2009-10-28 Thread Roland McGrath
I've made a new branch, utrace-cleanup. This forks from utrace-indirect and has: 26fefca utrace: sticky resume action 28b2774 utrace: remove -stopped field Those are the two changes we talked about during tangential ptrace discussions. Again, I have compiled these but not tested a lick. I'd