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 E87F5C433EF for ; Mon, 20 Dec 2021 19:58:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 101566B0071; Mon, 20 Dec 2021 14:58:51 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 0B0B56B0073; Mon, 20 Dec 2021 14:58:51 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E93056B0074; Mon, 20 Dec 2021 14:58:50 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0037.hostedemail.com [216.40.44.37]) by kanga.kvack.org (Postfix) with ESMTP id D76BA6B0071 for ; Mon, 20 Dec 2021 14:58:50 -0500 (EST) Received: from smtpin06.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 889DF8A3D3 for ; Mon, 20 Dec 2021 19:58:50 +0000 (UTC) X-FDA: 78939235620.06.9393825 Received: from mail-yb1-f172.google.com (mail-yb1-f172.google.com [209.85.219.172]) by imf08.hostedemail.com (Postfix) with ESMTP id 1666D160029 for ; Mon, 20 Dec 2021 19:58:43 +0000 (UTC) Received: by mail-yb1-f172.google.com with SMTP id v64so32075041ybi.5 for ; Mon, 20 Dec 2021 11:58:49 -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=9lDdg+MUWH6RIaXbhbBHcadu/VOGGuYgmaFf6YkYH2Q=; b=eMvVq35drMk8sk2dkSowFPRdBDicG95euCGgm7DAonJxf+ofRXeRuDR2cb40/LWiuA 1kaphwTNameVPDRMN3t6ORd2Pgf5oSp2MwFY32oWvqDe0TIWR1byGrWOqWtL5d3ipt/h rp+uMFVK/YnuwRBHWA0qWpkeolVET3vw9OGJgzKKtIxlyoeEJjPgZieOjY7NMHwNNVwt Pbs0gpvz0CAIafq4Ah67fkVSPp70jHRtLAvaGfTgnmURpaCDkTXKMePqwgPx8m1zQ3qe 61F7+NlWvAz8C8A5uxxvxUmQxZYHdIng4uICkrIlFneTFL9WDsqFvFyV3DZjIB4P/Omh gytg== 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=9lDdg+MUWH6RIaXbhbBHcadu/VOGGuYgmaFf6YkYH2Q=; b=qPiHyOM5OH/0txn8p0DcbPbczm9o5efR3BlElfCuYWyhvUhvx81zcORthMJ3yNi9Dz ulyWTYee5X/R8kQAin7EUqVqI4XlzlkOaoDOsrbk8qn6nA2woz+71mlwDHGkJkYRb+9w duxeddq4lCTMz3jg0tfUCY0zxPnLbY+aShgYWA6H0Qfu/wrcLEUiMbRqtngh4WVfworp 8hLQsX8NIceKLU0ejNUJd850vl55LNpCjpl0wU4u+/GDwUeHZpYvj3SNXSeOMCUHj1FT WNwBBntNWl32RiPjavE3YNaXVXi80sI/4M9ZAwQpreTSneoDriBU4J3XRDCJCQDNNjJp pk/w== X-Gm-Message-State: AOAM532XxWnBZDd5C93sPuT1QJApJOPfoFHr/azG2TGrC3kvSYmqCbb+ TKm9Ryb2edg+uosavOoMhKuTSMkKaXWqmKWfurz+7A== X-Google-Smtp-Source: ABdhPJyfeFtLMyeNs1xYvK67zNRo6BTDnF2e8rJqc+2MvbKjgNvR9PCFuyWX7fukFlnsXRA9KWlpUpBT+ENQ6X1OqTg= X-Received: by 2002:a25:d186:: with SMTP id i128mr10844633ybg.602.1640030329168; Mon, 20 Dec 2021 11:58:49 -0800 (PST) MIME-Version: 1.0 References: <1639721264-12294-1-git-send-email-huangzhaoyang@gmail.com> In-Reply-To: From: Suren Baghdasaryan Date: Mon, 20 Dec 2021 11:58:38 -0800 Message-ID: Subject: Re: [PATCH] psi: fix possible trigger missing in the window To: Zhaoyang Huang Cc: Johannes Weiner , Zhaoyang Huang , "open list:MEMORY MANAGEMENT" , LKML Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=eMvVq35d; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf08.hostedemail.com: domain of surenb@google.com designates 209.85.219.172 as permitted sender) smtp.mailfrom=surenb@google.com X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 1666D160029 X-Stat-Signature: gbpabzz6py83fcuwq5tp4j11sfephu3y X-HE-Tag: 1640030323-471941 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 Fri, Dec 17, 2021 at 10:03 PM Zhaoyang Huang wrote: > > loop Suren Thanks. > > On Fri, Dec 17, 2021 at 2:08 PM Huangzhaoyang wrote: > > > > From: Zhaoyang Huang > > > > 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. Could you please expand on this description? I'm unclear what the problem is. I assume "polling_min_period" in this description refers to the group->poll_min_period. >From the code, looks like the change results in update_triggers() calling window_update() once there was a new stall recorded for the trigger state and until the tracking window is complete. I don't see the point of calling window_update() if there was no stall change since the last call to window_update(). The resulting growth will not increase if there is no new stall. Maybe what you want to achieve here is more than one trigger per window if the stall limit was breached? If so, then this goes against the design for psi triggers in which we want to rate-limit the number of generated triggers per tracking window (see: https://elixir.bootlin.com/linux/latest/source/kernel/sched/psi.c#L545). Please clarify the issue and the intentions here. Thanks! > > > > Signed-off-by: Zhaoyang Huang > > --- > > include/linux/psi_types.h | 2 ++ > > kernel/sched/psi.c | 30 ++++++++++++++++++------------ > > 2 files changed, 20 insertions(+), 12 deletions(-) > > > > diff --git a/include/linux/psi_types.h b/include/linux/psi_types.h > > index 0a23300..9533d2e 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 new_stall; > > }; > > > > struct psi_group { > > diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c > > index 1652f2b..402718c 100644 > > --- a/kernel/sched/psi.c > > +++ b/kernel/sched/psi.c > > @@ -458,9 +458,12 @@ static void psi_avgs_work(struct work_struct *work) > > static void window_reset(struct psi_window *win, u64 now, u64 value, > > u64 prev_growth) > > { > > + struct psi_trigger *t = container_of(win, struct psi_trigger, win); > > + > > win->start_time = now; > > win->start_value = value; > > win->prev_growth = prev_growth; > > + t->new_stall = false; > > } > > > > /* > > @@ -515,7 +518,6 @@ 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; > > u64 *total = group->total[PSI_POLL]; > > > > /* > > @@ -523,19 +525,26 @@ static u64 update_triggers(struct psi_group *group, u64 now) > > * watchers know when their specified thresholds are exceeded. > > */ > > list_for_each_entry(t, &group->triggers, node) { > > - u64 growth; > > - > > /* 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. > > + * update the trigger if there is new stall which will be > > + * reset when run out of the window > > */ > > - new_stall = true; > > + t->new_stall = true; > > + > > + memcpy(&group->polling_total[t->state], &total[t->state], > > + sizeof(group->polling_total[t->state])); > > + } > > + > > + list_for_each_entry(t, &group->triggers, node) { > > + u64 growth; > > + > > + /* check if new stall happened during this window*/ > > + if (!t->new_stall) > > + continue; > > > > /* Calculate growth since last update */ > > growth = window_update(&t->win, now, total[t->state]); > > @@ -552,10 +561,6 @@ static u64 update_triggers(struct psi_group *group, u64 now) > > t->last_event_time = now; > > } > > > > - if (new_stall) > > - memcpy(group->polling_total, total, > > - sizeof(group->polling_total)); > > - > > return now + group->poll_min_period; > > } > > > > @@ -1152,6 +1157,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->new_stall = false; > > > > mutex_lock(&group->trigger_lock); > > > > -- > > 1.9.1 > >