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. -- Peter Xu
