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

Linus Torvalds
2020-12-26T21:18:23Z

On Sat, Dec 26, 2020 at 1:04 PM Hugh Dickins <hughd@google.com> wrote:
>
>
> Hold on. I guess this one will suffer from the same bug as the previous.
> I was about to report back, after satisfactory overnight testing of that
> version - provided that one big little bug is fixed:
>
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -2919,7 +2919,7 @@ static bool filemap_map_pmd(struct vm_fa
>
> if (pmd_none(*vmf->pmd) &&
> PageTransHuge(page) &&
> - do_set_pmd(vmf, page)) {
> + do_set_pmd(vmf, page) == 0) {
> unlock_page(page);
> return true;
> }

I missed that entirely, because when just reading the patch it looks
fine and I didn't look at what do_set_pmd() function returns outside
the patch.

And maybe it would be better to write it as

if (pmd_none(*vmf->pmd) && PageTransHuge(page)) {
vm_fault_t ret = do_set_pmd(vmf, page);
if (!ret) {
...

instead to make it a bit more explicit about how that return value is
a vm_fault_t there...

And see my other email about how I suspect there is still a leak in
that patch for the previous test-case.

Linus