[PATCH v1 20/29] mm: convert "movable" flag in page->mapping to a page flag

[David Hildenbrand]

On 02.07.25 15:01, Harry Yoo wrote:
> On Wed, Jul 02, 2025 at 02:01:33PM +0200, David Hildenbrand wrote:
>> On 02.07.25 13:54, Harry Yoo wrote:
>>> On Mon, Jun 30, 2025 at 03:00:01PM +0200, David Hildenbrand wrote:
>>>> Instead, let's use a page flag. As the page flag can result in
>>>> false-positives, glue it to the page types for which we
>>>> support/implement movable_ops page migration.
>>>>
>>>> The flag reused by PageMovableOps() might be sued by other pages, so
>>>> warning in case it is set in page_has_movable_ops() might result in
>>>> false-positive warnings.
>>>>
>>>> Reviewed-by: Zi Yan <ziy at nvidia.com>
>>>> Signed-off-by: David Hildenbrand <david at redhat.com>
>>>> ---
>>>
>>> LGTM,
>>> Reviewed-by: Harry Yoo <harry.yoo at oracle.com>
>>>
>>> With a question: is there any reason to change the page flag
>>> operations to use atomic bit ops?
>>
>> As we have the page lock in there, it's complicated. I thought about this
>> when writing that code, and was not able to convince myself that it is safe.
>>
>> But that was when I was prototyping and reshuffling patches, and we would
>> still have code that would clear the flag.
>   
>> Given that we only allow setting the flag, it might be okay to use the
>> non-atomic variant as long as there can be nobody racing with us when
>> modifying flags. Especially trying to lock the folio concurrently is the big
>> problem.
>>
>> In isolate_movable_ops_page(), there is a comment about checking the flag
>> before grabbing the page lock, so that should be handled.
> 
> Right.
> 
>> I'll have to check some other cases in balloon/zsmalloc code.
> 
> Okay, it's totally fine to go with atomic version and then
> switching back to non atomic ops when we're sure it's safe.
> 

I'll definitely do the following:

diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 8b23a74963feb..5f2b570735852 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -1145,9 +1145,11 @@ PAGEFLAG(Isolated, isolated, PF_ANY);
   * the flag might be used in other context for other pages. Always use
   * page_has_movable_ops() instead.
   */
-PAGEFLAG(MovableOps, movable_ops, PF_NO_TAIL);
+TESTPAGEFLAG(MovableOps, movable_ops, PF_NO_TAIL);
+SETPAGEFLAG(MovableOps, movable_ops, PF_NO_TAIL);
  #else /* !CONFIG_MIGRATION */
-PAGEFLAG_FALSE(MovableOps, movable_ops);
+TESTPAGEFLAG_FALSE(MovableOps, movable_ops);
+SETPAGEFLAG_NOOP(MovableOps, movable_ops);
  #endif /* CONFIG_MIGRATION */
  
  /**

Because the flag must not get cleared.

There is no __SETPAGEFLAG_NOOP yet, unfortunately.
-- 
Cheers,

David / dhildenb