On Wed, Sep 27, 2023 at 11:08 PM Suren Baghdasaryan surenb@google.com wrote:
On Wed, Sep 27, 2023 at 1:42 PM Suren Baghdasaryan surenb@google.com wrote:
On Wed, Sep 27, 2023 at 1:04 PM Jann Horn jannh@google.com wrote:
On Wed, Sep 27, 2023 at 8:08 PM Suren Baghdasaryan surenb@google.com wrote:
On Wed, Sep 27, 2023 at 5:47 AM Jann Horn jannh@google.com wrote:
On Sat, Sep 23, 2023 at 3:31 AM Suren Baghdasaryan surenb@google.com wrote:
dst_pmdval = pmdp_get_lockless(dst_pmd);/** If the dst_pmd is mapped as THP don't override it and just* be strict. If dst_pmd changes into TPH after this check, the* remap_pages_huge_pmd() will detect the change and retry* while remap_pages_pte() will detect the change and fail.*/if (unlikely(pmd_trans_huge(dst_pmdval))) {err = -EEXIST;break;}ptl = pmd_trans_huge_lock(src_pmd, src_vma);if (ptl && !pmd_trans_huge(*src_pmd)) {spin_unlock(ptl);ptl = NULL;}This still looks wrong - we do still have to split_huge_pmd() somewhere so that remap_pages_pte() works.
Hmm, I guess this extra check is not even needed...
Hm, and instead we'd bail at the pte_offset_map_nolock() in remap_pages_pte()? I guess that's unusual but works...
Yes, that's what I was thinking but I agree, that seems fragile. Maybe just bail out early if (ptl && !pmd_trans_huge())?
No, actually we can still handle is_swap_pmd() case by splitting it and remapping the individual ptes. So, I can bail out only in case of pmd_devmap().
FWIW I only learned today that "real" swap PMDs don't actually exist - only migration entries, which are encoded as swap PMDs, exist. You can see that when you look through the cases that something like __split_huge_pmd() or zap_pmd_range() actually handles.
So I think if you wanted to handle all the PMD types properly here without splitting, you could do that without _too_ much extra code. But idk if it's worth it.