Re: [PATCH v2] mm/migrate: rename PAGE_ migration flags to FOLIO_

[David Hildenbrand (Arm)]

On 3/25/26 15:21, Zi Yan wrote:
> On 25 Mar 2026, at 7:04, Garg, Shivank wrote:
> 
>> On 3/25/2026 2:55 PM, David Hildenbrand (Arm) wrote:
>>>
>>>
>>> Isn't folio_change_private() part of the
>>> folio_attach_private()/folio_detach_private() interface that has
>>> completely different semantics?
>>>
>>> "The page must previously have had data attached and the data must be
>>> detached before the folio will be freed."
>>>
>>> (btw, that comment should refer to pages)
>>>
>>> So I would not use folio_change_private(). An accidental
>>> folio_attach_private() / folio_detach_private() would be rather undesired.
> 
> Hi David,

Hi,

> 
> In terms of folio_change_private(), I did not think it is related to
> folio_{attach,detach}_private(), since the latter change folio refcount during
> the operation. If folio_change_private() is related to attach/detach,
> I imagine it would check folio refcount before touches ->private. But
> that is my interpretation.

I mean, given that

a) It's located in pagemap.h in between folio_attach_private() and
folio_detach_private()

b) It clearly states that "The page must previously have had data
attached and the data must be detached before the folio will be freed."

This is the wrong API to use?

Sure, it sets folio->private but in different context.

I can spot one user in mm/hugetlb.c, that likely also should not be
using this API, because there likely was no previous attach/detach.

> 
> BTW, do you know why we have set_page_private() but no folio_set_private()?
> I would suggest folio_set_private() if it exists.

folio_set_private() sets ... PG_private. :)

folio_test_private() checks PG_private and folio_get_private() returns
page->private.

A cursed interface.

What we should likely do is:

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 3944b51ebac6..702cb2c0bc0e 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -426,6 +426,7 @@ struct folio {
                        union {
                                void *private;
                                swp_entry_t swap;
+                               unsigned long migrate_info;
                        };
                        atomic_t _mapcount;
                        atomic_t _refcount;

And not using folio->private in the first place. Just like we did for swap.

-- 
Cheers,

David