[PATCH] utrace_barrier(detached_engine) must not sping without checking ->reporting
Oleg Nesterov
oleg at redhat.com
Wed Aug 18 17:27:04 UTC 2010
If engine is detached (has utrace_detached_ops), utrace_barrier(engine)
spins until engine->ops becomes NULL. This is just wrong.
Suppose that utrace_control(DETACH) returns -EINPROGRESS, now we should
call utrace_barrier(). However, it is possible that -EINPROGRESS means
we raced with sys_sleep(A_LOT) doing report_syscall_entry(). Or it could
race with any callback, but another engine requested UTRACE_STOP.
If start_callback() notices utrace_detached_ops and sets report->detaches
everything is fine. But it is quite possible that this doesn't happen,
and in this case utrace_barrier() will spin "forever" waiting for the
next utrace_reset().
Change get_utrace_lock() to succeed if the caller is utrace_barrier()
and ops == &utrace_detached_ops. I do not see any reason why this case
should be special from utrace_barrier's pov. It can just check
->reporting and return 0 or do another iteration.
Note: we should also reconsider() utrace_barrier()->signal_pending()
check. Also, it is not clear why utrace_barrier() needs utrace->lock,
except to ensure it is safe to dereference target/utrace.
Signed-off-by: Oleg Nesterov <oleg at redhat.com>
---
kernel/utrace.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
--- kstub/kernel/utrace.c~9_utrace_barrier_and_detached 2010-08-18 17:47:53.000000000 +0200
+++ kstub/kernel/utrace.c 2010-08-18 19:00:50.000000000 +0200
@@ -416,23 +416,21 @@ static struct utrace *get_utrace_lock(st
return ERR_PTR(-ESRCH);
}
- if (unlikely(engine->ops == &utrace_detached_ops)) {
+ if (!attached && unlikely(engine->ops == &utrace_detached_ops)) {
rcu_read_unlock();
- return attached ? ERR_PTR(-ESRCH) : ERR_PTR(-ERESTARTSYS);
+ return ERR_PTR(-ESRCH);
}
utrace = task_utrace_struct(target);
spin_lock(&utrace->lock);
if (unlikely(utrace->reap) || unlikely(!engine->ops) ||
- unlikely(engine->ops == &utrace_detached_ops)) {
+ (!attached && unlikely(engine->ops == &utrace_detached_ops))) {
/*
* By the time we got the utrace lock,
* it had been reaped or detached already.
*/
spin_unlock(&utrace->lock);
utrace = ERR_PTR(-ESRCH);
- if (!attached && engine->ops == &utrace_detached_ops)
- utrace = ERR_PTR(-ERESTARTSYS);
}
rcu_read_unlock();
@@ -1286,8 +1284,7 @@ int utrace_barrier(struct task_struct *t
utrace = get_utrace_lock(target, engine, false);
if (unlikely(IS_ERR(utrace))) {
ret = PTR_ERR(utrace);
- if (ret != -ERESTARTSYS)
- break;
+ break;
} else {
/*
* All engine state changes are done while
More information about the utrace-devel
mailing list