[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