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 833DFC433EF for ; Fri, 18 Feb 2022 17:25:45 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1553D6B0071; Fri, 18 Feb 2022 12:25:45 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 0DCDC6B0074; Fri, 18 Feb 2022 12:25:45 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EC07F6B0075; Fri, 18 Feb 2022 12:25:44 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0018.hostedemail.com [216.40.44.18]) by kanga.kvack.org (Postfix) with ESMTP id DA78D6B0071 for ; Fri, 18 Feb 2022 12:25:44 -0500 (EST) Received: from smtpin13.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 93D279AC8D for ; Fri, 18 Feb 2022 17:25:44 +0000 (UTC) X-FDA: 79156577808.13.4118579 Received: from mail-lj1-f181.google.com (mail-lj1-f181.google.com [209.85.208.181]) by imf10.hostedemail.com (Postfix) with ESMTP id 2A462C0003 for ; Fri, 18 Feb 2022 17:25:43 +0000 (UTC) Received: by mail-lj1-f181.google.com with SMTP id e2so1908045ljq.12 for ; Fri, 18 Feb 2022 09:25:43 -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=uJsUkkYuKsFjmFuSrzJx6Z65BOc8raptsVlPvLM8N1M=; b=DQrnOr+7ZnH78FYvqffaE346vGIkV2x2C9ECWU9yqE4RVKdgMog75eA1y87iH06bUW xH0/fu78Skp4CvfXhzy6o2zYv3LGx/qwRJulidi4KrOjrA2U42XmP28RlZShOeNgqbka Cs4C0ETSQhG/vv/IDOf0o2HHaiUmd1ZFgIfwxIZLWbO/MOvqIYCazVduLcs8zp0wuuhI 4+v5fciZgeeBNgdpAYcl2j/woTbUupNnDfY1AItJjxVg8pIpsVMjcLjfamvYsBd9zqnU mIcZXICCAD8nhgSnUNP4ziv0mZr+imOkr6s1d5lyGDxGlLaYYsjX1lA1RAZE9JqtUBeI EzSw== 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=uJsUkkYuKsFjmFuSrzJx6Z65BOc8raptsVlPvLM8N1M=; b=3jtOfnnpE/GcDKxzwcX819j0C96pnQn0PYQ99iqvi3sOivHKPLWPoW/woi5IvEhYHS ZvPsEnzafhbJPCGT174BoBFnT2+drBYOfBJrceeVY2dGqkB4mMKNKZxmeddt/mmIXPZH 993vClMvqC8dEJEoVKXh58oCLihFZjgOLFIZOdf4dUQemMR6/48cbdd+aEOBzlhDcFKW IfWQpmxMMPOsIeynuH1UZe+79+GLjvO2S8yRh1PyYXlQoeXtuUOprfu/dzZJ8nhS5we7 04t5aXK6UFaNRAKoUxvE3ueTKfDc9R2z11b6r+9hUXb/ZB0LKf7L7TII/g5JPz1Qf0Yp YQdg== X-Gm-Message-State: AOAM533mONj6qBCP+L5zGzPoXq2MIMxugkQC/alCtrg40s/LXkHh2EMX tvS8/nSRNfcF7lbhoxTXCO0YdIt7D5B6hcBZ1bgXiA== X-Google-Smtp-Source: ABdhPJz7B845uKGybPywxSqX/oQBwMJEoTY1oe+5SmrKHMrw+7UPwyBsRDz8K3Lsg8DgAiBpIvy8JJxQkuPWYU4456I= X-Received: by 2002:a2e:9cd3:0:b0:23e:53fd:a7e5 with SMTP id g19-20020a2e9cd3000000b0023e53fda7e5mr6418436ljj.475.1645205142357; Fri, 18 Feb 2022 09:25:42 -0800 (PST) MIME-Version: 1.0 References: <20220217094802.3644569-1-bigeasy@linutronix.de> <20220217094802.3644569-4-bigeasy@linutronix.de> In-Reply-To: <20220217094802.3644569-4-bigeasy@linutronix.de> From: Shakeel Butt Date: Fri, 18 Feb 2022 09:25:29 -0800 Message-ID: Subject: Re: [PATCH v3 3/5] mm/memcg: Protect per-CPU counter by disabling preemption on PREEMPT_RT where needed. To: Sebastian Andrzej Siewior Cc: Cgroups , Linux MM , Andrew Morton , Johannes Weiner , Michal Hocko , =?UTF-8?Q?Michal_Koutn=C3=BD?= , Peter Zijlstra , Thomas Gleixner , Vladimir Davydov , Waiman Long , Roman Gushchin Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 2A462C0003 X-Stat-Signature: pddntm4nmcu7tqy8wk5i35tkja3ug4w6 X-Rspam-User: Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=DQrnOr+7; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf10.hostedemail.com: domain of shakeelb@google.com designates 209.85.208.181 as permitted sender) smtp.mailfrom=shakeelb@google.com X-Rspamd-Server: rspam03 X-HE-Tag: 1645205143-780001 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 Thu, Feb 17, 2022 at 1:48 AM Sebastian Andrzej Siewior wrote: > > The per-CPU counter are modified with the non-atomic modifier. The > consistency is ensured by disabling interrupts for the update. > On non PREEMPT_RT configuration this works because acquiring a > spinlock_t typed lock with the _irq() suffix disables interrupts. On > PREEMPT_RT configurations the RMW operation can be interrupted. > > Another problem is that mem_cgroup_swapout() expects to be invoked with > disabled interrupts because the caller has to acquire a spinlock_t which > is acquired with disabled interrupts. Since spinlock_t never disables > interrupts on PREEMPT_RT the interrupts are never disabled at this > point. > > The code is never called from in_irq() context on PREEMPT_RT therefore > disabling preemption during the update is sufficient on PREEMPT_RT. > The sections which explicitly disable interrupts can remain on > PREEMPT_RT because the sections remain short and they don't involve > sleeping locks (memcg_check_events() is doing nothing on PREEMPT_RT). > > Disable preemption during update of the per-CPU variables which do not > explicitly disable interrupts. > > Signed-off-by: Sebastian Andrzej Siewior > Acked-by: Roman Gushchin > --- > mm/memcontrol.c | 29 ++++++++++++++++++++++++++++- > 1 file changed, 28 insertions(+), 1 deletion(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 0b5117ed2ae08..36ab3660f2c6d 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -630,6 +630,28 @@ static DEFINE_SPINLOCK(stats_flush_lock); > static DEFINE_PER_CPU(unsigned int, stats_updates); > static atomic_t stats_flush_threshold = ATOMIC_INIT(0); > > +/* > + * Accessors to ensure that preemption is disabled on PREEMPT_RT because it can > + * not rely on this as part of an acquired spinlock_t lock. These functions are > + * never used in hardirq context on PREEMPT_RT and therefore disabling preemtion > + * is sufficient. > + */ > +static void memcg_stats_lock(void) > +{ > +#ifdef CONFIG_PREEMPT_RT > + preempt_disable(); > +#else > + VM_BUG_ON(!irqs_disabled()); > +#endif > +} > + > +static void memcg_stats_unlock(void) > +{ > +#ifdef CONFIG_PREEMPT_RT > + preempt_enable(); > +#endif > +} > + > static inline void memcg_rstat_updated(struct mem_cgroup *memcg, int val) > { > unsigned int x; > @@ -706,6 +728,7 @@ void __mod_memcg_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx, > pn = container_of(lruvec, struct mem_cgroup_per_node, lruvec); > memcg = pn->memcg;) > > + memcg_stats_lock(); The call chains from rmap.c have not really disabled irqs. Actually there is a comment in do_page_add_anon_rmap() "We use the irq-unsafe __{inc|mod}_zone_page_stat because these counters are not modified in interrupt context, and pte lock(a spinlock) is held, which implies preemption disabled". VM_BUG_ON(!irqs_disabled()) within memcg_stats_lock() would be giving false error reports for CONFIG_PREEMPT_NONE kernels.