From: Alex Shi <alex.shi@linux.alibaba.com> To: Matthew Wilcox <willy@infradead.org> 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, shakeelb@google.com, hannes@cmpxchg.org, "Michal Hocko" <mhocko@kernel.org>, "Vladimir Davydov" <vdavydov.dev@gmail.com>, "Roman Gushchin" <guro@fb.com>, "Chris Down" <chris@chrisdown.name>, "Thomas Gleixner" <tglx@linutronix.de>, "Vlastimil Babka" <vbabka@suse.cz>, "Qian Cai" <cai@lca.pw>, "Andrey Ryabinin" <aryabinin@virtuozzo.com>, "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>, "Jérôme Glisse" <jglisse@redhat.com>, "Andrea Arcangeli" <aarcange@redhat.com>, "David Rientjes" <rientjes@google.com>, "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>, swkhack <swkhack@gmail.com>, "Potyra, Stefan" <Stefan.Potyra@elektrobit.com>, "Mike Rapoport" <rppt@linux.vnet.ibm.com>, "Stephen Rothwell" <sfr@canb.auug.org.au>, "Colin Ian King" <colin.king@canonical.com>, "Jason Gunthorpe" <jgg@ziepe.ca>, "Mauro Carvalho Chehab" <mchehab+samsung@kernel.org>, "Peng Fan" <peng.fan@nxp.com>, "Nikolay Borisov" <nborisov@suse.com>, "Ira Weiny" <ira.weiny@intel.com>, "Kirill Tkhai" <ktkhai@virtuozzo.com>, "Yafang Shao" <laoar.shao@gmail.com> Subject: Re: [PATCH v5 2/8] mm/lru: replace pgdat lru_lock with lruvec lock Date: Wed, 11 Dec 2019 11:51:47 +0800 [thread overview] Message-ID: <b86dc335-fad6-01a1-77ba-3d2a9d5c4c22@linux.alibaba.com> (raw) In-Reply-To: <20191210134133.GI32169@bombadil.infradead.org> 在 2019/12/10 下午9:41, Matthew Wilcox 写道: > On Tue, Dec 10, 2019 at 07:46:18PM +0800, Alex Shi wrote: >> -static void lock_page_lru(struct page *page, int *isolated) >> +static struct lruvec *lock_page_lru(struct page *page, int *isolated) >> { >> - pg_data_t *pgdat = page_pgdat(page); >> + struct lruvec *lruvec = lock_page_lruvec_irq(page); >> >> - spin_lock_irq(&pgdat->lru_lock); >> if (PageLRU(page)) { >> - struct lruvec *lruvec; >> >> - lruvec = mem_cgroup_page_lruvec(page, pgdat); >> ClearPageLRU(page); >> del_page_from_lru_list(page, lruvec, page_lru(page)); >> *isolated = 1; >> } else >> *isolated = 0; >> + >> + return lruvec; >> } > > I still don't understand how this is supposed to work for !PageLRU > pages. Which lruvec have you locked if this page isn't on an LRU? > Good question. We could just fold it under PageLRU and no meaning changes. Is this better to has this patch? Thanks Alex commit 0f4b66d4a42397d57638352b738c3f9658003e44 Author: Alex Shi <alex.shi@linux.alibaba.com> Date: Wed Dec 11 11:31:53 2019 +0800 mm/memcg: fold lock in lock_page_lru According to the calling path of commit_charge, the lrucare is bound with PageLRU, so we could just fold it under PageLRU. This has no functional change. Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com> Cc: Johannes Weiner <hannes@cmpxchg.org> Cc: Michal Hocko <mhocko@kernel.org> Cc: Vladimir Davydov <vdavydov.dev@gmail.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: cgroups@vger.kernel.org Cc: linux-mm@kvack.org Cc: linux-kernel@vger.kernel.org diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 833df0ce1bc1..4fe2252cf437 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -2622,9 +2622,10 @@ static void cancel_charge(struct mem_cgroup *memcg, unsigned int nr_pages) static struct lruvec *lock_page_lru(struct page *page, int *isolated) { - struct lruvec *lruvec = lock_page_lruvec_irq(page); + struct lruvec *lruvec = NULL; if (PageLRU(page)) { + lruvec = lock_page_lruvec_irq(page); ClearPageLRU(page); del_page_from_lru_list(page, lruvec, page_lru(page)); @@ -2638,17 +2639,18 @@ static struct lruvec *lock_page_lru(struct page *page, int *isolated) static void unlock_page_lru(struct page *page, int isolated, struct lruvec *locked_lruvec) { - struct lruvec *lruvec; + if (isolated) { + struct lruvec *lruvec; - unlock_page_lruvec_irq(locked_lruvec); - lruvec = lock_page_lruvec_irq(page); + if (locked_lruvec) + unlock_page_lruvec_irq(locked_lruvec); + lruvec = lock_page_lruvec_irq(page); - if (isolated) { VM_BUG_ON_PAGE(PageLRU(page), page); SetPageLRU(page); add_page_to_lru_list(page, lruvec, page_lru(page)); + unlock_page_lruvec_irq(lruvec); } - unlock_page_lruvec_irq(lruvec); } static void commit_charge(struct page *page, struct mem_cgroup *memcg,
WARNING: multiple messages have this Message-ID (diff)
From: Alex Shi <alex.shi@linux.alibaba.com> To: Matthew Wilcox <willy@infradead.org> 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, shakeelb@google.com, hannes@cmpxchg.org, "Michal Hocko" <mhocko@kernel.org>, "Vladimir Davydov" <vdavydov.dev@gmail.com>, "Roman Gushchin" <guro@fb.com>, "Chris Down" <chris@chrisdown.name>, "Thomas Gleixner" <tglx@linutronix.de>, "Vlastimil Babka" <vbabka@suse.cz>, "Qian Cai" <cai@lca.pw>, "Andrey Ryabinin" <aryabinin@virtuozzo.com>, "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>, "Jérôme Glisse" <jglisse@redhat.com>, "Andrea Arcangeli" <aarcange@redhat.com>, "David Rientjes" <rientjes@google.com> Subject: Re: [PATCH v5 2/8] mm/lru: replace pgdat lru_lock with lruvec lock Date: Wed, 11 Dec 2019 11:51:47 +0800 [thread overview] Message-ID: <b86dc335-fad6-01a1-77ba-3d2a9d5c4c22@linux.alibaba.com> (raw) In-Reply-To: <20191210134133.GI32169@bombadil.infradead.org> ÔÚ 2019/12/10 ÏÂÎç9:41, Matthew Wilcox дµÀ: > On Tue, Dec 10, 2019 at 07:46:18PM +0800, Alex Shi wrote: >> -static void lock_page_lru(struct page *page, int *isolated) >> +static struct lruvec *lock_page_lru(struct page *page, int *isolated) >> { >> - pg_data_t *pgdat = page_pgdat(page); >> + struct lruvec *lruvec = lock_page_lruvec_irq(page); >> >> - spin_lock_irq(&pgdat->lru_lock); >> if (PageLRU(page)) { >> - struct lruvec *lruvec; >> >> - lruvec = mem_cgroup_page_lruvec(page, pgdat); >> ClearPageLRU(page); >> del_page_from_lru_list(page, lruvec, page_lru(page)); >> *isolated = 1; >> } else >> *isolated = 0; >> + >> + return lruvec; >> } > > I still don't understand how this is supposed to work for !PageLRU > pages. Which lruvec have you locked if this page isn't on an LRU? > Good question. We could just fold it under PageLRU and no meaning changes. Is this better to has this patch? Thanks Alex commit 0f4b66d4a42397d57638352b738c3f9658003e44 Author: Alex Shi <alex.shi@linux.alibaba.com> Date: Wed Dec 11 11:31:53 2019 +0800 mm/memcg: fold lock in lock_page_lru According to the calling path of commit_charge, the lrucare is bound with PageLRU, so we could just fold it under PageLRU. This has no functional change. Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com> Cc: Johannes Weiner <hannes@cmpxchg.org> Cc: Michal Hocko <mhocko@kernel.org> Cc: Vladimir Davydov <vdavydov.dev@gmail.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: cgroups@vger.kernel.org Cc: linux-mm@kvack.org Cc: linux-kernel@vger.kernel.org diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 833df0ce1bc1..4fe2252cf437 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -2622,9 +2622,10 @@ static void cancel_charge(struct mem_cgroup *memcg, unsigned int nr_pages) static struct lruvec *lock_page_lru(struct page *page, int *isolated) { - struct lruvec *lruvec = lock_page_lruvec_irq(page); + struct lruvec *lruvec = NULL; if (PageLRU(page)) { + lruvec = lock_page_lruvec_irq(page); ClearPageLRU(page); del_page_from_lru_list(page, lruvec, page_lru(page)); @@ -2638,17 +2639,18 @@ static struct lruvec *lock_page_lru(struct page *page, int *isolated) static void unlock_page_lru(struct page *page, int isolated, struct lruvec *locked_lruvec) { - struct lruvec *lruvec; + if (isolated) { + struct lruvec *lruvec; - unlock_page_lruvec_irq(locked_lruvec); - lruvec = lock_page_lruvec_irq(page); + if (locked_lruvec) + unlock_page_lruvec_irq(locked_lruvec); + lruvec = lock_page_lruvec_irq(page); - if (isolated) { VM_BUG_ON_PAGE(PageLRU(page), page); SetPageLRU(page); add_page_to_lru_list(page, lruvec, page_lru(page)); + unlock_page_lruvec_irq(lruvec); } - unlock_page_lruvec_irq(lruvec); } static void commit_charge(struct page *page, struct mem_cgroup *memcg,
next prev parent reply other threads:[~2019-12-11 3:52 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-12-10 11:46 [PATCH v5 0/8] per lruvec lru_lock for memcg Alex Shi 2019-12-10 11:46 ` [PATCH v5 1/8] mm/vmscan: remove unnecessary lruvec adding Alex Shi 2019-12-10 11:46 ` [PATCH v5 2/8] mm/lru: replace pgdat lru_lock with lruvec lock Alex Shi 2019-12-10 11:46 ` Alex Shi 2019-12-10 13:41 ` Matthew Wilcox 2019-12-10 13:41 ` Matthew Wilcox 2019-12-11 3:51 ` Alex Shi [this message] 2019-12-11 3:51 ` Alex Shi 2019-12-10 11:46 ` [PATCH v5 3/8] mm/lru: introduce the relock_page_lruvec function Alex Shi 2019-12-10 11:46 ` [PATCH v5 4/8] mm/mlock: optimize munlock_pagevec by relocking Alex Shi 2019-12-10 11:46 ` [PATCH v5 5/8] mm/swap: only change the lru_lock iff page's lruvec is different Alex Shi 2019-12-10 11:46 ` [PATCH v5 6/8] mm/pgdat: remove pgdat lru_lock Alex Shi 2019-12-10 11:46 ` [PATCH v5 7/8] mm/lru: revise the comments of lru_lock Alex Shi 2019-12-10 11:46 ` Alex Shi 2019-12-10 11:46 ` [PATCH v5 8/8] mm/lru: debug checking for page memcg moving and lock_page_memcg Alex Shi
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=b86dc335-fad6-01a1-77ba-3d2a9d5c4c22@linux.alibaba.com \ --to=alex.shi@linux.alibaba.com \ --cc=Stefan.Potyra@elektrobit.com \ --cc=aarcange@redhat.com \ --cc=akpm@linux-foundation.org \ --cc=aneesh.kumar@linux.ibm.com \ --cc=aryabinin@virtuozzo.com \ --cc=cai@lca.pw \ --cc=cgroups@vger.kernel.org \ --cc=chris@chrisdown.name \ --cc=colin.king@canonical.com \ --cc=daniel.m.jordan@oracle.com \ --cc=guro@fb.com \ --cc=hannes@cmpxchg.org \ --cc=hughd@google.com \ --cc=ira.weiny@intel.com \ --cc=jgg@ziepe.ca \ --cc=jglisse@redhat.com \ --cc=khlebnikov@yandex-team.ru \ --cc=kirill.shutemov@linux.intel.com \ --cc=ktkhai@virtuozzo.com \ --cc=laoar.shao@gmail.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mchehab+samsung@kernel.org \ --cc=mgorman@techsingularity.net \ --cc=mhocko@kernel.org \ --cc=nborisov@suse.com \ --cc=peng.fan@nxp.com \ --cc=rientjes@google.com \ --cc=rppt@linux.vnet.ibm.com \ --cc=sfr@canb.auug.org.au \ --cc=shakeelb@google.com \ --cc=swkhack@gmail.com \ --cc=tglx@linutronix.de \ --cc=tj@kernel.org \ --cc=vbabka@suse.cz \ --cc=vdavydov.dev@gmail.com \ --cc=willy@infradead.org \ --cc=yang.shi@linux.alibaba.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.