Blob Blame History Raw
From: Mel Gorman <mgorman@suse.de>
Date: Thu, 9 Apr 2020 19:23:38 +0100
Subject: [PATCH] cgroup, rstat: Revert "cgroup: Add memory barriers to plug
 cgroup_rstat_updated() race window"

Patch-mainline: Submitted, lore.kernel.org/r/20200409175621.GA37608@mtj.thefacebook.com
References: bsc#1158748

This reverts commit 9a9e97b2f1f2 ("cgroup: Add memory barriers to plug
cgroup_rstat_updated()"). This was discussed upstream and the maintainer
will be doing his own revert. The barriers closed a data race that would
leave cpu statistics out of date for a very short interval. The barriers
closed the race because "we plan to use rstat to track counters which
need to be accurate".

The code that would require it never got merged and the barriers are
unnecessary. While an upstream patch does not exist right now, the
maintainer states

	For now, I'll revert the patch. Nothing in tree needs that right
	now. If the need for synchronous counting comes back later,
	I'll make that a separate path.

Signed-off-by: Mel Gorman <mgorman@suse.de>
---
 kernel/cgroup/rstat.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c
index ca19b4c8acf5..9be2f30d646d 100644
--- a/kernel/cgroup/rstat.c
+++ b/kernel/cgroup/rstat.c
@@ -33,12 +33,9 @@ void cgroup_rstat_updated(struct cgroup *cgrp, int cpu)
 		return;
 
 	/*
-	 * Paired with the one in cgroup_rstat_cpu_pop_upated().  Either we
-	 * see NULL updated_next or they see our updated stat.
-	 */
-	smp_mb();
-
-	/*
+	 * Speculative already-on-list test.  This may race leading to
+	 * temporary inaccuracies, which is fine.
+	 *
 	 * Because @parent's updated_children is terminated with @parent
 	 * instead of NULL, we can tell whether @cgrp is on the list by
 	 * testing the next pointer for NULL.
@@ -134,13 +131,6 @@ static struct cgroup *cgroup_rstat_cpu_pop_updated(struct cgroup *pos,
 		*nextp = rstatc->updated_next;
 		rstatc->updated_next = NULL;
 
-		/*
-		 * Paired with the one in cgroup_rstat_cpu_updated().
-		 * Either they see NULL updated_next or we see their
-		 * updated stat.
-		 */
-		smp_mb();
-
 		return pos;
 	}