On Thu, Sep 02, 2021 at 04:17:16PM -0400, Peter Xu wrote:

[Based on tag v5.14, but it should still apply to -mm too.  If not, I can
 repost anytime]

Hugh,

So I found one thing that I feel like a bug of commit 22061a1ffabdb9c3, but I'm
not sure.  If that's the case, patch 5 of this series may be the fix for it.

The problem is unmap_mapping_page() in current tree is calling
unmap_mapping_range_tree() with a details pointer, while by default when detail
pointer is specified, it means "we want to skip zapping swap entries".

I didn't mention this in v1 simply because I thought it was fine, e.g., swap
entry won't be kept in shmem ptes so skipped is okay (it is never okay with
shmem uffd-wp but uffd-wp code is not landed yet).  However I just remembered
there could also be e.g. shmem migration entries if I'm not wrong.  From that
pov, skipping swap entries for unmap_mapping_page() seems wrong.  Would you
please help check?

I figured this seems to have no problem: firstly commit 22061a1ffabdb9c3 didn't
really change the behavior of the code because previously it was using
unmap_mapping_range(), which will always pass in a "details" pointer anyway.

Meanwhile there won't be migration entry for this page because the new helper
unmap_mapping_page() has page lock held, while migration requires that too
during moving the pages.

Anyway, sorry for the noise. I'll respin but drop these paragraphs, also in the
last patch's commit message.

-- 
Peter Xu