[prev in list] [next in list] [prev in thread] [next in thread]
List: linux-mm
Subject: Re: page cache: Store only head pages in i_pages
From: Matthew Wilcox <willy () infradead ! org>
Date: 2019-03-31 3:23:26
Message-ID: 20190331032326.GA10344 () bombadil ! infradead ! org
[Download RAW message or body]
On Sat, Mar 30, 2019 at 07:10:52AM -0700, Matthew Wilcox wrote:
> On Fri, Mar 29, 2019 at 08:04:32PM -0700, Matthew Wilcox wrote:
> > Excellent! I'm not comfortable with the rule that you have to be holding
> > the i_pages lock in order to call find_get_page() on a swap address_space.
> > How does this look to the various smart people who know far more about the
> > MM than I do?
> >
> > The idea is to ensure that if this race does happen, the page will be
> > handled the same way as a pagecache page. If __delete_from_swap_cache()
> > can be called while the page is still part of a VMA, then this patch
> > will break page_to_pgoff(). But I don't think that can happen ... ?
>
> Oh, blah, that can totally happen. reuse_swap_page() calls
> delete_from_swap_cache(). Need a new plan.
I don't see a good solution here that doesn't involve withdrawing this
patch and starting over. Bad solutions:
- Take the i_pages lock around each page lookup call in the swap code
(not just the one you found; there are others like mc_handle_swap_pte()
in memcontrol.c)
- Call synchronize_rcu() in __delete_from_swap_cache()
- Swap the roles of ->index and ->private for swap pages, and then don't
clear ->index when deleting a page from the swap cache
The first two would be slow and non-scalable. The third is still prone
to a race where the page is looked up on one CPU, while another CPU
removes it from one swap file then moves it to a different location,
potentially in a different swap file. Hard to hit, but not a race we
want to introduce.
I believe that the swap code actually never wants to see subpages. So if
we start again, introducing APIs (eg find_get_head()) which return the
head page, then convert the swap code over to use those APIs, we don't
need to solve the problem of finding the subpage of a swap page while
not holding the page lock.
I'm obviously reluctant to withdraw the patch, but I don't see a better
option. Your testing has revealed a problem that needs a deeper solution
than just adding a fix patch.
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic