commit 5792fd628617da9b6486428d3c6aedc3036cd1e0
author Minchan Kim <minchan@google.com> Thu Jan 17 11:14:14 2019 +0900
committer Minchan Kim <minchan@google.com> Wed Jan 30 07:50:14 2019 +0900
tree 7f22209b3bb2bdc298caee95c262bc1e549c0d2e
parent 1fd6d74217b6db1d52d3cb08a5408ff692f94ab0

mm: synchronize period update interval

Wei pointed out period update is racy so it could make partial
update, which could lose a ton of trace potentially.

To close period_ms race between updating and reading, use rwlock
to reduce contention.
To close vmstat_period_ms between updating and reading,
use vmstat_lock.

This patch has small refactoring, too.

Bug: 80168800
Change-Id: I7f84cff758b533b7881f47889c7662b743bc3c12
Signed-off-by: Minchan Kim <minchan@google.com>
(cherry picked from commit 42bbc3be3f2b7663385c141e1fcf4d3b9e70ebe7)

mm/mm_event.c

diff --git a/mm/mm_event.c b/mm/mm_event.c
index bd6c48b..c59824c 100644
--- a/mm/mm_event.c
+++ b/mm/mm_event.c
@@ -8,11 +8,13 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/mm_event.h>
 /* msec */
-static unsigned long period_ms = 500;
-static unsigned long vmstat_period_ms = 1000;
-static DEFINE_SPINLOCK(vmstat_lock);
+static unsigned long period_ms __read_mostly = 500;
+static unsigned long vmstat_period_ms __read_mostly = 1000;
 static unsigned long vmstat_next_period;
 
+static DEFINE_SPINLOCK(vmstat_lock);
+static DEFINE_RWLOCK(period_lock);
+
 void mm_event_task_init(struct task_struct *tsk)
 {
 	memset(tsk->mm_event, 0, sizeof(tsk->mm_event));
@@ -24,12 +26,12 @@
 	int cpu;
 	struct mm_event_vmstat vmstat;
 
-	if (!time_is_before_eq_jiffies(vmstat_next_period))
+	if (time_is_after_jiffies(vmstat_next_period))
 		return;
 
 	/* Need double check under the lock */
 	spin_lock(&vmstat_lock);
-	if (!time_is_before_eq_jiffies(vmstat_next_period)) {
+	if (time_is_after_jiffies(vmstat_next_period)) {
 		spin_unlock(&vmstat_lock);
 		return;
 	}
@@ -101,23 +103,28 @@
 
 static void record_stat(void)
 {
-	if (time_is_before_eq_jiffies(current->next_period)) {
-		int i;
-		bool need_vmstat = false;
+	int i;
+	bool need_vmstat = false;
 
-		for (i = 0; i < MM_TYPE_NUM; i++) {
-			if (current->mm_event[i].count == 0)
-				continue;
-			if (i == MM_COMPACTION || i == MM_RECLAIM)
-				need_vmstat = true;
-			trace_mm_event_record(i, &current->mm_event[i]);
-			memset(&current->mm_event[i], 0,
-					sizeof(struct mm_event_task));
-		}
-		current->next_period = jiffies + msecs_to_jiffies(period_ms);
-		if (need_vmstat)
-			record_vmstat();
+	if (time_is_after_jiffies(current->next_period))
+		return;
+
+	read_lock(&period_lock);
+	current->next_period = jiffies + msecs_to_jiffies(period_ms);
+	read_unlock(&period_lock);
+
+	for (i = 0; i < MM_TYPE_NUM; i++) {
+		if (current->mm_event[i].count == 0)
+			continue;
+		if (i == MM_COMPACTION || i == MM_RECLAIM)
+			need_vmstat = true;
+		trace_mm_event_record(i, &current->mm_event[i]);
+		memset(&current->mm_event[i], 0,
+				sizeof(struct mm_event_task));
 	}
+
+	if (need_vmstat)
+		record_vmstat();
 }
 
 void mm_event_start(ktime_t *time)
@@ -149,13 +156,18 @@
 	if (val < 1 || val > ULONG_MAX)
 		return -EINVAL;
 
+	write_lock(&period_lock);
 	period_ms = (unsigned long)val;
+	write_unlock(&period_lock);
 	return 0;
 }
 
 static int period_ms_get(void *data, u64 *val)
 {
+	read_lock(&period_lock);
 	*val = period_ms;
+	read_unlock(&period_lock);
+
 	return 0;
 }
 
@@ -164,13 +176,17 @@
 	if (val < 1 || val > ULONG_MAX)
 		return -EINVAL;
 
+	spin_lock(&vmstat_lock);
 	vmstat_period_ms = (unsigned long)val;
+	spin_unlock(&vmstat_lock);
 	return 0;
 }
 
 static int vmstat_period_ms_get(void *data, u64 *val)
 {
+	spin_lock(&vmstat_lock);
 	*val = vmstat_period_ms;
+	spin_unlock(&vmstat_lock);
 	return 0;
 }