Re: [PATCH v4 1/4] mm/shmem: Unconditionally set pte dirty in mfill_atomic_install_pte

From: Peter Xu <peterx@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Hugh Dickins <hughd@google.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Andrea Arcangeli <aarcange@redhat.com>,
	Liam Howlett <liam.howlett@oracle.com>,
	Mike Rapoport <rppt@linux.vnet.ibm.com>,
	Yang Shi <shy828301@gmail.com>,
	David Hildenbrand <david@redhat.com>,
	"Kirill A . Shutemov" <kirill@shutemov.name>,
	Jerome Glisse <jglisse@redhat.com>,
	Alistair Popple <apopple@nvidia.com>,
	Miaohe Lin <linmiaohe@huawei.com>,
	Matthew Wilcox <willy@infradead.org>,
	Axel Rasmussen <axelrasmussen@google.com>
Subject: Re: [PATCH v4 1/4] mm/shmem: Unconditionally set pte dirty in mfill_atomic_install_pte
Date: Fri, 5 Nov 2021 09:01:32 +0800	[thread overview]
Message-ID: <YYSCbJ46kxaz2Ms9@xz-m1.local> (raw)
In-Reply-To: <20211104143440.d0056137ee07d3e96d6b56ec@linux-foundation.org>

On Thu, Nov 04, 2021 at 02:34:40PM -0700, Andrew Morton wrote:
> On Tue, 28 Sep 2021 17:37:31 -0400 Peter Xu <peterx@redhat.com> wrote:
> 
> > > How about a compromise (if you really want to continue with this patch):
> > > you leave the SetPageDirty(page) in shmem_mfill_atomic_pte(), where I
> > > feel a responsibility for it; but you do whatever works for you with
> > > pte_mkdirty() at the mm/userfaultfd.c end?
> > 
> > Sure.  Duplicating dirty bit is definitely fine to me as it achieves the same
> > goal as I hoped - we're still 100% clear we won't free a uffd page without
> > being noticed, then that's enough to me for the goal of this patch.  I won't
> > initiate that NACK myself since I still think duplicating is unnecessary no
> > matter it resides in shmem or uffd code, but please go ahead doing that and
> > I'll be fine with it, just in case Andrew didn't follow the details.
> 
> I think Hugh was asking you to implement this...
> 
> I guess I'll send this patch upstream.  But it does sound like Hugh
> would prefer a followon patch for this kernel release which makes the
> above change, please.

Thanks Andrew for helping.

But as I mentioned I still think that's odd to set dirty in both places.
That's why I don't want to draft the patch because I am not very willing to
sign-off..

If Hugh agrees, I can post the patch with Hugh's sign-off, adding the PageDirty
back too.  I am during a holiday so I cannot follow up the whole thing today,
but if it's easier for you to drop that patch or even drop the whole series,
please feel free to do.  I can rework everything too, then I'll try to get
Hugh's ack again on every single patch, as long as Hugh will have time to look
at it in the future.

Thanks,

-- 
Peter Xu