On 02.07.25 15:01, Harry Yoo wrote:

On Wed, Jul 02, 2025 at 02:01:33PM +0200, David Hildenbrand wrote:

quoted

On 02.07.25 13:54, Harry Yoo wrote:

quoted

On Mon, Jun 30, 2025 at 03:00:01PM +0200, David Hildenbrand wrote:

quoted

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@nvidia.com>
Signed-off-by: David Hildenbrand <redacted>
---

LGTM,
Reviewed-by: Harry Yoo <redacted>

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.

quoted

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.

quoted

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