From: Shakeel Butt <shakeelb@google.com>
To: Alex Shi <alex.shi@linux.alibaba.com>
Cc: "Johannes Weiner" <hannes@cmpxchg.org>,
Cgroups <cgroups@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
"Linux MM" <linux-mm@kvack.org>,
"Andrew Morton" <akpm@linux-foundation.org>,
"Mel Gorman" <mgorman@techsingularity.net>,
"Tejun Heo" <tj@kernel.org>, "Hugh Dickins" <hughd@google.com>,
"Konstantin Khlebnikov" <khlebnikov@yandex-team.ru>,
"Daniel Jordan" <daniel.m.jordan@oracle.com>,
"Yang Shi" <yang.shi@linux.alibaba.com>,
"Matthew Wilcox" <willy@infradead.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 v4 3/9] mm/lru: replace pgdat lru_lock with lruvec lock
Date: Mon, 25 Nov 2019 09:27:25 -0800 [thread overview]
Message-ID: <CALvZod4ZKh3HbDWJz5-tD9Q0gcMUjWmqzBGUD-ejOLCoS7ga2w@mail.gmail.com> (raw)
In-Reply-To: <e629f595-df0a-71b2-35b4-b266ba1d0431@linux.alibaba.com>
On Mon, Nov 25, 2019 at 1:26 AM Alex Shi <alex.shi@linux.alibaba.com> wrote:
>
>
> >
> > But that leaves me with one more worry: compaction. We locked out
> > charge moving now, so between that and knowing that the page is alive,
> > we have page->mem_cgroup stable. But compaction doesn't know whether
> > the page is alive - it comes from a pfn and finds out using PageLRU.
> >
> > In the current code, pgdat->lru_lock remains the same before and after
> > the page is charged to a cgroup, so once compaction has that locked
> > and it observes PageLRU, it can go ahead and isolate the page.
> >
> > But lruvec->lru_lock changes during charging, and then compaction may
> > hold the wrong lock during isolation:
> >
> > compaction: generic_file_buffered_read:
> >
> > page_cache_alloc()
> >
> > !PageBuddy()
> >
> > lock_page_lruvec(page)
> > lruvec = mem_cgroup_page_lruvec()
> > spin_lock(&lruvec->lru_lock)
> > if lruvec != mem_cgroup_page_lruvec()
> > goto again
> >
> > add_to_page_cache_lru()
> > mem_cgroup_commit_charge()
> > page->mem_cgroup = foo
> > lru_cache_add()
> > __pagevec_lru_add()
> > SetPageLRU()
> >
> > if PageLRU(page):
> > __isolate_lru_page()
> >
> > I don't see what prevents the lruvec from changing under compaction,
> > neither in your patches nor in Hugh's. Maybe I'm missing something?
> >
>
> Hi Johannes,
>
> It looks my patch do the lruvec recheck/relock after PageLRU in compaction.c.
> It should be fine for your question. So I will try more testing after all changes.
Actually no, unless PageLRU check and taking lruvec lock are atomic,
the race mentioned by Johannes still exist.
Shakeel
next prev parent reply other threads:[~2019-11-25 17:27 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-19 12:23 [PATCH v4 0/9] per lruvec lru_lock for memcg Alex Shi
2019-11-19 12:23 ` [PATCH v4 1/9] mm/swap: fix uninitialized compiler warning Alex Shi
2019-11-19 15:41 ` Johannes Weiner
2019-11-20 11:42 ` Alex Shi
2019-11-19 12:23 ` [PATCH v4 2/9] mm/huge_memory: " Alex Shi
2019-11-19 15:42 ` Johannes Weiner
2019-11-19 12:23 ` [PATCH v4 3/9] mm/lru: replace pgdat lru_lock with lruvec lock Alex Shi
2019-11-19 15:57 ` Matthew Wilcox
2019-11-19 16:44 ` Johannes Weiner
2019-11-20 11:50 ` Alex Shi
2019-11-19 16:04 ` Johannes Weiner
2019-11-20 11:41 ` Alex Shi
2019-11-21 22:06 ` Johannes Weiner
2019-11-22 2:36 ` Alex Shi
2019-11-22 16:16 ` Johannes Weiner
2019-11-23 0:58 ` Hugh Dickins
2019-11-24 15:19 ` Alex Shi
2019-11-25 9:26 ` Alex Shi
2019-11-25 17:27 ` Shakeel Butt [this message]
2019-11-19 16:49 ` Shakeel Butt
2019-11-19 12:23 ` [PATCH v4 4/9] mm/mlock: only change the lru_lock iff page's lruvec is different Alex Shi
2019-11-19 12:23 ` [PATCH v4 5/9] mm/swap: " Alex Shi
2019-11-19 12:23 ` [PATCH v4 6/9] mm/vmscan: " Alex Shi
2019-11-19 12:23 ` [PATCH v4 7/9] mm/pgdat: remove pgdat lru_lock Alex Shi
2019-11-19 12:23 ` [PATCH v4 8/9] mm/lru: likely enhancement Alex Shi
2019-11-19 12:23 ` [PATCH v4 9/9] mm/lru: revise the comments of lru_lock Alex Shi
2019-11-19 16:19 ` Johannes Weiner
2019-11-20 11:48 ` Alex Shi
2019-11-24 15:49 ` [PATCH v4 0/9] per lruvec lru_lock for memcg Konstantin Khlebnikov
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=CALvZod4ZKh3HbDWJz5-tD9Q0gcMUjWmqzBGUD-ejOLCoS7ga2w@mail.gmail.com \
--to=shakeelb@google.com \
--cc=Stefan.Potyra@elektrobit.com \
--cc=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=alex.shi@linux.alibaba.com \
--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=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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).