Messages in this thread | | | Date | Wed, 22 Jan 2020 13:31:13 -0500 | From | Johannes Weiner <> | Subject | Re: [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)
| |