All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv4] psi: fix possible trigger missing in the window
@ 2022-01-25  6:56 Huangzhaoyang
  2022-01-25 14:40 ` Johannes Weiner
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Huangzhaoyang @ 2022-01-25  6:56 UTC (permalink / raw)
  To: Johannes Weiner, Suren Baghdasaryan, Peter Zijlstra,
	Zhaoyang Huang, Ingo Molnar, linux-mm, linux-kernel

From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>

When a new threshold breaching stall happens after a psi event was
generated and within the window duration, the new event is not
generated because the events are rate-limited to one per window. If
after that no new stall is recorded then the event will not be
generated even after rate-limiting duration has passed. This is
happening because with no new stall, window_update will not be called
even though threshold was previously breached. To fix this, record
threshold breaching occurrence and generate the event once window
duration is passed.

Suggested-by: Suren Baghdasaryan <surenb@google.com>
Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
---
v2: modify the logic according to Suren's suggestion
v3: update commit message
v4: update variable name and comments
---
---
 include/linux/psi_types.h |  3 +++
 kernel/sched/psi.c        | 46 ++++++++++++++++++++++++++++++----------------
 2 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/include/linux/psi_types.h b/include/linux/psi_types.h
index 0a23300..e0ec129 100644
--- a/include/linux/psi_types.h
+++ b/include/linux/psi_types.h
@@ -132,6 +132,9 @@ struct psi_trigger {
 
 	/* Refcounting to prevent premature destruction */
 	struct kref refcount;
+
+	/* Deferred event(s) from previous ratelimit window */
+	bool pending_event;
 };
 
 struct psi_group {
diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c
index 1652f2b..dd80bd2 100644
--- a/kernel/sched/psi.c
+++ b/kernel/sched/psi.c
@@ -515,7 +515,7 @@ static void init_triggers(struct psi_group *group, u64 now)
 static u64 update_triggers(struct psi_group *group, u64 now)
 {
 	struct psi_trigger *t;
-	bool new_stall = false;
+	bool update_total = false;
 	u64 *total = group->total[PSI_POLL];
 
 	/*
@@ -524,24 +524,35 @@ static u64 update_triggers(struct psi_group *group, u64 now)
 	 */
 	list_for_each_entry(t, &group->triggers, node) {
 		u64 growth;
+		bool new_stall;
 
-		/* Check for stall activity */
-		if (group->polling_total[t->state] == total[t->state])
-			continue;
+		new_stall = group->polling_total[t->state] != total[t->state];
 
+		/* Check for stall activity or a previous threshold breach */
+		if (!new_stall && !t->pending_event)
+			continue;
 		/*
-		 * Multiple triggers might be looking at the same state,
-		 * remember to update group->polling_total[] once we've
-		 * been through all of them. Also remember to extend the
-		 * polling time if we see new stall activity.
+		 * Check for new stall activity, as well as deferred
+		 * events that occurred in the last window after the
+		 * trigger had already fired (we want to ratelimit
+		 * events without dropping any).
 		 */
-		new_stall = true;
-
-		/* Calculate growth since last update */
-		growth = window_update(&t->win, now, total[t->state]);
-		if (growth < t->threshold)
-			continue;
-
+		if (new_stall) {
+			/*
+			 * Multiple triggers might be looking at the same state,
+			 * remember to update group->polling_total[] once we've
+			 * been through all of them. Also remember to extend the
+			 * polling time if we see new stall activity.
+			 */
+			update_total = true;
+
+			/* Calculate growth since last update */
+			growth = window_update(&t->win, now, total[t->state]);
+			if (growth < t->threshold)
+				continue;
+
+			t->pending_event = true;
+		}
 		/* Limit event signaling to once per window */
 		if (now < t->last_event_time + t->win.size)
 			continue;
@@ -550,9 +561,11 @@ static u64 update_triggers(struct psi_group *group, u64 now)
 		if (cmpxchg(&t->event, 0, 1) == 0)
 			wake_up_interruptible(&t->event_wait);
 		t->last_event_time = now;
+		/* Reset threshold breach flag once event got generated */
+		t->pending_event = false;
 	}
 
-	if (new_stall)
+	if (update_total)
 		memcpy(group->polling_total, total,
 				sizeof(group->polling_total));
 
@@ -1152,6 +1165,7 @@ struct psi_trigger *psi_trigger_create(struct psi_group *group,
 	t->last_event_time = 0;
 	init_waitqueue_head(&t->event_wait);
 	kref_init(&t->refcount);
+	t->pending_event = false;
 
 	mutex_lock(&group->trigger_lock);
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCHv4] psi: fix possible trigger missing in the window
  2022-01-25  6:56 [PATCHv4] psi: fix possible trigger missing in the window Huangzhaoyang
@ 2022-01-25 14:40 ` Johannes Weiner
  2022-01-26 16:44   ` Suren Baghdasaryan
  2022-02-14 15:05 ` Johannes Weiner
  2022-02-17 18:56 ` [tip: sched/core] " tip-bot2 for Zhaoyang Huang
  2 siblings, 1 reply; 6+ messages in thread
From: Johannes Weiner @ 2022-01-25 14:40 UTC (permalink / raw)
  To: Huangzhaoyang
  Cc: Suren Baghdasaryan, Peter Zijlstra, Zhaoyang Huang, Ingo Molnar,
	linux-mm, linux-kernel

On Tue, Jan 25, 2022 at 02:56:58PM +0800, Huangzhaoyang wrote:
> From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
> 
> When a new threshold breaching stall happens after a psi event was
> generated and within the window duration, the new event is not
> generated because the events are rate-limited to one per window. If
> after that no new stall is recorded then the event will not be
> generated even after rate-limiting duration has passed. This is
> happening because with no new stall, window_update will not be called
> even though threshold was previously breached. To fix this, record
> threshold breaching occurrence and generate the event once window
> duration is passed.
> 
> Suggested-by: Suren Baghdasaryan <surenb@google.com>
> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com>

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv4] psi: fix possible trigger missing in the window
  2022-01-25 14:40 ` Johannes Weiner
