[PATCH 4/4] utrace_release_task: check REAP before utrace_reap()

[Roland McGrath]

> Again, I am not sure this make sense as a cleanup, up to you.
> But utrace_release_task() can check UTRACE_EVENT(REAP) and
> optimize out utrace_reap() when there are no attached engines.

In the original code, utrace_release_task() would not be called at all when
there are no engines attached.  IMHO that is the optimization we want to
get back to.

It may still be too early in the morning for me to be thinking clearly.
But what about this?


Thanks,
Roland


diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index e249770..0000000 100644  
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -362,7 +362,12 @@ static inline void tracehook_report_vfor
  */
 static inline void tracehook_prepare_release_task(struct task_struct *task)
 {
-	utrace_release_task(task);
+	/*
+	 * See kernel/utrace.c:utrace_attach_task() about this barrier.
+	 */
+	smp_mb();
+	if (task_utrace_flags(task))
+		utrace_release_task(task);
 }
 
 /**
diff --git a/kernel/utrace.c b/kernel/utrace.c
index 76d0d84..0000000 100644  
--- a/kernel/utrace.c
+++ b/kernel/utrace.c
@@ -143,13 +143,13 @@ static int utrace_add_engine(struct task
 
 	spin_lock(&utrace->lock);
 
-	if (utrace->reap) {
+	if (unlikely(utrace->reap)) {
 		/*
 		 * Already entered utrace_release_task(), cannot attach now.
 		 */
 		ret = -ESRCH;
 	} else if ((flags & UTRACE_ATTACH_EXCLUSIVE) &&
-	    unlikely(matching_engine(utrace, flags, ops, data))) {
+		   unlikely(matching_engine(utrace, flags, ops, data))) {
 		ret = -EEXIST;
 	} else {
 		/*
@@ -216,16 +216,19 @@ struct utrace_engine *utrace_attach_task
 
 	utrace = &target->utrace;
 
-	if (unlikely(target->exit_state == EXIT_DEAD)) {
-		/*
-		 * The target has already been reaped.
-		 * Check this early, though it's not synchronized.
-		 * utrace_add_engine() will do the final check.
-		 */
-		if (!(flags & UTRACE_ATTACH_CREATE))
-			return ERR_PTR(-ENOENT);
-		return ERR_PTR(-ESRCH);
-	}
+	/*
+	 * If it already could be getting reaped, we cannot attach now.
+	 * The barrier ensures this check precedes utrace_add_engine()
+	 * setting ->utrace_flags.  tracehook_prepare_release_task() has
+	 * the corresponding barrier, after setting EXIT_DEAD and before
+	 * checking ->utrace_flags to decide to call utrace_release_task().
+	 */
+	ret = target->exit_state;
+	smp_mb();
+
+	if (unlikely(ret == EXIT_DEAD))
+		return ERR_PTR((flags & UTRACE_ATTACH_CREATE) ?
+			       -ESRCH : -ENOENT);
 
 	if (!(flags & UTRACE_ATTACH_CREATE)) {
 		spin_lock(&utrace->lock);