Re: [PATCH RFC v3] mm: Proper document tail pages fields for folio

[Matthew Wilcox]

On Wed, Aug 16, 2023 at 08:51:49PM +0200, David Hildenbrand wrote:
> On 16.08.23 20:41, Matthew Wilcox wrote:
> > On Wed, Aug 16, 2023 at 03:33:30PM +0200, David Hildenbrand wrote:
> > > My simple tests passed so far. If there isn't something obvious missing,
> > > I can do more testing and send this as an official patch.
> > 
> > I think you missed one:
> > 
> > +++ b/mm/swapfile.c
> > @@ -1490,7 +1490,7 @@ int swp_swapcount(swp_entry_t entry)
> > 
> >          page = vmalloc_to_page(p->swap_map + offset);
> >          offset &= ~PAGE_MASK;
> > -       VM_BUG_ON(page_private(page) != SWP_CONTINUED);
> > +       VM_BUG_ON(page_swap_entry(page).val != SWP_CONTINUED);
> 
> That falls under the "weird handling of SWP_CONTINUED using vmalloced
> pages". So different user of page_private().
> 
> Note that we don't even store swap entries in there but extended swap
> counts.

Ah, right.  I see now.


Not necessarily as part of this patch, but it got me wondering ...
should we do this?  And then maybe we could remove folio_swap_entry()
and folio_set_swap_entry() and just use folio->swap directly.


diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 3880b3f2e321..e23d1356e504 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -266,6 +266,14 @@ static inline struct page *encoded_page_ptr(struct encoded_page *page)
 	return (struct page *)(~ENCODE_PAGE_BITS & (unsigned long)page);
 }
 
+/*
+ * A swap entry has to fit into a "unsigned long", as the entry is hidden
+ * in the "index" field of the swapper address space.
+ */
+typedef struct {
+	unsigned long val;
+} swp_entry_t;
+
 /**
  * struct folio - Represents a contiguous set of bytes.
  * @flags: Identical to the page flags.
@@ -276,7 +284,7 @@ static inline struct page *encoded_page_ptr(struct encoded_page *page)
  * @index: Offset within the file, in units of pages.  For anonymous memory,
  *    this is the index from the beginning of the mmap.
  * @private: Filesystem per-folio data (see folio_attach_private()).
- *    Used for swp_entry_t if folio_test_swapcache().
+ * @swap: Used for swp_entry_t if folio_test_swapcache().
  * @_mapcount: Do not access this member directly.  Use folio_mapcount() to
  *    find out how many times this folio is mapped by userspace.
  * @_refcount: Do not access this member directly.  Use folio_ref_count()
@@ -319,7 +327,10 @@ struct folio {
 			};
 			struct address_space *mapping;
 			pgoff_t index;
-			void *private;
+			union {
+				void *private;
+				swp_entry_t swap;
+			};
 			atomic_t _mapcount;
 			atomic_t _refcount;
 #ifdef CONFIG_MEMCG
@@ -1158,14 +1169,6 @@ enum tlb_flush_reason {
 	NR_TLB_FLUSH_REASONS,
 };
 
- /*
-  * A swap entry has to fit into a "unsigned long", as the entry is hidden
-  * in the "index" field of the swapper address space.
-  */
-typedef struct {
-	unsigned long val;
-} swp_entry_t;
-
 /**
  * enum fault_flag - Fault flag definitions.
  * @FAULT_FLAG_WRITE: Fault was a write fault.
diff --git a/include/linux/swap.h b/include/linux/swap.h
index bb5adc604144..59b0f37eae5b 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -335,13 +335,12 @@ struct swap_info_struct {
 
 static inline swp_entry_t folio_swap_entry(struct folio *folio)
 {
-	swp_entry_t entry = { .val = page_private(&folio->page) };
-	return entry;
+	return folio->swap;
 }
 
 static inline void folio_set_swap_entry(struct folio *folio, swp_entry_t entry)
 {
-	folio->private = (void *)entry.val;
+	folio->swap = entry;
 }
 
 /* linux/mm/workingset.c */