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 44326C678D4 for ; Thu, 2 Mar 2023 15:30:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B2E826B0074; Thu, 2 Mar 2023 10:30:21 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id ADEC36B0075; Thu, 2 Mar 2023 10:30:21 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9A65E6B0078; Thu, 2 Mar 2023 10:30:21 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 873BA6B0074 for ; Thu, 2 Mar 2023 10:30:21 -0500 (EST) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 471F340781 for ; Thu, 2 Mar 2023 15:30:21 +0000 (UTC) X-FDA: 80524344642.17.C5335A0 Received: from mail-qv1-f50.google.com (mail-qv1-f50.google.com [209.85.219.50]) by imf17.hostedemail.com (Postfix) with ESMTP id 285ED40021 for ; Thu, 2 Mar 2023 15:30:18 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=cmpxchg-org.20210112.gappssmtp.com header.s=20210112 header.b=Yuc3GjL5; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf17.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.219.50 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1677771019; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=MLSfo+hz5vTJ5ohI3IndEOkVuqNdnrD+B9QuUnbZ7t0=; b=XCNVjKuJH7d4W0w5nbmr29hjJTrRuvQJnywD3SOzg64RhCpAmowFJ15z+IZm2WC7/ZkG/N xQAm0Wzg7gBJy3W0I+PSx3DgscsdvrAPJOL2rSs/aWrix6zLJJUxxjipjDyOCL45l9vKAX 5Chpz05oPIYiGV6ilsyvjs+WgppKZQU= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=cmpxchg-org.20210112.gappssmtp.com header.s=20210112 header.b=Yuc3GjL5; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf17.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.219.50 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1677771019; a=rsa-sha256; cv=none; b=RMmblkiHz9kZskr8oMJNrt/6rCVCX5OOlhrIdYL6kkxFiPdeNnF9yl0QfZET3hmQYeV99w t2wmeO2CKclzoKhKlkMEfUnHTwR29qwHaRUO0JnaA2iy4f1roK1byHq0dmT0/eNCUNaVfI cL3srxYolH2jHWkpMeI5i5SeINfMUEM= Received: by mail-qv1-f50.google.com with SMTP id ne1so11855706qvb.9 for ; Thu, 02 Mar 2023 07:30:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20210112.gappssmtp.com; s=20210112; t=1677771018; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=MLSfo+hz5vTJ5ohI3IndEOkVuqNdnrD+B9QuUnbZ7t0=; b=Yuc3GjL5Ae3STASZNhGMCqtIxArcAh5Wnnbvn4E9b8+BMSkHR1RoivucbdqQ/fcWMb pUqUgMFarQvdmGoZEOycuK8epRhiUmc3k8itieZ6PwaIla/rlWd8hVJjACl/b3oqemuw 7JN1JLE/p0Jd9o/WpjBcsFZ9qAJ2kAZ0XvvIW28lUpEwsJgluteqYbfF0IDigG6MlJfI KbHP+F86TOuVcoSsouz+IUw9UwdSOg5N3LugwO1HlldQl2spyTX/dh5e5grSMQOhdIrq XnvvCM7U4ynFjeQLWUvR3t0P/dTBiWq7LQN6dRssMQtt5fGIJ5psvko5kv1o0ITqDoGt slJQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677771018; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=MLSfo+hz5vTJ5ohI3IndEOkVuqNdnrD+B9QuUnbZ7t0=; b=COnZuVgOvcVb4bwVsVIhc2XMvlskVLaDqtkJRkVYwiny/06YZELIY9mFQcVinSd165 qfZHUc62PV/u1Jf9gn2QpzwofUGFc9cFF8WiEd1jxfnGS1ub1IUTRMV3+qb6Hym2RgFa 3PEndlhgrzt8+lTo6htcTCC5XGZKCYwlk57fahSBjYsJcXOQ85HhGyrvGrJtn5IWTJwv 3N0ZoGoafGnJtN9cP3cjBQU7sFahImmthIpz1FpHgHejRTtM0gLZqzQryjBmiAJ/fJAM b0vLYjhXtqH1p3o2v/7n4sPBk8UtzzLjpUoHSLsYlPha7hYr5kxCgeiE3Ze/dkQ+XGgb +WRQ== X-Gm-Message-State: AO0yUKXuKxLqk4bKNrx6sugHyY/VjFE7K81klfaTvv+mGxCYsowx/Loy 2r9E9SZ6m2ZZ+Dy4HxLmqIittg== X-Google-Smtp-Source: AK7set8Zi94EO/G0MiD2tk0zJB/YMF0sCaWg4Nr73waCi4EUvedOqA7UJ9O/l9aYTW8lBwtkvGMozw== X-Received: by 2002:a05:6214:e8f:b0:537:7061:89d7 with SMTP id hf15-20020a0562140e8f00b00537706189d7mr19950984qvb.40.1677771018035; Thu, 02 Mar 2023 07:30:18 -0800 (PST) Received: from localhost ([2620:10d:c091:480::1:19d]) by smtp.gmail.com with ESMTPSA id t3-20020a05620a034300b006fa16fe93bbsm11111379qkm.15.2023.03.02.07.30.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Mar 2023 07:30:17 -0800 (PST) Date: Thu, 2 Mar 2023 10:30:16 -0500 From: Johannes Weiner To: Suren Baghdasaryan Cc: tj@kernel.org, lizefan.x@bytedance.com, peterz@infradead.org, johunt@akamai.com, mhocko@suse.com, keescook@chromium.org, quic_sudaraja@quicinc.com, cgroups@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/1] psi: remove 500ms min window size limitation for triggers Message-ID: References: <20230301193403.1507484-1-surenb@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Queue-Id: 285ED40021 X-Rspamd-Server: rspam09 X-Rspam-User: X-Stat-Signature: wsf17pb6a5ii4dyqaiykm8rx3rh58t5t X-HE-Tag: 1677771018-119642 X-HE-Meta: U2FsdGVkX18/mUXyMqbk00zeFmuYDriQZWRorLSziq99AUg5MH+RmHAntepMNv+rdidm7xXvTbsTOk9TN03dafVUBMTP0fFupGhVitbrNVK5rjq0a4Oxa9AGeIhY37zqCvenCxN/4Zk1FdhM7cTZBU62983us23ZytMwO7kcoh7nbdUpoeqUbys0254t8ZrNDd85oRaNEGhb6XJ3g5pbiehBiSIcWzXKO/cEP8H6rE8zYCcK8LO1yV3RqLY57mznhR7q5y+fNboCuWCopnHRtnlekxXBy8oSWHivNc0chSgfChkCuuyUzg62nskcVG/Lf47gD/BUUBeo//71aI18QjsfTW/X3fBMfETCHshRRLv3anr8i9rzZBUMVMHIVcY19pKUq/4Ydk4K8VsRI7hmIbzUs/04LsCCsOwuwqrdW92OA3tcbsurBrjXbJkHXrwvgZOcnsE0+VOgxoYfm0bUMIHWPmka9nfO0Z5Pr7sRrMasmJkQ74x55gggcFcN6gKT2ZjPcOStZSl40ZnMAjPR12+g/Jt/DfyReFu5asXMIi7HeA+ofZsesUXnlVU7ERgiGJLxOEc3zpyB0YPGAJufwL00PISRFY0A1N5hKSmQqPEjrrK1dRhryFvnLZ1qpdr7vpoJghXNZIFpyege/rwC5lHQFqduTEuzLGF0joD+r5oquQF1JT7hRisxs01hVNnvtWciLUkrZR+ynJnMKZD/Z+iwcAinPSKyTK2KIVldz98iK3+52WDJgweVSCV6D6asKUT/FxbQR2jpUM6jf9gj15IeSzsmyM94DSSibPw+MSSSS8ASzLQZvXsicTE7r864W5EyEB2IKNq6Lk24nzXPn8//IJPMSpH5jKy7tEKTNM4qfyeD3u9tbqO48A6ciLEtBsqcmRp+hLrWgrPlcKm17xeqlBLFqGB9fKqamgTufE62gfJ07l0Rp3feidvhhFFDpl53w9ikDGHWwL8Db30 daif0XuE FeUvIz0X0I6QN1zIxUeUjM5baDT4gPT3iJO18k2QgaCc42J3tEQdvV3QVsMIz9NkKOrDMDtjHI/YRAdd6FD08YfioTAejy7NyzhMnVM8jlYrcSi5LGzfWnLMW5FExbG8Mn/VJVyeU1oaBRtV3BC+lgBKwyhsK3MJGoVih/J0j+aqIiJGNveQN7UszCh0J82uo4pLILzIMgjCO7ezYy/KArdXK5G4MIU+Ube9JB3cFHNMslAexFzPWOr5Zl/A1UuyCGmh21wBmJ+og7Aptlm2D+VYni2xqKwsdMOzj2Qmy83nJ6ph0pj9ux9kfrS+/butiS6O9KQvO4A2mIXcr8Jkr0AtxGWoWZRQIst2ONjU9wHKc48TifSOYc3oPCyKfeZYxRW+mEbQP1gf411Sp5obI7TW5cr5ooRi/6oHZRXJpWlJyV5VPju2a8+lFeNO+d/74qv4NU8EGejspubEGNUYJ3gu/qcXKQ7R5wSd92mZ5lQri1dqWywnywDGjigmVtvKgngfYM3YKE4ze1Stiuz3EhQlrAopl22kvRHP6/wtWzEPxaxNEdBvZnTm8PQn+kxiJT1wC0WWcOQmUCMtNpKBdMq5vwLZBIyocl1kVQChdYgbQMZ8= 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 Wed, Mar 01, 2023 at 12:48:38PM -0800, Suren Baghdasaryan wrote: > On Wed, Mar 1, 2023 at 12:07 PM Johannes Weiner wrote: > > > > On Wed, Mar 01, 2023 at 11:34:03AM -0800, 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 > > > --- > > > kernel/cgroup/cgroup.c | 10 ++++++++++ > > > kernel/sched/psi.c | 4 +--- > > > 2 files changed, 11 insertions(+), 3 deletions(-) > > > > > > diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c > > > index 935e8121b21e..b600a6baaeca 100644 > > > --- a/kernel/cgroup/cgroup.c > > > +++ b/kernel/cgroup/cgroup.c > > > @@ -3867,6 +3867,12 @@ static __poll_t cgroup_pressure_poll(struct kernfs_open_file *of, > > > return psi_trigger_poll(&ctx->psi.trigger, of->file, pt); > > > } > > > > > > +static int cgroup_pressure_open(struct kernfs_open_file *of) > > > +{ > > > + return (of->file->f_mode & FMODE_WRITE && !capable(CAP_SYS_RESOURCE)) ? > > > + -EPERM : 0; > > > +} > > > > I agree with the change, but it's a bit unfortunate that this check is > > duplicated between system and cgroup. > > > > What do you think about psi_trigger_create() taking the file and > > checking FMODE_WRITE and CAP_SYS_RESOURCE against file->f_cred? > > That's definitely doable and we don't even need to pass file to > psi_trigger_create() since it's called only when we write to the file. > However by moving the capability check into psi_trigger_create() we > also postpone the check until write() instead of failing early in > open(). I always assumed failing early is preferable but if > consolidating the code here makes more sense then I can make the > switch. Please let me know if you still prefer to move the check. Just for context, a person on our team is working on allowing unprivileged polls with windows that are multiples of 2s, which can be triggered from the regular aggregator threads. This should be useful for container delegation, and also for the desktop monitor app usecase that Chris Down brought up some time ago. At that point, everybody can open the file for write, and permissions are checked against the trigger parameters. So I don't think it's a big deal to check this particular permission at write time. But if you prefer we can also merge your patch as-is and do the refactor as part of the other series. Your call. In either case, please feel free to add Acked-by: Johannes Weiner