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

[Linus Torvalds]

On Sat, Dec 26, 2020 at 1:04 PM Hugh Dickins <hughd@xxxxxxxxxx> 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