lkml.org 
[lkml]   [2017]   [Aug]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: + mm-madvise-fix-freeing-of-locked-page-with-madv_free.patch added to -mm tree
From
Date
On 08/25/2017 03:02 PM, Nadav Amit wrote:
> Michal Hocko <mhocko@kernel.org> wrote:
>
>> Hmm, I do not see this neither in linux-mm nor LKML. Strange
>>
>> On Wed 23-08-17 14:41:21, Andrew Morton wrote:
>>> From: Eric Biggers <ebiggers@google.com>
>>> Subject: mm/madvise.c: fix freeing of locked page with MADV_FREE
>>>
>>> If madvise(..., MADV_FREE) split a transparent hugepage, it called
>>> put_page() before unlock_page(). This was wrong because put_page() can
>>> free the page, e.g. if a concurrent madvise(..., MADV_DONTNEED) has
>>> removed it from the memory mapping. put_page() then rightfully complained
>>> about freeing a locked page.
>>>
>>> Fix this by moving the unlock_page() before put_page().
>
> Quick grep shows that a similar flow (put_page() followed by an
> unlock_page() ) also happens in hugetlbfs_fallocate(). Isn’t it a problem as
> well?

I assume you are asking about this block of code?

/*
* page_put due to reference from alloc_huge_page()
* unlock_page because locked by add_to_page_cache()
*/
put_page(page);
unlock_page(page);

Well, there is a typo (page_put) in the comment. :(

However, in this case we have just added the huge page to a hugetlbfs
file. The put_page() is there just to drop the reference count on the
page (taken when allocated). It will still be non-zero as we have
successfully added it to the page cache. So, we are not freeing the
page here, just dropping the reference count.

This should not cause a problem like that seen in madvise.
--
Mike Kravetz

\
 
 \ /
  Last update: 2017-08-26 00:31    [W:0.159 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site