Received-SPF: pass (google.com: domain of linux-kernel+bounces-69381-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Fri, 16 Feb 2024 21:55:32 +0000
From: Matthew Wilcox <willy@infradead.org>
To: Sidhartha Kumar <sidhartha.kumar@oracle.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	akpm@linux-foundation.org, apopple@nvidia.com
Subject: Re: [PATCH v2 4/6] mm/migrate_device: convert
 __migrate_device_pages() to folios
Message-ID: <Zc_Z1PT20tNlsiFf@casper.infradead.org>
References: <20240216211320.222431-1-sidhartha.kumar@oracle.com>
 <20240216211320.222431-4-sidhartha.kumar@oracle.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240216211320.222431-4-sidhartha.kumar@oracle.com>

On Fri, Feb 16, 2024 at 01:13:18PM -0800, Sidhartha Kumar wrote:
> Use migrate_pfn_to_folio() so we can work with folios directly in
> __migrate_device_pages().

i don't understand why this would be correct if we have multipage
folios.

> @@ -719,33 +719,29 @@ static void __migrate_device_pages(unsigned long *src_pfns,
>  					migrate->pgmap_owner);
>  				mmu_notifier_invalidate_range_start(&range);
>  			}
> -			migrate_vma_insert_page(migrate, addr, newpage,
> +			migrate_vma_insert_page(migrate, addr, &dst->page,

seems to me that a migration pfn is going to refer to a precise page.
now you're telling it to insert the head page of the folio.  isn't this
wrong?

> @@ -753,13 +749,11 @@ static void __migrate_device_pages(unsigned long *src_pfns,
>  			continue;
>  		}
>  
> -		if (migrate && migrate->fault_page == page)
> -			r = migrate_folio_extra(mapping, page_folio(newpage),
> -						page_folio(page),
> -						MIGRATE_SYNC_NO_COPY, 1);
> +		if (migrate && migrate->fault_page == &src->page)

shouldn't this rather be "page_folio(migrate->fault_page) == src"?
ie we're looking for two pages from the same folio, rather than the page
being the same as the head page of the folio?