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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 708CFC678D4 for ; Thu, 2 Mar 2023 16:09:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229748AbjCBQJV (ORCPT ); Thu, 2 Mar 2023 11:09:21 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39370 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229740AbjCBQJS (ORCPT ); Thu, 2 Mar 2023 11:09:18 -0500 Received: from mail-yw1-x1129.google.com (mail-yw1-x1129.google.com [IPv6:2607:f8b0:4864:20::1129]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2D2631517C for ; Thu, 2 Mar 2023 08:09:07 -0800 (PST) Received: by mail-yw1-x1129.google.com with SMTP id 00721157ae682-5384ff97993so436558137b3.2 for ; Thu, 02 Mar 2023 08:09:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1677773329; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=o4oQ31BBWd17/OrBmaIYzd49tP3Dix54xt4VYG6efag=; b=c68m6eik+53s5ghZ8Wk6fDeh16+MIGLL2unfAA/ShiTQX81DA+PrcE8XnLQGzZCvZQ vN2XPVLbn1wfC/8NCIubij0nUZJqxKcCmBxWTOn6qAUtuflqTF9mZvIdkS3e3c0vzbH4 qsMNWAQtrEFDCDscPFpvLxjJdM4HlDeWrxdj8WSt1tlECtIHKD1+HqjwILLeSnLzHibu bAOXTvREbnpopYu1ry3LZ8SS4rFApGoN7zpEEmt/iN5oxzI/PXgiH5odoJEZJ656yeRH /8tViM5OehqWujUVRORIpO0V2vnIVAPeaDw2AhQ8WpgPomB5qvQZvmUB/kQ0Db9oEprd zqMw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677773329; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=o4oQ31BBWd17/OrBmaIYzd49tP3Dix54xt4VYG6efag=; b=RaH6HfnRDbdzcDz5LYq0jq1tPpLWRGLOWxhs77lSmCkCUeVVDmHuxsbqKXSmQK6II2 F7iHRUNImm/56uHZp451Mee3wy4buOJPbU1IklIoXpbqBrJI5Yu/GWP39rcc0pkk/UxF k+lXKidiyfAvgenzbEW2qZK6rFX7sZasqzubiLhpPKXA8Uzlvn9ho79MJ0vWSn3AfG6G epiBeFbQO2dV9h7fawbE9n2cZehNVnGnlNOIr8/W0FNx752iEOrLSwD0THzDMtKM/EHx TqlNTCJ1eQHu82qDDQPvFlzepNjw9U1fHOiy4BKPdFsnPajWzB4nuZdp57gchSc3pw1F 5eiw== X-Gm-Message-State: AO0yUKV9iTy4he/vY2dupDYl+TfNq2gKFAjt3fCzFko7UN//Xx3zD6Wj hTqglt50mAZti34qI91N4JVA67nTe5Tz742NQe9zrg== X-Google-Smtp-Source: AK7set8k2bnR4c69h4GFdeOlJ4iXt43r+FFbCpo8zSPi+823yfuAQIu8hrDp4KyWpc4vzcJgN/+M6QusrqrGoKPfgPI= X-Received: by 2002:a81:ac16:0:b0:536:4194:e6eb with SMTP id k22-20020a81ac16000000b005364194e6ebmr6615295ywh.0.1677773329554; Thu, 02 Mar 2023 08:08:49 -0800 (PST) MIME-Version: 1.0 References: <20230301193403.1507484-1-surenb@google.com> In-Reply-To: From: Suren Baghdasaryan Date: Thu, 2 Mar 2023 08:08:38 -0800 Message-ID: Subject: Re: [PATCH 1/1] psi: remove 500ms min window size limitation for triggers To: Michal Hocko Cc: tj@kernel.org, hannes@cmpxchg.org, lizefan.x@bytedance.com, peterz@infradead.org, johunt@akamai.com, keescook@chromium.org, quic_sudaraja@quicinc.com, cgroups@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Mar 2, 2023 at 5:58 AM Michal Hocko wrote: > > On Wed 01-03-23 11:34:03, Suren Baghdasaryan wrote: > > Current 500ms min window size for psi triggers limits polling interval > > to 50ms to prevent polling threads from using too much cpu bandwidth by > > polling too frequently. However the number of cgroups with triggers is > > unlimited, so this protection can be defeated by creating multiple > > cgroups with psi triggers (triggers in each cgroup are served by a single > > "psimon" kernel thread). > > Instead of limiting min polling period, which also limits the latency of > > psi events, it's better to limit psi trigger creation to authorized users > > only, like we do for system-wide psi triggers (/proc/pressure/* files can > > be written only by processes with CAP_SYS_RESOURCE capability). This also > > makes access rules for cgroup psi files consistent with system-wide ones. > > Add a CAP_SYS_RESOURCE capability check for cgroup psi file writers and > > remove the psi window min size limitation. > > > > Suggested-by: Sudarshan Rajagopalan > > Link: https://lore.kernel.org/all/cover.1676067791.git.quic_sudaraja@quicinc.com/ > > Signed-off-by: Suren Baghdasaryan > > Acked-by: Michal Hocko > > with this to fix > [...] > > @@ -1278,8 +1277,7 @@ struct psi_trigger *psi_trigger_create(struct psi_group *group, > > if (state >= PSI_NONIDLE) > > return ERR_PTR(-EINVAL); > > > > - if (window_us < WINDOW_MIN_US || > > - window_us > WINDOW_MAX_US) > > + if (window_us <= 0 || window_us > WINDOW_MAX_US) > > return ERR_PTR(-EINVAL); > > window_us is u32 sp the check for <= 0 doesn't make any sense. Completely missed that. Will change to == 0 and post the new version. window_us is later multiplied by NSEC_PER_USEC and then divided by UPDATES_PER_WINDOW(10), so even the smallest value of 1 still results in a positive poll_min_period. I think we should be fine with that check. Thanks! > > > > > /* Check threshold */ > > -- > > 2.40.0.rc0.216.gc4246ad0f0-goog > > -- > Michal Hocko > SUSE Labs From mboxrd@z Thu Jan 1 00:00:00 1970 From: Suren Baghdasaryan Subject: Re: [PATCH 1/1] psi: remove 500ms min window size limitation for triggers Date: Thu, 2 Mar 2023 08:08:38 -0800 Message-ID: References: <20230301193403.1507484-1-surenb@google.com> Mime-Version: 1.0 Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1677773329; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=o4oQ31BBWd17/OrBmaIYzd49tP3Dix54xt4VYG6efag=; b=c68m6eik+53s5ghZ8Wk6fDeh16+MIGLL2unfAA/ShiTQX81DA+PrcE8XnLQGzZCvZQ vN2XPVLbn1wfC/8NCIubij0nUZJqxKcCmBxWTOn6qAUtuflqTF9mZvIdkS3e3c0vzbH4 qsMNWAQtrEFDCDscPFpvLxjJdM4HlDeWrxdj8WSt1tlECtIHKD1+HqjwILLeSnLzHibu bAOXTvREbnpopYu1ry3LZ8SS4rFApGoN7zpEEmt/iN5oxzI/PXgiH5odoJEZJ656yeRH /8tViM5OehqWujUVRORIpO0V2vnIVAPeaDw2AhQ8WpgPomB5qvQZvmUB/kQ0Db9oEprd zqMw== In-Reply-To: List-ID: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Michal Hocko Cc: tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org, lizefan.x-EC8Uxl6Npydl57MIdRCFDg@public.gmane.org, peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org, johunt-JqFfY2XvxFXQT0dZR+AlfA@public.gmane.org, keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org, quic_sudaraja-jfJNa2p1gH1BDgjK7y7TUQ@public.gmane.org, cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On Thu, Mar 2, 2023 at 5:58 AM Michal Hocko wrote: > > On Wed 01-03-23 11:34:03, Suren Baghdasaryan wrote: > > Current 500ms min window size for psi triggers limits polling interval > > to 50ms to prevent polling threads from using too much cpu bandwidth by > > polling too frequently. However the number of cgroups with triggers is > > unlimited, so this protection can be defeated by creating multiple > > cgroups with psi triggers (triggers in each cgroup are served by a single > > "psimon" kernel thread). > > Instead of limiting min polling period, which also limits the latency of > > psi events, it's better to limit psi trigger creation to authorized users > > only, like we do for system-wide psi triggers (/proc/pressure/* files can > > be written only by processes with CAP_SYS_RESOURCE capability). This also > > makes access rules for cgroup psi files consistent with system-wide ones. > > Add a CAP_SYS_RESOURCE capability check for cgroup psi file writers and > > remove the psi window min size limitation. > > > > Suggested-by: Sudarshan Rajagopalan > > Link: https://lore.kernel.org/all/cover.1676067791.git.quic_sudaraja-jfJNa2p1gH1BDgjK7y7TUQ@public.gmane.org/ > > Signed-off-by: Suren Baghdasaryan > > Acked-by: Michal Hocko > > with this to fix > [...] > > @@ -1278,8 +1277,7 @@ struct psi_trigger *psi_trigger_create(struct psi_group *group, > > if (state >= PSI_NONIDLE) > > return ERR_PTR(-EINVAL); > > > > - if (window_us < WINDOW_MIN_US || > > - window_us > WINDOW_MAX_US) > > + if (window_us <= 0 || window_us > WINDOW_MAX_US) > > return ERR_PTR(-EINVAL); > > window_us is u32 sp the check for <= 0 doesn't make any sense. Completely missed that. Will change to == 0 and post the new version. window_us is later multiplied by NSEC_PER_USEC and then divided by UPDATES_PER_WINDOW(10), so even the smallest value of 1 still results in a positive poll_min_period. I think we should be fine with that check. Thanks! > > > > > /* Check threshold */ > > -- > > 2.40.0.rc0.216.gc4246ad0f0-goog > > -- > Michal Hocko > SUSE Labs