[PATCH v1 09/29] mm/migrate: factor out movable_ops page handling into migrate_movable_ops_page()

[David Hildenbrand]

On 01.07.25 11:24, David Hildenbrand wrote:
> On 30.06.25 19:05, Lorenzo Stoakes wrote:
>> On Mon, Jun 30, 2025 at 02:59:50PM +0200, David Hildenbrand wrote:
>>> Let's factor it out, simplifying the calling code.
>>>
>>> The assumption is that flush_dcache_page() is not required for
>>> movable_ops pages: as documented for flush_dcache_folio(), it really
>>> only applies when the kernel wrote to pagecache pages / pages in
>>> highmem. movable_ops callbacks should be handling flushing
>>> caches if ever required.
>>
>> But we've enot changed this have we? The flush_dcache_folio() invocation seems
>> to happen the same way now as before? Did I miss something?
> 
> I think, before this change we would have called it also for movable_ops
> pages
> 
> 
> if (rc == MIGRATEPAGE_SUCCESS) {
> 	if (__folio_test_movable(src)) {
> 		...
> 	}
> 
> 	...
> 
> 	if (likely(!folio_is_zone_device(dst)))
> 		flush_dcache_folio(dst);
> }
> 
> Now, we no longer do that for movable_ops pages.
> 
> For balloon pages, we're not copying anything, so we never possibly have
> to flush the dcache.
> 
> For zsmalloc, we do the copy in zs_object_copy() through kmap_local.
> 
> I think we could have HIGHMEM, so I wonder if we should just do a
> flush_dcache_page() in zs_object_copy().
> 
> At least, staring at highmem.h with memcpy_to_page(), it looks like that
> might be the right thing to do.
> 
> 
> So likely I'll add a patch before this one that will do the
> flush_dcache_page() in there.

But reading the docs again:

"This routine need only be called for page cache pages which can 
potentially ever be mapped into the address space of a user process."

So, not required IIUC. I'll clarify in the patch description.

-- 
Cheers,

David / dhildenb