Re: [PATCH v4 1/3] mm/mprotect: Fix soft-dirty check in can_change_pte_writable()

[Peter Xu]

On Tue, Dec 20, 2022 at 11:15:00PM +0500, Muhammad Usama Anjum wrote:
> Hi Peter,

Hi, Muhammad,

[...]

> Nothing has changed for the userspace. But when the default soft-dirty
> feature always updates the soft-dirty flag in the PTEs regardless of
> VM_SOFTDIRTY is set or not

But it was not?  I don't see why the pte flags must be considered at all if
VM_SOFTDIRTY is set in existing code base, or before this patch.

> why does other components of the mm stop caring for soft-dirty flag in
> the PTE when VM_SOFTDIRTY is set?
> 
> > 
> > Your approach introduced PAGEMAP_NO_REUSED_REGIONS but that special
> > information is not remembered in vma, IIUC that's why you find things
> > messed up.  Fundamentally, it's because you're trying to reuse soft-dirty
> > design but it's not completely soft-dirty anymore.
> Correct, that's why I'm trying to find a way to correct the soft-dirty
> support instead of using anything else. We should try and correct it. I've
> sent a RFC to track the soft-dirty flags for sub regions in the VMA.

Note that I'm not against the change if that's servicing the purpose of the
enhancement you're proposing.  But again I wouldn't use "correct" as the
word here because I still didn't see anything wrong with the old code.

so IMHO the extra complexity on handling VM_SOFTDIRTY (even if to drop it
and replace with other structures to maintain ranged soft-dirty) needs to
be justified along with the feature introduced, not be justified as a fix.

Thanks,

-- 
Peter Xu