@ 2022-01-26 16:44   ` Suren Baghdasaryan
  0 siblings, 0 replies; 6+ messages in thread
From: Suren Baghdasaryan @ 2022-01-26 16:44 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: Huangzhaoyang, Peter Zijlstra, Zhaoyang Huang, Ingo Molnar,
	linux-mm, LKML

On Tue, Jan 25, 2022 at 6:40 AM Johannes Weiner <hannes@cmpxchg.org> wrote:
>
> On Tue, Jan 25, 2022 at 02:56:58PM +0800, Huangzhaoyang wrote:
> > From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
> >
> > When a new threshold breaching stall happens after a psi event was
> > generated and within the window duration, the new event is not
> > generated because the events are rate-limited to one per window. If
> > after that no new stall is recorded then the event will not be
> > generated even after rate-limiting duration has passed. This is
> > happening because with no new stall, window_update will not be called
> > even though threshold was previously breached. To fix this, record
> > threshold breaching occurrence and generate the event once window
> > duration is passed.
> >
> > Suggested-by: Suren Baghdasaryan <surenb@google.com>
> > Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
>
> Acked-by: Johannes Weiner <hannes@cmpxchg.org>

Looks good. Thanks!

Acked-by: Suren Baghdasaryan <surenb@google.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv4] psi: fix possible trigger missing in the window
  2022-01-25  6:56 [PATCHv4] psi: fix possible trigger missing in the window Huangzhaoyang
  2022-01-25 14:40 ` Johannes Weiner
