On 08/26, Oleg Nesterov wrote:
>
> action => NULL may cause the unnecessary utrace_wakeup(current), but
> this case is unlikely and harmless.
Harmless, except BUG_ON(wake != (task != current)).
Updated patch below. I was going to remove this BUG_ON() anyway (see
other patches I am going to send), but didn't realize it should be
killed now.
Oleg.
-------------------------------------------------------------------------------
[PATCH v2] utrace: fix utrace_stop()->utrace_reset() path
utrace_reset(action) change *action only when there are no attached
engines, utrace_stop() can't rely on "stop != UTRACE_STOP" check.
Change utrace_stop() to call utrace_reset(action => NULL) and recheck
->utrace_flags.
action => NULL may cause the unnecessary utrace_wakeup(current), but
this case is unlikely and harmless.
Also, move "unlikely()" from the first check to the second, afaics this
matches the reality.
Well, as I said I believe the comment is misleading, but I can't suggest
something better.
Signed-off-by: Oleg Nesterov <o...@redhat.com>
--- __UTRACE/kernel/utrace.c~1_STOP_DETACH_FIX 2009-08-26 11:42:17.000000000
+0200
+++ __UTRACE/kernel/utrace.c 2009-08-26 14:12:29.000000000 +0200
@@ -731,7 +731,6 @@ static void utrace_reset(struct task_str
unsigned long flags = 0;
LIST_HEAD(detached);
bool wake = !action;
- BUG_ON(wake != (task != current));
splice_attaching(utrace);
@@ -811,7 +810,7 @@ static void utrace_stop(struct task_stru
relock:
spin_lock(&utrace->lock);
- if (unlikely(!(task->utrace_flags & ENGINE_STOP))) {
+ if (!(task->utrace_flags & ENGINE_STOP)) {
/*
* UTRACE_DETACH was used in some utrace_control()
* call since the last time we ran utrace_reset().
@@ -820,9 +819,8 @@ relock:
* would be nobody to resume us. So we must check over
* still-attached engines and recompute the flags.
*/
- enum utrace_resume_action stop = UTRACE_STOP;
- utrace_reset(task, utrace, &stop);
- if (stop != UTRACE_STOP)
+ utrace_reset(task, utrace, NULL);
+ if (unlikely(!(task->utrace_flags & ENGINE_STOP)))
return;
goto relock;
}
