| Message ID | 1643093818-19835-1-git-send-email-huangzhaoyang@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [PATCHv4] psi: fix possible trigger missing in the window | expand |
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>
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>
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!
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!
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);