@ 2022-02-14 15:05 ` Johannes Weiner
  2022-02-14 15:09   ` Peter Zijlstra
  2022-02-17 18:56 ` [tip: sched/core] " tip-bot2 for Zhaoyang Huang
  2 siblings, 1 reply; 6+ messages in thread
From: Johannes Weiner @ 2022-02-14 15:05 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Suren Baghdasaryan, Huangzhaoyang, Zhaoyang Huang, Ingo Molnar,
	linux-mm, linux-kernel

On Tue, Jan 25, 2022 at 02:56:58PM +0800, Huangzhaoyang wrote:
> From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
> 
> When a new threshold breaching stall happens after a psi event was
> generated and within the window duration, the new event is not
> generated because the events are rate-limited to one per window. If
> after that no new stall is recorded then the event will not be
> generated even after rate-limiting duration has passed. This is
> happening because with no new stall, window_update will not be called
> even though threshold was previously breached. To fix this, record
> threshold breaching occurrence and generate the event once window
> duration is passed.
> 
> Suggested-by: Suren Baghdasaryan <surenb@google.com>
> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com>

Hey Peter, would you mind taking this through the scheduler tree? It's
got my and Suren's acks. It's not a recent regression so I'm thinking
for 5.18 is fine. Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv4] psi: fix possible trigger missing in the window
  2022-02-14 15:05 ` Johannes Weiner
@ 2022-02-14 15:09   ` Peter Zijlstra
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2022-02-14 15:09 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: Suren Baghdasaryan, Huangzhaoyang, Zhaoyang Huang, Ingo Molnar,
	linux-mm, linux-kernel

On Mon, Feb 14, 2022 at 10:05:37AM -0500, Johannes Weiner wrote:
> On Tue, Jan 25, 2022 at 02:56:58PM +0800, Huangzhaoyang wrote:
> > From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
> > 
> > When a new threshold breaching stall happens after a psi event was
> > generated and within the window duration, the new event is not
> > generated because the events are rate-limited to one per window. If
> > after that no new stall is recorded then the event will not be
> > generated even after rate-limiting duration has passed. This is
> > happening because with no new stall, window_update will not be called
> > even though threshold was previously breached. To fix this, record
> > threshold breaching occurrence and generate the event once window
> > duration is passed.
> > 
> > Suggested-by: Suren Baghdasaryan <surenb@google.com>
> > Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
> 
> Hey Peter, would you mind taking this through the scheduler tree? It's
> got my and Suren's acks. It's not a recent regression so I'm thinking
> for 5.18 is fine. Thanks!

Sorry, missed it, got it now. Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [tip: sched/core] psi: fix possible trigger missing in the window
  2022-01-25  6:56 [PATCHv4] psi: fix possible trigger missing in the window Huangzhaoyang
  2022-01-25 14:40 ` Johannes Weiner
  2022-02-14 15:05 ` Johannes Weiner
@ 2022-02-17 18:56 ` tip-bot2 for Zhaoyang Huang
  2 siblings, 0 replies; 6+ messages in thread
From: tip-bot2 for Zhaoyang Huang @ 2022-02-17 18:56 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Suren Baghdasaryan, Zhaoyang Huang, Peter Zijlstra (Intel),
	Johannes Weiner, x86, linux-kernel

The following commit has been merged into the sched/core branch of tip:

Commit-ID:     e6df4ead85d9da1b07dd40bd4c6d2182f3e210c4
Gitweb:        https://git.kernel.org/tip/e6df4ead85d9da1b07dd40bd4c6d2182f3e210c4
Author:        Zhaoyang Huang <zhaoyang.huang@unisoc.com>
AuthorDate:    Tue, 25 Jan 2022 14:56:58 +08:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Wed, 16 Feb 2022 15:57:54 +01:00

psi: fix possible trigger missing in the window

When a new threshold breaching stall happens after a psi event was
generated and within the window duration, the new event is not
generated because the events are rate-limited to one per window. If
after that no new stall is recorded then the event will not be
generated even after rate-limiting duration has passed. This is
happening because with no new stall, window_update will not be called
even though threshold was previously breached. To fix this, record
threshold breaching occurrence and generate the event once window
duration is passed.

Suggested-by: Suren Baghdasaryan <surenb@google.com>
Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Acked-by: Suren Baghdasaryan <surenb@google.com>
Link: https://lore.kernel.org/r/1643093818-19835-1-git-send-email-huangzhaoyang@gmail.com
---
 include/linux/psi_types.h |  3 ++-
 kernel/sched/psi.c        | 46 ++++++++++++++++++++++++--------------
 2 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/include/linux/psi_types.h b/include/linux/psi_types.h
