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 38773C433F5 for ; Wed, 25 May 2022 15:38:39 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B113D8D0006; Wed, 25 May 2022 11:38:38 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A9CB98D0002; Wed, 25 May 2022 11:38:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 98A908D0006; Wed, 25 May 2022 11:38:38 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 8A05C8D0002 for ; Wed, 25 May 2022 11:38:38 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay11.hostedemail.com (Postfix) with ESMTP id 4E53E81134 for ; Wed, 25 May 2022 15:38:38 +0000 (UTC) X-FDA: 79504672716.21.46B5C3A Received: from mail-pf1-f182.google.com (mail-pf1-f182.google.com [209.85.210.182]) by imf13.hostedemail.com (Postfix) with ESMTP id 6986B20021 for ; Wed, 25 May 2022 15:38:08 +0000 (UTC) Received: by mail-pf1-f182.google.com with SMTP id f21so7192918pfa.3 for ; Wed, 25 May 2022 08:38:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance-com.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=XfUA65d3SdbF2oQUu55YnSpsWlaG2pXPKVSvY3AeaaY=; b=HdWmnFghAeH4LB9aqAeQKPfx4YaL1PQAnvmeL63qjEhQAsAKGPbUoWiw805DbWMH7w XoiBlH8sgL6Qd92qTjABYEtmtm8Cg+0qLN6NFvKknvRaO+mJDJqgzHyWB8HEZuWXscrH Xz1wzE6LmtCY4J5IB9STw0NM5YtUlc4+LnGaMH+uaVJ+9hl2W1D75vtJW+gstoCx0AkA d24A8qRFz6/7+LHkWFIXmI8WCtXovRtftQ/wkbSch57kOh30B0HA6YIDAZKaOqrc/HGo AI0GxMStontX+lP5N4q0iFkpJeCCyO1oQ5OEx/TB6KuwVerlzTV7dTM9Yp0K4fMtVoKp 48LA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=XfUA65d3SdbF2oQUu55YnSpsWlaG2pXPKVSvY3AeaaY=; b=7jNykuvaE+ORvoqAm+SqchvapTlV/3i52f0YCggGloTlTJWl0vbXJdLOrZkFHq+WWf MeEBv+0Lc46HZOHGYBmtQVNAT3J9XZzL2KVa55vgym6/LyTeFPSxJI6KgJ2vRKngZGFY MQ07DKv/yghdkhFGiIAS8ot66Cz04zcM66oqn0s4slb0ysoqQlj99cHKo9cUta38bQqk 7/9+KgrnKqAUJFMujoTYF3UyGxMz3GXY02bB6iStgGpfVQ99JkH7gEaZtlPAFpL/PtuR KRAaf94OxlOJXLNyTA48JCJ8bkpqSxIqk0QV7jUH2hLohwiwr/55cjygVuwtRn6arYX0 WJjQ== X-Gm-Message-State: AOAM533x/WTbaga3h55Y5zAJG+v4HpkCRruFGtgjHn3vJHyHHOEFfDJW js/JgXOP6CSwvG/X4SAyAKfdyg== X-Google-Smtp-Source: ABdhPJw8ma6FFkHhjBy1EOWCJFVdGQF0wo4kNYjbEkPmz677E+idKlyhbhGwlIzdJdTyXCoafh/Lkg== X-Received: by 2002:a63:1a17:0:b0:3fa:e901:1c68 with SMTP id a23-20020a631a17000000b003fae9011c68mr2278636pga.243.1653493114887; Wed, 25 May 2022 08:38:34 -0700 (PDT) Received: from localhost ([2408:8207:18da:2310:c40f:7b5:4fa8:df3f]) by smtp.gmail.com with ESMTPSA id t2-20020a170902e84200b001618fee3900sm9707878plg.196.2022.05.25.08.38.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 May 2022 08:38:34 -0700 (PDT) Date: Wed, 25 May 2022 23:38:30 +0800 From: Muchun Song To: Johannes Weiner Cc: mhocko@kernel.org, roman.gushchin@linux.dev, shakeelb@google.com, cgroups@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, duanxiongchun@bytedance.com, longman@redhat.com Subject: Re: [PATCH v4 03/11] mm: memcontrol: make lruvec lock safe when LRU pages are reparented Message-ID: References: <20220524060551.80037-1-songmuchun@bytedance.com> <20220524060551.80037-4-songmuchun@bytedance.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 6986B20021 X-Stat-Signature: 5pw9mgkeeimft5jif1beuuxqxq4xmmwz Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=bytedance-com.20210112.gappssmtp.com header.s=20210112 header.b=HdWmnFgh; spf=pass (imf13.hostedemail.com: domain of songmuchun@bytedance.com designates 209.85.210.182 as permitted sender) smtp.mailfrom=songmuchun@bytedance.com; dmarc=pass (policy=none) header.from=bytedance.com X-Rspam-User: X-HE-Tag: 1653493088-326295 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, May 25, 2022 at 10:48:54AM -0400, Johannes Weiner wrote: > On Wed, May 25, 2022 at 09:03:59PM +0800, Muchun Song wrote: > > On Wed, May 25, 2022 at 08:30:15AM -0400, Johannes Weiner wrote: > > > On Wed, May 25, 2022 at 05:53:30PM +0800, Muchun Song wrote: > > > > On Tue, May 24, 2022 at 03:27:20PM -0400, Johannes Weiner wrote: > > > > > On Tue, May 24, 2022 at 02:05:43PM +0800, Muchun Song wrote: > > > > > > @@ -1230,10 +1213,23 @@ void lruvec_memcg_debug(struct lruvec *lruvec, struct folio *folio) > > > > > > */ > > > > > > struct lruvec *folio_lruvec_lock(struct folio *folio) > > > > > > { > > > > > > - struct lruvec *lruvec = folio_lruvec(folio); > > > > > > + struct lruvec *lruvec; > > > > > > > > > > > > + rcu_read_lock(); > > > > > > +retry: > > > > > > + lruvec = folio_lruvec(folio); > > > > > > spin_lock(&lruvec->lru_lock); > > > > > > - lruvec_memcg_debug(lruvec, folio); > > > > > > + > > > > > > + if (unlikely(lruvec_memcg(lruvec) != folio_memcg(folio))) { > > > > > > + spin_unlock(&lruvec->lru_lock); > > > > > > + goto retry; > > > > > > + } > > > > > > + > > > > > > + /* > > > > > > + * Preemption is disabled in the internal of spin_lock, which can serve > > > > > > + * as RCU read-side critical sections. > > > > > > + */ > > > > > > + rcu_read_unlock(); > > > > > > > > > > The code looks right to me, but I don't understand the comment: why do > > > > > we care that the rcu read-side continues? With the lru_lock held, > > > > > reparenting is on hold and the lruvec cannot be rcu-freed anyway, no? > > > > > > > > > > > > > Right. We could hold rcu read lock until end of reparting. So you mean > > > > we do rcu_read_unlock in folio_lruvec_lock()? > > > > > > The comment seems to suggest that disabling preemption is what keeps > > > the lruvec alive. But it's the lru_lock that keeps it alive. The > > > cgroup destruction path tries to take the lru_lock long before it even > > > gets to synchronize_rcu(). Once you hold the lru_lock, having an > > > implied read-side critical section as well doesn't seem to matter. > > > > > > > Well, I thought that spinlocks have implicit read-side critical sections > > because it disables preemption (I learned from the comments above > > synchronize_rcu() that says interrupts, preemption, or softirqs have been > > disabled also serve as RCU read-side critical sections). So I have a > > question: is it still true in a PREEMPT_RT kernel (I am not familiar with > > this)? > > Yes, but you're missing my point. > > > > Should the comment be deleted? > > > > I think we could remove the comments. If the above question is false, seems > > like we should continue holding rcu read lock. > > It's true. > Thanks for your answer. > But assume it's false for a second. Why would you need to continue > holding it? What would it protect? The lruvec would be pinned by the > spinlock even if it DIDN'T imply an RCU lock, right? > > So I don't understand the point of the comment. If the implied RCU > lock is protecting something not covered by the bare spinlock itself, > it should be added to the comment. Otherwise, the comment should go. > Got it. Thanks for your nice explanation. I'll remove the comment here.