Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 20 Dec 2022 14:02:07 -0500
From:   Peter Xu <peterx@redhat.com>
To:     Muhammad Usama Anjum <usama.anjum@collabora.com>
Cc:     David Hildenbrand <david@redhat.com>,
        Cyrill Gorcunov <gorcunov@gmail.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Paul Gofman <pgofman@codeweavers.com>,
        Nadav Amit <nadav.amit@gmail.com>,
        Andrea Arcangeli <aarcange@redhat.com>,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        kernel@collabora.com
Subject: Re: [PATCH v4 1/3] mm/mprotect: Fix soft-dirty check in
 can_change_pte_writable()
Message-ID: <Y6IGr2Y21GlLTSRl@x1n>
References: <20220725142048.30450-1-peterx@redhat.com>
 <20220725142048.30450-2-peterx@redhat.com>
 <b75653b8-5264-ca03-bf5c-671e07e7fdd8@collabora.com>
 <Y3gRy8pUiYWFGqcI@x1n>
 <a9984aa6-41bc-17c3-b564-87b997c0f2d0@collabora.com>
 <Y3vq18eTOE0e7I1+@x1n>
 <e3c26b56-5458-eb25-c753-fc4c058ba1b1@collabora.com>
 <Y6Hc2d+7eTKs7AiH@x1n>
 <0a3e3397-6ff3-1203-52cb-49636ef38247@collabora.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <0a3e3397-6ff3-1203-52cb-49636ef38247@collabora.com>
Precedence: bulk

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