index 516c0fe..dc3ec5e 100644
--- a/include/linux/psi_types.h
+++ b/include/linux/psi_types.h
@@ -144,6 +144,9 @@ struct psi_trigger {
 
 	/* Refcounting to prevent premature destruction */
 	struct kref refcount;
+
+	/* Deferred event(s) from previous ratelimit window */
+	bool pending_event;
 };
 
 struct psi_group {
diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c
index cfe76f7..e9d623c 100644
--- a/kernel/sched/psi.c
+++ b/kernel/sched/psi.c
@@ -523,7 +523,7 @@ static void init_triggers(struct psi_group *group, u64 now)
 static u64 update_triggers(struct psi_group *group, u64 now)
 {
 	struct psi_trigger *t;
-	bool new_stall = false;
+	bool update_total = false;
 	u64 *total = group->total[PSI_POLL];
 
 	/*
@@ -532,24 +532,35 @@ static u64 update_triggers(struct psi_group *group, u64 now)
 	 */
 	list_for_each_entry(t, &group->triggers, node) {
 		u64 growth;
+		bool new_stall;
 
-		/* Check for stall activity */
-		if (group->polling_total[t->state] == total[t->state])
-			continue;
+		new_stall = group->polling_total[t->state] != total[t->state];
 
+		/* Check for stall activity or a previous threshold breach */
+		if (!new_stall && !t->pending_event)
+			continue;
 		/*
-		 * Multiple triggers might be looking at the same state,
-		 * remember to update group->polling_total[] once we've
-		 * been through all of them. Also remember to extend the
-		 * polling time if we see new stall activity.
+		 * Check for new stall activity, as well as deferred
+		 * events that occurred in the last window after the
+		 * trigger had already fired (we want to ratelimit
+		 * events without dropping any).
 		 */
-		new_stall = true;
-
-		/* Calculate growth since last update */
-		growth = window_update(&t->win, now, total[t->state]);
-		if (growth < t->threshold)
-			continue;
-
+		if (new_stall) {
+			/*
+			 * Multiple triggers might be looking at the same state,
+			 * remember to update group->polling_total[] once we've
+			 * been through all of them. Also remember to extend the
+			 * polling time if we see new stall activity.
+			 */
+			update_total = true;
+
+			/* Calculate growth since last update */
+			growth = window_update(&t->win, now, total[t->state]);
+			if (growth < t->threshold)
+				continue;
+
+			t->pending_event = true;
+		}
 		/* Limit event signaling to once per window */
 		if (now < t->last_event_time + t->win.size)
 			continue;
@@ -558,9 +569,11 @@ static u64 update_triggers(struct psi_group *group, u64 now)
 		if (cmpxchg(&t->event, 0, 1) == 0)
 			wake_up_interruptible(&t->event_wait);
 		t->last_event_time = now;
+		/* Reset threshold breach flag once event got generated */
+		t->pending_event = false;
 	}
 
-	if (new_stall)
+	if (update_total)
 		memcpy(group->polling_total, total,
 				sizeof(group->polling_total));
 
@@ -1125,6 +1138,7 @@ struct psi_trigger *psi_trigger_create(struct psi_group *group,
 	t->last_event_time = 0;
 	init_waitqueue_head(&t->event_wait);
 	kref_init(&t->refcount);
+	t->pending_event = false;
 
 	mutex_lock(&group->trigger_lock);
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-02-17 18:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-25  6:56 [PATCHv4] psi: fix possible trigger missing in the window Huangzhaoyang
2022-01-25 14:40 ` Johannes Weiner
2022-01-26 16:44   ` Suren Baghdasaryan
2022-02-14 15:05 ` Johannes Weiner
2022-02-14 15:09   ` Peter Zijlstra
2022-02-17 18:56 ` [tip: sched/core] " tip-bot2 for Zhaoyang Huang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.