Re: [PATCH -mm] introduce tracehook_finish_jctl() helper
Confused. This patch has no effect. Indeed, the three patches changing do_signal_stop() have no immediate material effect. They all just clean up the code and prepare the hooks that utrace (or something like it) needs to make tracing stops mesh well with job control stops. I'd thought it might be simpler for review to just roll these three together and send you a single patch to replace signals-tracehook_notify_jctl-change.patch; but Oleg decided to keep it sliced smaller. Thanks, Roland
Re: [PATCH 5/6] finish_utrace_stop: check -stopped lockless
finish_utrace_stop() can check -stopped lockless. It was set by us, we can't miss it. We enter utrace_stop() for some stop. Either then or later, a group jctl stop finishes and sets SIGNAL_STOP_STOPPED. Later, utrace_wakeup() sees that and we switch to TASK_STOPPED after clearing -stopped. We stay in jctl stop for a few days. Some new debugger comes along with utrace_control(,,UTRACE_STOP) and utrace_do_stop() switches us to TASK_TRACED after setting -stopped. Meanwhile, SIGCONT is coming along and clearing SIGNAL_STOP_STOPPED (sibling threads run again, etc.). Now the debugger calls utrace_control(,,UTRACE_RESUME), so utrace_wakeup clears -stopped and wakes us up. Now -stopped was just an instant ago set and then cleared on the other CPU and we are running. Are we really sure that we see it as clear rather than set? (I think there is also a racier version without jctl stops at all, just where utrace_wakeup+utrace_do_stop+utrace_wakeup all came and went before we actually got scheduled.) I might be overlooking some interlock that covers this. I put the patch in, anyway. But I think that I can even worry about this is a good indicator that we'd probably be happier if we can indeed get rid of -stopped altogether. Thanks, Roland
Re: [PATCH 6/6] get_utrace_lock: do not check EXIT_DEAD
get_utrace_lock() checks -state != EXIT_DEAD to make sure it safe to use -utrace. This is unneeded since -utrace was embedded into task_struct. If we can read -state, we can read -utrace as well. I see. My immediate reaction to this was that it should have more comments that make clear it's relying on that rule of struct utrace lifetime. That thought was motivated by my interest in changing the data structure (again) to using a struct utrace pointer in the future. But the reason that it's potentially worthwhile to have a pointer that can go away is to reclaim the memory on live detach. Even if we do go that way again, we won't reintroduce the same kind of race with utrace_release_task, because it's far simpler just to delay the deallocation until tracehook_free_task. So whatever magic we would need in get_utrace_lock would be worrying only about interlocks with utrace_reset, not with -exit_state changes. i.e. it is orthogonal to this change. Thanks, Roland
