I consider UFFD-wp a special case: while the default VMA protection might state that it is writable, you actually want individual PTEs to be write-protected and have to manually remove the protection.
softdirty tracking is another special case: however, softdirty tracking is enabled for the whole VMA. For remove_migration_pte() that should be fine (I guess) because writenotify is active when the VMA needs to track softdirty bits, and consequently vma->vm_page_prot has the proper default permissions.
I wonder if the following (valid), for example is possible:
- clear_refs() clears VM_SOFTDIRTY and pte_wrprotect() the pte.
-> writenotify is active and vma->vm_page_prot updated accordingly
VM_SOFTDIRTY is reset due to VMA merging and vma->vm_page_prot is updated accordingly. See mmap_region() where we set VM_SOFTDIRTY.
If you now migrate the (still write-protected in the PTE) page, it was not writable, but it can be writable on the destination.
I didn't even notice merging could work with soft-dirty enabled, that's interesting to know.
Yes I think it's possible and I agree it's safe, as VM_SOFTDIRTY is set for the merged vma so afaiu the write bit is safe to set. We get a bunch of false positives but that's how soft-dirty works.
I think the whole problem is easier if we see this at a higher level. You're discussing this from vma pov and it's fair to do so, at least I agree with what you mentioned so far and I can't see anything outside uffd-wp that can be affected. However, it is also true when you noticed we already have quite a few paragraphs trying to discuss the safety for this and that, that's the part where I think we need justification and it's not that "natural".
For "natural", I meant fundamentally we're talking about page migrations here. The natural way (at least to me) for page migration to happen as a fundamental rule is that, we leverag the migration pte to make sure the pte being stable so we can do the migration work, then we "recover" the pte to present either by a full recovery or just (hopefully) only replace the pfn, keeping all the rest untouched.
One thing to prove that is we have two migration entries not one (I'm temporarily put the exclusive read one aside since that's solving different problems): MIGRATION_READ and MIGRATION_WRITE. If we only rely on vma flags logically we don't need MIGRATION_READ and MIGRATION_WRITE, we only need MIGRATION generic swap pte then we recover the write bit from vma flags and other things (like uffd-wp, currently we have the bit set in swap pte besides the swap entry type).
So maybe one day we can use two migration types rather than three (MIGRATION and MIGRATION_EXCLUSIVE)? I can't tell, but hopefully that shows what I meant, that we need further justification to grant write bit only base on vma, rather than recovering write bit based on migration entry type.
That's precisely what I had in mind recently, and I am happy to hear that you have similar idea:
https://lkml.kernel.org/r/20221108174652.198904-6-david@redhat.com
" Note that we don't optimize for the actual migration case: (1) When migration succeeds the new PTE will not be writable because the source PTE was not writable (protnone); in the future we might just optimize that case similarly by reusing can_change_pte_writable()/can_change_pmd_writable() when removing migration PTEs. "
Currently, "readable_migration_entry" is even wrong: it might be PROT_NONE and not even readable.