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

[Linus Torvalds]

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@xxxxxxxxxxxxx> 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