Re: [PATCH] mm/uffd: Always wr-protect pte in pte|pmd_mkuffd_wp()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 14.12.22 15:26, Peter Xu wrote:
On Wed, Dec 14, 2022 at 11:59:35AM +0100, David Hildenbrand wrote:
On 08.12.22 20:46, Peter Xu wrote:
This patch is a cleanup to always wr-protect pte/pmd in mkuffd_wp paths.

The reasons I still think this patch is worthwhile, are:

    (1) It is a cleanup already; diffstat tells.

    (2) It just feels natural after I thought about this, if the pte is uffd
        protected, let's remove the write bit no matter what it was.

    (2) Since x86 is the only arch that supports uffd-wp, it also redefines
        pte|pmd_mkuffd_wp() in that it should always contain removals of
        write bits.  It means any future arch that want to implement uffd-wp
        should naturally follow this rule too.  It's good to make it a
        default, even if with vm_page_prot changes on VM_UFFD_WP.

    (3) It covers more than vm_page_prot.  So no chance of any potential
        future "accident" (like pte_mkdirty() sparc64 or loongarch, even
        though it just got its pte_mkdirty fixed <1 month ago).  It'll be
        fairly clear when reading the code too that we don't worry anything
        before a pte_mkuffd_wp() on uncertainty of the write bit.

Don't necessarily agree with (3). If you'd have a broken pte_mkdirty() and
do the pte_mkdirty() after pte_mkuffd_wp() it would still be broken. Because
sparc64 and loongarch are simply broken.

That's why I mentioned on the order of operations matters.



We may call pte_wrprotect() one more time in some paths (e.g. thp split),
but that should be fully local bitop instruction so the overhead should be
negligible.

Although this patch should logically also fix all the known issues on
uffd-wp too recently on either page migration or numa balancing, but this
is not the plan for that fix.  So no fixes, and stable doesn't need this.

I don't see how this would fix do_numa_page(), where we only do a
pte_modify().

Yes, this patch won't, because it's a pure cleanup.  Otherwise we need
another line of wr-protect in numa recover path.

I can remove that sentence in v2 commit log.

Feel free to add my

Acked-by: David Hildenbrand <david@xxxxxxxxxx>

Nothing jumped at me.

--
Thanks,

David / dhildenb





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux