linux-kernel - Re: mm: pages are not freed from lru_add

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 6 May 2016 09:04:34 -0700
From:	Dave Hansen <dave.hansen@...el.com>
To:	"Odzioba, Lukasz" <lukasz.odzioba@...el.com>,
	Michal Hocko <mhocko@...nel.org>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	"Shutemov, Kirill" <kirill.shutemov@...el.com>,
	"Anaczkowski, Lukasz" <lukasz.anaczkowski@...el.com>,
	"Shutemov, Kirill" <kirill.shutemov@...el.com>
Subject: Re: mm: pages are not freed from lru_add_pvecs after process
 termination

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.