From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id E4AE8C433EF for ; Tue, 21 Dec 2021 22:40:42 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 27E4C6B0071; Tue, 21 Dec 2021 17:40:42 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 22E396B0073; Tue, 21 Dec 2021 17:40:42 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0CF616B0074; Tue, 21 Dec 2021 17:40:42 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id ED9866B0071 for ; Tue, 21 Dec 2021 17:40:41 -0500 (EST) Received: from smtpin08.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id AF2468A3E2 for ; Tue, 21 Dec 2021 22:40:41 +0000 (UTC) X-FDA: 78943272282.08.72E6E37 Received: from mail-yb1-f179.google.com (mail-yb1-f179.google.com [209.85.219.179]) by imf06.hostedemail.com (Postfix) with ESMTP id 3F8DA18001D for ; Tue, 21 Dec 2021 22:40:40 +0000 (UTC) Received: by mail-yb1-f179.google.com with SMTP id e136so761705ybc.4 for ; Tue, 21 Dec 2021 14:40:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=hD0pZoQcS3KfcyiV0zGmIHG20iPzEg5VDSH1vfbov/g=; b=pW8gWI/jXD1bJTfs0SmV/vRFgZ/l0OgWGoCeQJIUXMToVGWQjowAEFmE8bTjEsXKKr OVz91r8Apn2zb1pfGQ9THwvt3YnCMt8lJ23OyzFvzWbfoqY6SHp4LmtQPU6Mhca3xptN Vu/8AAsttTOCKpmdyPLx8d4SZ8c69vNhZ/BUOWwQXLKZSpEJh7Ybh4KSwAKUypIzIGJH Twh2S8CKOHJqEpLpSJP+hltUxa3yo5zcak81sXNHz+2O9FwcHB370RoHYmh1gvkL4ESL THgawQXsrZImhr0IoF4r72Ij0TSvMKTPuDBaRlJ7XKgR0t9izDBll9bPslN+n4j8UB6/ rbSQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=hD0pZoQcS3KfcyiV0zGmIHG20iPzEg5VDSH1vfbov/g=; b=ZdbII59w3giEJsY5meuxLNHRrkmtyt8wC5ZVw/HMfgFMz2mug+U2b4f8rSLtcs9Y56 9WriLWVwomsWaHQUZdd9+/g/60imGoQEEte9xZd8x4oih1sNscMKOlvBQFzhbg51413p rTT7uFkXgpWxlnXe5WFmCkF62nfK7Pap/Ji4YTjd7kvWoj1dVQLsXpS7tBAvIAi/5yKI OGSRPATxht7lFcYnpVAyD/k7ArkRwhRVx5dW1qXj4Qq4HUDhUJ8EZC6GFqvI+BHypx75 LAXbVt1Adlqwi5Jr8ztmDCvxcbTeLim/k7+O+xS65k4piJv7obg5mzJb1Ef6P9xeqFyT HAbA== X-Gm-Message-State: AOAM531lIIGJ/fry0Yias4pBwAC96vXaZq7SWSLi3GzaEjWKm8UNSmh0 g+iqVTzy2+7nkdXWfZt5oXViJdBbMwqd+N1Hdq9Y1Q== X-Google-Smtp-Source: ABdhPJwewWZuE8UNQR14Q3RT96/FbV3ekF2iUw1OO7au3+ISkSasPaUEout5OHOQO2w1HJi/gQW2EnKR18y+6kndlkg= X-Received: by 2002:a5b:c0b:: with SMTP id f11mr694892ybq.488.1640126440296; Tue, 21 Dec 2021 14:40:40 -0800 (PST) MIME-Version: 1.0 References: <1640075637-14520-1-git-send-email-huangzhaoyang@gmail.com> In-Reply-To: <1640075637-14520-1-git-send-email-huangzhaoyang@gmail.com> From: Suren Baghdasaryan Date: Tue, 21 Dec 2021 14:40:29 -0800 Message-ID: Subject: Re: [PATCH v2] psi: fix possible trigger missing in the window To: Huangzhaoyang Cc: Johannes Weiner , Zhaoyang Huang , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Peter Zijlstra Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 3F8DA18001D X-Stat-Signature: 6jjcpakazaaon3rb5sdgy8kouiyy5cj5 Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b="pW8gWI/j"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf06.hostedemail.com: domain of surenb@google.com designates 209.85.219.179 as permitted sender) smtp.mailfrom=surenb@google.com X-Rspamd-Server: rspam02 X-HE-Tag: 1640126440-98277 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Dec 21, 2021 at 12:34 AM Huangzhaoyang wrote: > > From: Zhaoyang Huang CC'ing PeterZ since I think this change will need to be accepted into his tree. Please include Peter in the future versions of this patch. > > There could be missing wake up if the rest of the window remain the > same stall states as the polling_total updates for every polling_min_period. > Introducing threshold_breach flag to record the trigger's status and update > the logic. At least for me, it's hard to understand the problem from this description. Suggest something like this: 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. The code looks good to me. Thanks! > > Suggested-by: Suren Baghdasaryan > Signed-off-by: Zhaoyang Huang > --- > v2: modify the logic according to Suren's suggestion > --- > --- > include/linux/psi_types.h | 2 ++ > kernel/sched/psi.c | 38 +++++++++++++++++++++++--------------- > 2 files changed, 25 insertions(+), 15 deletions(-) > > diff --git a/include/linux/psi_types.h b/include/linux/psi_types.h > index 0a23300..87b694a 100644 > --- a/include/linux/psi_types.h > +++ b/include/linux/psi_types.h > @@ -132,6 +132,8 @@ struct psi_trigger { > > /* Refcounting to prevent premature destruction */ > struct kref refcount; > + > + bool threshold_breach; > }; > > struct psi_group { > diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c > index 1652f2b..5c67ab9 100644 > --- a/kernel/sched/psi.c > +++ b/kernel/sched/psi.c > @@ -524,24 +524,29 @@ static u64 update_triggers(struct psi_group *group, u64 now) > */ > list_for_each_entry(t, &group->triggers, node) { > u64 growth; > + bool trigger_stalled = > + group->polling_total[t->state] != total[t->state]; > > - /* Check for stall activity */ > - if (group->polling_total[t->state] == total[t->state]) > - 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. > - */ > - new_stall = true; > - > - /* Calculate growth since last update */ > - growth = window_update(&t->win, now, total[t->state]); > - if (growth < t->threshold) > + /* Check for stall activity or a previous threshold breach */ > + if (!trigger_stalled && !t->threshold_breach) > continue; > > + if (trigger_stalled) { > + /* > + * 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. > + */ > + new_stall = true; > + > + /* Calculate growth since last update */ > + growth = window_update(&t->win, now, total[t->state]); > + if (growth < t->threshold) > + continue; > + > + t->threshold_breach = true; > + } > /* Limit event signaling to once per window */ > if (now < t->last_event_time + t->win.size) > continue; > @@ -550,6 +555,8 @@ 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->threshold_breach = false; > } > > if (new_stall) > @@ -1152,6 +1159,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->threshold_breach = false; > > mutex_lock(&group->trigger_lock); > > -- > 1.9.1 >