Messages in this thread | | | From | Linus Torvalds <> | Date | Sat, 26 Dec 2020 13:07:14 -0800 | Subject | Re: [PATCH 1/2] mm: Allow architectures to request 'old' entries when prefaulting |
| |
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
| |