Messages in this thread |  | | Subject | Re: + mm-madvise-fix-freeing-of-locked-page-with-madv_free.patch added to -mm tree | From | Mike Kravetz <> | Date | Fri, 25 Aug 2017 15:31:22 -0700 |
| |
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
|  |