On 23.08.23 17:21, Yosry Ahmed wrote:

On Wed, Aug 23, 2023 at 8:17 AM David Hildenbrand [off-list ref] wrote:

quoted

On 23.08.23 17:12, Yosry Ahmed wrote:

quoted

On Mon, Aug 21, 2023 at 9:09 AM David Hildenbrand [off-list ref] wrote:

quoted

Let's stop using page->private on tail pages, making it possible to
just unconditionally reuse that field in the tail pages of large folios.

The remaining usage of the private field for THP_SWAP is in the THP
splitting code (mm/huge_memory.c), that we'll handle separately later.

Update the THP_SWAP documentation and sanity checks in mm_types.h and
__split_huge_page_tail().

Signed-off-by: David Hildenbrand <redacted>

The mm part looks good to me (with the added fixup):

Reviewed-by: Yosry Ahmed <redacted>

Thanks!

quoted

quoted

   /**

diff --git a/include/linux/swap.h b/include/linux/swap.h
index bb5adc604144..84fe0e94f5cd 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h

@@ -339,6 +339,15 @@ static inline swp_entry_t folio_swap_entry(struct folio *folio)
          return entry;
   }

+static inline swp_entry_t page_swap_entry(struct page *page)
+{
+       struct folio *folio = page_folio(page);
+       swp_entry_t entry = folio_swap_entry(folio);
+
+       entry.val += page - &folio->page;
+       return entry;
+}
+
   static inline void folio_set_swap_entry(struct folio *folio, swp_entry_t entry)
   {
          folio->private = (void *)entry.val;

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index cc2f65f8cc62..c04702ae71d2 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c

@@ -2446,18 +2446,15 @@ static void __split_huge_page_tail(struct page *head, int tail,
          page_tail->index = head->index + tail;

          /*
-        * page->private should not be set in tail pages with the exception
-        * of swap cache pages that store the swp_entry_t in tail pages.
-        * Fix up and warn once if private is unexpectedly set.
-        *
-        * What of 32-bit systems, on which folio->_pincount overlays
-        * head[1].private?  No problem: THP_SWAP is not enabled on 32-bit, and
-        * pincount must be 0 for folio_ref_freeze() to have succeeded.
+        * page->private should not be set in tail pages. Fix up and warn once
+        * if private is unexpectedly set.
           */
-       if (!folio_test_swapcache(page_folio(head))) {
-               VM_WARN_ON_ONCE_PAGE(page_tail->private != 0, page_tail);
+       if (unlikely(page_tail->private)) {
+               VM_WARN_ON_ONCE_PAGE(true, page_tail);
                  page_tail->private = 0;
          }

Could probably save a couple of lines here:

if (VM_WARN_ON_ONCE_PAGE(page_tail->private != 0, page_tail))

         page_tail->private = 0;

That would mean that we eventually compile out the runtime check

#define VM_WARN_ON_ONCE_PAGE(cond, page)  BUILD_BUG_ON_INVALID(cond)

I thought the warning would be compiled out but not the check, my bad.

I even remembered that VM_WARN_ON_ONCE and friends could/should not be
used in conditionals.

But we do seem to have two users now:

  $ git grep "if (VM_WARN_ON"
mm/mmap.c:              if (VM_WARN_ON_ONCE_MM(vma->vm_end != vmi_end, mm))
mm/mmap.c:              if (VM_WARN_ON_ONCE_MM(vma->vm_start != vmi_start, mm))

But they only do warning-related action, to dump the stack, the vma, ...

So if the warnings get compiled out, also all the other stuff gets compiled out as well,
which makes sense here.

-- 
Cheers,

David / dhildenb


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel