On Sat, Dec 26, 2020 at 6:38 PM Hugh Dickins <hughd@google.com> wrote:
>
> This patch (like its antecedents) moves the pte_unmap_unlock() from
> after do_fault_around()'s "check if the page fault is solved" into
> filemap_map_pages() itself (which apparently does not NULLify vmf->pte
> after unmapping it, which is poor, but good for revealing this issue).
> That looks cleaner, but of course there was a very good reason for its
> original positioning.

Good catch.

> Maybe you want to change the ->map_pages prototype, to pass down the
> requested address too, so that it can report whether the requested
> address was resolved or not.  Or it could be left to __do_fault(),
> or even to a repeated fault; but those would be less efficient.

Let's keep the old really odd "let's unlock in the caller" for now,
and minimize the changes.

Adding a big big comment at the end of filemap_map_pages() to note the
odd delayed page table unlocking.

Here's an updated patch that combines Kirill's original patch, his
additional incremental patch, and the fix for the pte lock oddity into
one thing.

Does this finally pass your testing?

               Linus