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 812D1C433EF for ; Mon, 21 Feb 2022 11:31:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 013818D0002; Mon, 21 Feb 2022 06:31:22 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id F05098D0001; Mon, 21 Feb 2022 06:31:21 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DCD348D0002; Mon, 21 Feb 2022 06:31:21 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0247.hostedemail.com [216.40.44.247]) by kanga.kvack.org (Postfix) with ESMTP id CECF28D0001 for ; Mon, 21 Feb 2022 06:31:21 -0500 (EST) Received: from smtpin30.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 8D3DE998B4 for ; Mon, 21 Feb 2022 11:31:21 +0000 (UTC) X-FDA: 79166571162.30.6A0AD88 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by imf23.hostedemail.com (Postfix) with ESMTP id 047D2140010 for ; Mon, 21 Feb 2022 11:31:20 +0000 (UTC) Date: Mon, 21 Feb 2022 12:31:17 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1645443078; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=RulSMPiCVHplqpEdAg1RSNzopKMgaNgDwROyd5kzhU4=; b=ygUsaiTcDR+YlYfCOVWfPbfQf/So/3FCW/1vcZ1r6O/gZ2uUd2cOkrbTYu7EXiVVCRtZB9 9M5gS95vfitEMWKspuml850mok/MtwaGbTmnGPH4KXpZ//JvGcSHviDSp0eNDzMPY+cgle 0G/uKP2CehjErGJMA6QkgawDKeQjXz8xuHYr30BOOZVmk1M3Ux4X6cK5nrhWHplRcNBhWz xoL0OnZqhIJvgUL/5J0D0ns6PohpEQ0vJS9wpvg941pIglsRnIxYIWNekdXaMM0L75qO29 HP7MSCOivDHhNpSxDsPBeJJU2z5vxRscUEexNQo2/BjDO7F5CQ8cUxx/PiQD6g== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1645443078; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=RulSMPiCVHplqpEdAg1RSNzopKMgaNgDwROyd5kzhU4=; b=JYXtgNXu15YB09O4e1vbfUTzL3s4Xp30WcFxpRTR4AKtOAYP5bVC0eTdATimWakaj2Wcw0 op4C0mo+TYJGeKDw== From: Sebastian Andrzej Siewior To: Shakeel Butt Cc: Cgroups , Linux MM , Andrew Morton , Johannes Weiner , Michal Hocko , Michal =?utf-8?Q?Koutn=C3=BD?= , Peter Zijlstra , Thomas Gleixner , Vladimir Davydov , Waiman Long , Roman Gushchin Subject: Re: [PATCH v3 3/5] mm/memcg: Protect per-CPU counter by disabling preemption on PREEMPT_RT where needed. Message-ID: References: <20220217094802.3644569-1-bigeasy@linutronix.de> <20220217094802.3644569-4-bigeasy@linutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 047D2140010 X-Stat-Signature: z4aq6a5udjq8wk6iik8bg97qbd3tjdhh X-Rspam-User: Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=linutronix.de header.s=2020 header.b=ygUsaiTc; dkim=pass header.d=linutronix.de header.s=2020e header.b=JYXtgNXu; spf=pass (imf23.hostedemail.com: domain of bigeasy@linutronix.de designates 193.142.43.55 as permitted sender) smtp.mailfrom=bigeasy@linutronix.de; dmarc=pass (policy=none) header.from=linutronix.de X-Rspamd-Server: rspam05 X-HE-Tag: 1645443080-394470 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 2022-02-18 09:25:29 [-0800], Shakeel Butt wrote: > > 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. So three caller, including do_page_add_anon_rmap(): __mod_lruvec_page_state() -> __mod_lruvec_state() -> __mod_memcg_lruvec_state() is affected. Here we get false warnings because interrupts may not be disabled and it is intended. Hmmm. The odd part is that this only affects certain idx so any kind of additional debugging would need to take this into account. What about memcg_rstat_updated()? It does: | x = __this_cpu_add_return(stats_updates, abs(val)); | if (x > MEMCG_CHARGE_BATCH) { | atomic_add(x / MEMCG_CHARGE_BATCH, &stats_flush_threshold); | __this_cpu_write(stats_updates, 0); | } The writes to stats_updates can happen from IRQ-context and with disabled preemption only. So this is not good, right? Sebastian