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 2E71AC74A5B for ; Wed, 29 Mar 2023 18:42:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B31146B0080; Wed, 29 Mar 2023 14:42:20 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id ABA996B0081; Wed, 29 Mar 2023 14:42:20 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 936DA6B0082; Wed, 29 Mar 2023 14:42:20 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 7E1C36B0080 for ; Wed, 29 Mar 2023 14:42:20 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 5BE49160D19 for ; Wed, 29 Mar 2023 18:42:20 +0000 (UTC) X-FDA: 80622806040.22.ADB38BC Received: from mail-ed1-f49.google.com (mail-ed1-f49.google.com [209.85.208.49]) by imf15.hostedemail.com (Postfix) with ESMTP id 5A233A0007 for ; Wed, 29 Mar 2023 18:42:17 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=aPynEkp0; spf=pass (imf15.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.49 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1680115337; a=rsa-sha256; cv=none; b=UnntHrPf+dAwZjHl78O2JnMGmavyEaruggy1tTcgfDjFKe5MXCtHU/qoPUSm3tGVqKpFnu UseNcSTNVuQEGGRi87oWSYYwNqEd2t+1gwLLGDPDoDrOULUKDc0onGKNmr+ctBaMhf17KG W7FnWnNXqvxkWP3eD33yocs7/AFRwOw= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=aPynEkp0; spf=pass (imf15.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.49 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1680115337; 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=eIBZAFv2wQMorQ5lc8tGcKMX+DgU71oiqiwG/4lRhic=; b=NtvplfrnMMV0NJbW7yq+BA3IbYV9+zXnfNQI4xdEKwxwgetRwze/8vIhBgjBoy62i5vJVT BZaJooM7JOVNH6yWj+1yLMSNDYF5uBbT8LDsFpzgQUqg87G2q09ZH6oxeOw7FaJzVxsJOq r/mOgf50R+x6UeizjWhyuB3PX1ep2zs= Received: by mail-ed1-f49.google.com with SMTP id b20so67192827edd.1 for ; Wed, 29 Mar 2023 11:42:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1680115336; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=eIBZAFv2wQMorQ5lc8tGcKMX+DgU71oiqiwG/4lRhic=; b=aPynEkp0gt2yrrh6KFRsPFu4+QI+22LUjC71jLtYRdPz3tvtiYrWpQlMXP+93X3yGB FiYhI/lB1nm7o983gXqChQwZJgo7TUHZHozyA54eTsiL7qEpKTDtVNZjtT5OnUGO3FcT /Ghx5u5imvI3Gsd9iJAQJreaU9lhZCpMwvKLL4xwq3MEiupMjCgS2J6LanHI7FZzdNHE DD5BkwhMH87ZYXPw7tl2iLTM3LS+8SO6AidoBxBPFw0iS+D9jzkDX1w9ejhJCbS882ck 3Y9hQaaQPnDbj315Xgi6CAnGj+I61uN4wFCd/FWxc6pmq9vBaD7hAvSlXrhKcXQw6j9r 0zJQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680115336; h=content-transfer-encoding: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=eIBZAFv2wQMorQ5lc8tGcKMX+DgU71oiqiwG/4lRhic=; b=jtYNe3ZQEyzU+XBAhFkUbXHDSMJZK9b4lwC6WjQzQzm0Pagq/yvbDj6mRB6YT8M+iv sMKklm2nmc6nqC3uqJytwTswUQs0j9Zx8DdmSa+onO+nRSAjhgwqJOa3JTHw+VdpR5Eq Nd8LuQ90HXps0VPHAbtzLS3FEMtdzpHwudnFrtr5CoRcAy6lHfrbLHXvAxA3IU3b7ua6 +XBnbSxkTKk6dWhUL5zLj1kqfI3JVZQDi1l3P2D2drLCXVUt4zT57W13Bo49wCkdTYDw YQ4JpxRBy+Nf4xV7m3Djn2Mn/EdY6nFAkxLcz51Wg8uO4XbeTbdDXL+vaGFzcz7A4uLl 9Qpg== X-Gm-Message-State: AAQBX9eOV9zm71m87VOyADemzawXrWjxNocLCu8T6bIoYfM1qs2Gq2+c bqYfBWXgrPPgQdvqx5uFEuIAGRyRwOFhgvmGgcQtbw== X-Google-Smtp-Source: AKy350ZwxdVpDFdjfiQjd2T1+2RQI0O05U4sIZAUQmMvbedjibegmVGA6poPnWutEDJaZTCc4OE/bJRiTbzJipp2VXo= X-Received: by 2002:a17:906:81da:b0:92f:b329:cb75 with SMTP id e26-20020a17090681da00b0092fb329cb75mr2122145ejx.5.1680115335713; Wed, 29 Mar 2023 11:42:15 -0700 (PDT) MIME-Version: 1.0 References: <20230328221644.803272-1-yosryahmed@google.com> <20230328221644.803272-5-yosryahmed@google.com> In-Reply-To: From: Yosry Ahmed Date: Wed, 29 Mar 2023 11:41:39 -0700 Message-ID: Subject: Re: [PATCH v2 4/9] cgroup: rstat: add WARN_ON_ONCE() if flushing outside task context To: Michal Hocko Cc: Tejun Heo , Josef Bacik , Jens Axboe , Zefan Li , Johannes Weiner , Roman Gushchin , Shakeel Butt , Muchun Song , Andrew Morton , =?UTF-8?Q?Michal_Koutn=C3=BD?= , Vasily Averin , cgroups@vger.kernel.org, linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, bpf@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Queue-Id: 5A233A0007 X-Rspamd-Server: rspam01 X-Stat-Signature: grbfw1eoa5radas7nsax3ziowwu84fqn X-HE-Tag: 1680115337-550883 X-HE-Meta: U2FsdGVkX18sRtpgafqEL6A8+XPYZFOl+bK4sBheYtkL+/+nuE9BMKd97rmgWaPibGSot1227c75klNzJMEW/NKdayzj7DR+RaZeFzu9YiY0h0N1Qg77TE5EyYIjtWLyi2YwMlNUPiv+/AcXkkBtk4c1FcTGu3O4g0R3zYhYYeVWWLs2dUFprKeM3h/4gjex8g4jHptLHfvJDLbZOtXy40HY8BJ8Qt5UAFwXE2cjfh00Iy2aSOoJzw3dnjSLNbNh/cpzvXrpZ0xuQ9WW11UrP0vsGDEBXM8q8QGyg/HEgn2iX/anSmYNV1j/nknkshc8FwkUC3h9M5P73XVdmGmn/YJQ9pvoXkZD472GMdDOUN/FbL+sjrdEyCKn8jD9cUXNRtWaHyO1LW8LRNtAUJKcLyQR58gDFtMSIIyWEZooQSkV59DUXl8dQpS4h5Jxu7/qmTZsvVTs1fDdRxvHeLKl/eyl58jYEWhKSKJGynvUC4pep0pP2dd+Plp+y03XZQvMcQq43VSXyV4aNajrcSSdwOyIjD5m/XjB1UZNUx+l9P1QYNr6/mppY05CKFRNTEqEwbOSsAzbe6f/u27Tu4Oyq7q7k+m5/n2n3jPQuGsEiRqNj3gEK7aPEl7VH6ANlbWlsfpJMyt5JVwan45PvI3yB22JzggNlV/LF9+yK77cvPQqBw13wlEXrzlCYyTvnORqsDhI5trkKLcKMdsnLuSbWhdXuZDlFdwC2Lg+BJ8c7sQVRUalPQpHVHWviPP4sC3pV3RdVklmFNQxDWyeRa6ks/uQhRs6AvC4IP+qzPawTrSCe9VvhRVudFRH9J14HZ0kM3F1L5wxEV62QhsVPMgHHTOgqtxD3U2FfBn7PAmq+BPprn0NQGEbrP9mxCrj2qs0rrA8CrJ5ITEmvpgJBdZTsPnmva5ebro/u2mnIPGo1Q62lW+M5yYZQLwKBqNVGbBX6raWkM+p91f1zAGNko9 7lK7zWRT Jf1J9rxYnT2S0aVza9qBKU2MwUDsmzVXlO5kML5tDewWMXaYG+SRat1Hh7LjbEiP77prwivsNrJjbXi1D2YMn6BPxdv9kkGp/31FRI84xq4I+ZgQR+tPDub0HIn9dZzPs4uKfHOsyxLir7kDjFZ5TOh4xV4QfTOhFakjtUW9NNuh4OtW5zgI5KDcyWCNXamhF7YM0qnnPMmTdsWEE296ZTe2E9rpWnsLXlXlmrzpLSPxZqcK+RfntDIXUhC4CTxZqbgIdP/pvCqcX7yN68ECuHbYvaSTH6uPbYZNXSFRM61SEjmmz1q4NAN9zf7M4NoVfpPck+s/4owaxLRN2zughHCs5qwGGJBVU6igOe5kfHNhL7X8iBW4KOd4+bw== 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 29, 2023 at 4:22=E2=80=AFAM Michal Hocko wrot= e: > > On Tue 28-03-23 22:16:39, Yosry Ahmed wrote: > > rstat flushing is too expensive to perform in irq context. > > The previous patch removed the only context that may invoke an rstat > > flush from irq context, add a WARN_ON_ONCE() to detect future > > violations, or those that we are not aware of. > > > > Ideally, we wouldn't flush with irqs disabled either, but we have one > > context today that does so in mem_cgroup_usage(). Forbid callers from > > irq context for now, and hopefully we can also forbid callers with irqs > > disabled in the future when we can get rid of this callsite. > > I am sorry to be late to the discussion. I wanted to follow up on > Johannes reply in the previous version but you are too fast ;) > > I do agree that this looks rather arbitrary. You do not explain how the > warning actually helps. Is the intention to be really verbose to the > kernel log when somebody uses this interface from the IRQ context and > get bug reports? What about configurations with panic on warn? Do we > really want to crash their systems for something like that? Thanks for taking a look, Michal! The ultimate goal is not to flush in irq context or with irqs disabled, as in some cases it causes irqs to be disabled for a long time, as flushing is an expensive operation. The previous patch in the series should have removed the only context that flushes in irq context, and the purpose of the WARN_ON_ONCE() is to catch future uses or uses that we might have missed. There is still one code path that flushes with irqs disabled (also mem_cgroup_usage()), and we cannot remove this just yet; we need to deprecate usage threshold events for root to do that. So we cannot enforce not flushing with irqs disabled yet. So basically the patch is trying to enforce what we have now, not flushing in irq context, and hopefully at some point we will also be able to enforce not flushing with irqs disabled. If WARN_ON_ONCE() is the wrong tool for this, please let me know. > > > Signed-off-by: Yosry Ahmed > > Reviewed-by: Shakeel Butt > > --- > > kernel/cgroup/rstat.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c > > index d3252b0416b6..c2571939139f 100644 > > --- a/kernel/cgroup/rstat.c > > +++ b/kernel/cgroup/rstat.c > > @@ -176,6 +176,8 @@ static void cgroup_rstat_flush_locked(struct cgroup= *cgrp, bool may_sleep) > > { > > int cpu; > > > > + /* rstat flushing is too expensive for irq context */ > > + WARN_ON_ONCE(!in_task()); > > lockdep_assert_held(&cgroup_rstat_lock); > > > > for_each_possible_cpu(cpu) { > > -- > > 2.40.0.348.gf938b09366-goog > > -- > Michal Hocko > SUSE Labs