Re: [PATCH 1/2] mm: Allow architectures to request 'old' entries when prefaulting

Linus Torvalds
2020-12-26T21:09:25Z

I was going to just apply this patch, because I like it so much, but
then I decided to take one last look, and:

On Sat, Dec 26, 2020 at 12:43 PM Kirill A. Shutemov
<kirill@shutemov.name> wrote:
>
> +static bool filemap_map_pmd(struct vm_fault *vmf, struct page *page)
> +{
> + struct mm_struct *mm = vmf->vma->vm_mm;
> +
> + /* Huge page is mapped? No need to proceed. */
> + if (pmd_trans_huge(*vmf->pmd))
> + return true;

doesn't this cause us to leak a locked page?

I get the feeling that every single "return true" case here should
always unlock the page and - with the exception of a successful
do_set_pmd() - do a "put_page()".

Which kind of argues that we should just do it in the caller (and get
an extra ref in the do_set_pmd() case, so that the caller can always
do

if (filemap_map_pmd(..)) {
unlock_page(page);
put_page(page);
rcu_read_unlock();
return;
}

andf then there are no odd cases inside that filemap_map_pmd() function. Hmm?

Other than that, I really find it all much more legible.

Of course, if I'm wrong about the above, that just proves that I'm
missing something and it wasn't so legible after all..

Linus