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:   Wed, 10 Aug 2022 15:21:20 -0400
From:   Peter Xu <peterx@redhat.com>
To:     "Huang, Ying" <ying.huang@intel.com>
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        Andrea Arcangeli <aarcange@redhat.com>,
        David Hildenbrand <david@redhat.com>,
        Minchan Kim <minchan@kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Vlastimil Babka <vbabka@suse.cz>,
        Nadav Amit <nadav.amit@gmail.com>,
        Hugh Dickins <hughd@google.com>,
        Andi Kleen <andi.kleen@intel.com>,
        "Kirill A . Shutemov" <kirill@shutemov.name>,
        Christoph Hellwig <hch@lst.de>,
        Alistair Popple <apopple@nvidia.com>,
        Jason Gunthorpe <jgg@ziepe.ca>
Subject: Re: [PATCH v2 2/2] mm: Remember young/dirty bit for page migrations
Message-ID: <YvQFMMKxO2SD0T1T@xz-m1.local>
References: <20220804203952.53665-1-peterx@redhat.com>
 <20220804203952.53665-3-peterx@redhat.com>
 <877d3hhksz.fsf@yhuang6-desk2.ccr.corp.intel.com>
 <YvKglA2LQkYeznZ9@xz-m1.local>
 <87wnbggbqa.fsf@yhuang6-desk2.ccr.corp.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <87wnbggbqa.fsf@yhuang6-desk2.ccr.corp.intel.com>
Precedence: bulk

On Wed, Aug 10, 2022 at 08:53:49AM +0800, Huang, Ying wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > On Tue, Aug 09, 2022 at 04:40:12PM +0800, Huang, Ying wrote:
> [snip]
> >
> >> I don't find pte_dirty() is synced to PageDirty() as in
> >> try_to_migrate_one().  Is it a issue in the original code?
> >
> > I think it has?  There is:
> >
> > 		/* Set the dirty flag on the folio now the pte is gone. */
> > 		if (pte_dirty(pteval))
> > 			folio_mark_dirty(folio);
> >
> 
> Sorry, my original words are confusing.  Yes, there's dirty bit syncing
> in try_to_migrate_one().  But I don't find that in migrate_device.c
> 
>  $ grep dirty mm/migrate_device.c
> 				if (pte_soft_dirty(pte))
> 					swp_pte = pte_swp_mksoft_dirty(swp_pte);
> 				if (pte_swp_soft_dirty(pte))
> 					swp_pte = pte_swp_mksoft_dirty(swp_pte);
> 			entry = pte_mkwrite(pte_mkdirty(entry));
> 
> I guess that migrate_device.c is used to migrate between CPU visible
> page to CPU un-visible page (device visible), so the rule is different?

IIUC migrate_vma_collect() handles migrations for both directions (RAM <->
device mem).

Yeah, indeed I also didn't see how migrate_vma_collect_pmd() handles the
carry-over of pte dirty to page dirty, which looks a bit odd.  I also don't
see why the dirty bit doesn't need to be maintained, e.g. when a previous
page was dirty then after migration of ram->dev->ram it seems to be clean
with current code.

Another scenario is, even if the page was clean, as long as page migrated
to device mem, device DMAed to the page, then page migrated back to RAM.  I
also didn't see how we could detect the DMAs and set pte/page dirty
properly after migrated back.

Copy Alistair and Jason..

-- 
Peter Xu