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 X-Spam-Level: X-Spam-Status: No, score=-2.0 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2DE53C17442 for ; Wed, 13 Nov 2019 13:45:09 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id E5B5822459 for ; Wed, 13 Nov 2019 13:45:08 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="eLUpbLpu" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E5B5822459 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 77EA26B0007; Wed, 13 Nov 2019 08:45:08 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 72F726B0008; Wed, 13 Nov 2019 08:45:08 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 61E9F6B000A; Wed, 13 Nov 2019 08:45:08 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0028.hostedemail.com [216.40.44.28]) by kanga.kvack.org (Postfix) with ESMTP id 4764F6B0007 for ; Wed, 13 Nov 2019 08:45:08 -0500 (EST) Received: from smtpin03.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with SMTP id E2926180AD804 for ; Wed, 13 Nov 2019 13:45:07 +0000 (UTC) X-FDA: 76151375454.03.crown38_66f494a90e842 X-HE-Tag: crown38_66f494a90e842 X-Filterd-Recvd-Size: 4878 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) by imf30.hostedemail.com (Postfix) with ESMTP for ; Wed, 13 Nov 2019 13:45:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20170209; h=In-Reply-To:Content-Transfer-Encoding :Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Sender:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help: List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=zRi6DLbBkEdEozkjZm98oVK/x7aADkM+iJVk1EnTFWk=; b=eLUpbLpu+7Mqv1XqZbOAstW1Xr s3S9M4qabuNrKeHjr+aMtQQTUqyJI+s3ZAcWxGGHoOLFIGkzrOpj7FV2i53sWElgDVbC8aR6ahCw4 RBdV5TnpKLElNuiyfEOEHdptZ1Paw4v6/YHhRda6W3KlU4ay0toajZfXkccooGszCUXu/yLxgAaC/ pFK9jKjJHT0Yqv57ZjYInT11Yq0q2yNowES80QxjYTVAK0ETrYGnpDUU1t5IkbFEWy6JC/g55VRZ8 Y+YCQRbkdtBuG2DTLd8Fj+26ap7yNVh1HvvGzY2S79eh+sc42JHFGRS0kewhOnmDsZICEEYfZx3N+ TuByAZvw==; Received: from willy by bombadil.infradead.org with local (Exim 4.92.3 #3 (Red Hat Linux)) id 1iUsxC-0003wY-TT; Wed, 13 Nov 2019 13:45:02 +0000 Date: Wed, 13 Nov 2019 05:45:02 -0800 From: Matthew Wilcox To: Alex Shi Cc: cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, akpm@linux-foundation.org, mgorman@techsingularity.net, tj@kernel.org, hughd@google.com, khlebnikov@yandex-team.ru, daniel.m.jordan@oracle.com, yang.shi@linux.alibaba.com, Johannes Weiner , Michal Hocko , Vladimir Davydov , Roman Gushchin , Shakeel Butt , Chris Down , Thomas Gleixner , Vlastimil Babka , Andrey Ryabinin , swkhack , "Potyra, Stefan" , Jason Gunthorpe , Mauro Carvalho Chehab , Peng Fan , Nikolay Borisov , Ira Weiny , Kirill Tkhai , Yafang Shao Subject: Re: [PATCH v2 4/8] mm/lru: only change the lru_lock iff page's lruvec is different Message-ID: <20191113134502.GD7934@bombadil.infradead.org> References: <1573567588-47048-1-git-send-email-alex.shi@linux.alibaba.com> <1573567588-47048-5-git-send-email-alex.shi@linux.alibaba.com> <20191112143624.GA7934@bombadil.infradead.org> <297ad71c-081c-f7e1-d640-8720a0eeeeba@linux.alibaba.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <297ad71c-081c-f7e1-d640-8720a0eeeeba@linux.alibaba.com> User-Agent: Mutt/1.12.1 (2019-06-15) Content-Transfer-Encoding: quoted-printable 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, Nov 13, 2019 at 10:26:24AM +0800, Alex Shi wrote: > =E5=9C=A8 2019/11/12 =E4=B8=8B=E5=8D=8810:36, Matthew Wilcox =E5=86=99=E9= =81=93: > > On Tue, Nov 12, 2019 at 10:06:24PM +0800, Alex Shi wrote: > >> +/* Don't lock again iff page's lruvec locked */ > >> +static inline struct lruvec *relock_page_lruvec_irq(struct page *pa= ge, > >> + struct lruvec *locked_lruvec) > >> +{ > >> + struct pglist_data *pgdat =3D page_pgdat(page); > >> + struct lruvec *lruvec; > >> + > >> + rcu_read_lock(); > >> + lruvec =3D mem_cgroup_page_lruvec(page, pgdat); > >> + > >> + if (locked_lruvec =3D=3D lruvec) { > >> + rcu_read_unlock(); > >> + return lruvec; > >> + } > >> + rcu_read_unlock(); > >=20 > > Why not simply: > >=20 > > rcu_read_lock(); > > lruvec =3D mem_cgroup_page_lruvec(page, pgdat); > > rcu_read_unlock(); > >=20 > > if (locked_lruvec =3D=3D lruvec) >=20 > The rcu_read_unlock here is for guarding the locked_lruvec/lruvec compa= rsion. > Otherwise memcg/lruvec maybe changed, like, from memcg migration etc. I= guess. How does holding the RCU lock guard the comparison? You're comparing two pointers for equality. Nothing any other CPU can do at this point will change the results of that comparison. > > Also, why are you bothering to re-enable interrupts here? Surely if > > you're holding lock A with interrupts disabled , you can just drop lo= ck A, > > acquire lock B and leave the interrupts alone. That way you only nee= d > > one of this variety of function, and not the separate irq/irqsave var= iants. > >=20 >=20 > Thanks for the suggestion! Yes, if only do re-lock, it's better to leav= e the irq unchanging. but, when the locked_lruvec is NULL, it become a fi= rst time lock which irq or irqsave are different. Thus, in combined funct= ion we need a nother parameter to indicate if it do irqsaving. So compari= ng to a extra/indistinct parameter, I guess 2 inline functions would be a= bit more simple/cleary?=20 Ah, right, I missed the "if it's not held" case.