lkml.org 
[lkml]   [2020]   [Jan]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v8 03/10] mm/lru: replace pgdat lru_lock with lruvec lock
On Wed, Jan 22, 2020 at 08:01:29PM +0800, Alex Shi wrote:
> Yes I understand isolatation would exclusive by PageLRU, but forgive my
> stupid, I didn't figure out how a new page lruvec adding could be blocked.

I don't see why we would need this. Can you elaborate where you think
this is a problem?

If compaction races with charging for example, compaction doesn't need
to prevent a new page from being added to an lruvec. PageLRU is only
set after page->mem_cgroup is updated, so there are two race outcomes:

1) TestClearPageLRU() fails. That means the page isn't (fully) created
yet and cannot be migrated. We goto isolate_fail before even trying to
lock the lruvec.

2) TestClearPageLRU() succeeds. That means the page was fully created
and page->mem_cgroup has been set up. Anybody who now wants to change
page->mem_cgroup needs PageLRU, but we have it, so lruvec is stable.

I.e. cgroup charging does this:

page->mem_cgroup = new_group

lock(pgdat->lru_lock)
SetPageLRU()
add_page_to_lru_list()
unlock(pgdat->lru_lock)

and compaction currently does this:

lock(pgdat->lru_lock)
if (!PageLRU())
goto isolate_fail
// __isolate_lru_page:
if (!get_page_unless_zero())
goto isolate_fail
ClearPageLRU()
del_page_from_lru_list()
unlock(pgdat->lru_lock)

We can replace charging with this:

page->mem_cgroup = new_group

lock(lruvec->lru_lock)
add_page_to_lru_list()
unlock(lruvec->lru_lock)

SetPageLRU()

and the compaction sequence with something like this:

if (!get_page_unless_zero())
goto isolate_fail

if (!TestClearPageLRU())
goto isolate_fail_put

// We got PageLRU, so charging is complete and nobody
// can modify page->mem_cgroup until we set it again.

lruvec = mem_cgroup_page_lruvec(page, pgdat)
lock(lruvec->lru_lock)
del_page_from_lru_list()
unlock(lruvec->lru_lock)

\
 
 \ /
  Last update: 2020-01-22 19:33    [W:0.065 / U:1.956 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site