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
