Messages in this thread | | | Subject | Re: mm: pages are not freed from lru_add_pvecs after process termination | From | Dave Hansen <> | Date | Fri, 6 May 2016 09:04:34 -0700 |
| |
On 05/06/2016 08:10 AM, Odzioba, Lukasz wrote: > On Thu 05-05-16 09:21:00, Michal Hocko wrote: >> Or maybe the async nature of flushing turns >> out to be just impractical and unreliable and we will end up skipping >> THP (or all compound pages) for pcp LRU add cache. Let's see... > > What if we simply skip lru_add pvecs for compound pages? > That way we still have compound pages on LRU's, but the problem goes > away. It is not quite what this naïve patch does, but it works nice for me. > > diff --git a/mm/swap.c b/mm/swap.c > index 03aacbc..c75d5e1 100644 > --- a/mm/swap.c > +++ b/mm/swap.c > @@ -392,7 +392,9 @@ static void __lru_cache_add(struct page *page) > get_page(page); > if (!pagevec_space(pvec)) > __pagevec_lru_add(pvec); > pagevec_add(pvec, page); > + if (PageCompound(page)) > + __pagevec_lru_add(pvec); > put_cpu_var(lru_add_pvec); > }
That's not _quite_ what I had in mind since that drains the entire pvec every time a large page is encountered. But I'm conflicted about what the right behavior _is_.
We'd taking the LRU lock for 'page' anyway, so we might as well drain the pvec.
Or, does the additional work to put the page on to a pvec and then immediately drain it overwhelm that advantage?
Or does it just not matter?
Kirill, do you have a suggestion for how we should be checking for THP pages in code like this? PageCompound() will surely _work_ for anon-THP and your file-THP, but is it the best way to check?
> Do we have any tests that I could use to measure performance impact > of such changes before I start to tweak it up? Or maybe it doesn't make > sense at all ?
You probably want to very carefully calculate the time to fault a page, then separately to free a page. If we can't manage to detect a delta on a little microbenchmark like that then we'll probably never see one in practice.
You'll want to measure the fault time for a 4k pages, 2M pages, and then possibly a mix.
You'll want to do this in a highly parallel test to make sure any additional LRU lock overhead shows up.
| |