[PATCH 0/3] minor fixes and supplement for ptdesc

[PATCH 0/3] minor fixes and supplement for ptdesc

[Qi Zheng]

Hi all,

In this series, the [PATCH 1/3] and [PATCH 2/3] are fixes for some issues
discovered during code inspection.

The [PATCH 3/3] is a supplement to ptdesc conversion in s390, I don't know
why this is not done in the commit 6326c26c1514 ("s390: convert various pgalloc
functions to use ptdescs"), maybe I missed something. And since I don't have an
s390 environment, I hope kernel test robot can help compile and test, and this
is why I did not fold [PATCH 2/3] and [PATCH 3/3] into one patch.

Comments and suggestions are welcome.

Thanks,
Qi

Qi Zheng (3):
  mm: pgtable: correct the wrong comment about ptdesc->__page_flags
  mm: pgtable: add missing pt_index to struct ptdesc
  s390: supplement for ptdesc conversion

 arch/s390/include/asm/pgalloc.h |  4 ++--
 arch/s390/mm/gmap.c             | 38 +++++++++++++++++----------------
 arch/s390/mm/pgalloc.c          |  8 +++----
 include/linux/mm_types.h        |  5 ++++-
 4 files changed, 30 insertions(+), 25 deletions(-)

-- 
2.30.2

[PATCH 1/3] mm: pgtable: correct the wrong comment about ptdesc->__page_flags

[Qi Zheng]

The commit 32cc0b7c9d50 ("powerpc: add pte_free_defer() for pgtables
sharing page") introduced the use of PageActive flag to page table
fragments tracking, so the ptdesc->__page_flags is not unused, so
correct the wrong comment.

Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
---
 include/linux/mm_types.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index a7223ba3ea1e..5ea77969daae 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -419,7 +419,7 @@ FOLIO_MATCH(compound_head, _head_2a);
 
 /**
  * struct ptdesc -    Memory descriptor for page tables.
- * @__page_flags:     Same as page flags. Unused for page tables.
+ * @__page_flags:     Same as page flags. Powerpc only.
  * @pt_rcu_head:      For freeing page table pages.
  * @pt_list:          List of used page tables. Used for s390 and x86.
  * @_pt_pad_1:        Padding that aliases with page's compound head.
-- 
2.30.2

Re: [PATCH 1/3] mm: pgtable: correct the wrong comment about ptdesc->__page_flags

[Vishal Moola]

On Mon, Mar 04, 2024 at 07:07:18PM +0800, Qi Zheng wrote:
> The commit 32cc0b7c9d50 ("powerpc: add pte_free_defer() for pgtables
> sharing page") introduced the use of PageActive flag to page table
> fragments tracking, so the ptdesc->__page_flags is not unused, so
> correct the wrong comment.

Thanks for catching this!

In regards to naming, we're trying to prefix unused variables with
__underscores so I'd prefer to see the __ eliminated from the
ptdesc->page_flags field here as well. This doesn't warrant a fix
as it is already in 6.9-rc1, but is something to keep in
mind for the future. Aside from that, LGTM.

Reviewed-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
> ---
>  include/linux/mm_types.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index a7223ba3ea1e..5ea77969daae 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -419,7 +419,7 @@ FOLIO_MATCH(compound_head, _head_2a);
>  
>  /**
>   * struct ptdesc -    Memory descriptor for page tables.
> - * @__page_flags:     Same as page flags. Unused for page tables.
> + * @__page_flags:     Same as page flags. Powerpc only.
>   * @pt_rcu_head:      For freeing page table pages.
>   * @pt_list:          List of used page tables. Used for s390 and x86.
>   * @_pt_pad_1:        Padding that aliases with page's compound head.
> -- 
> 2.30.2
> 

Re: [PATCH 1/3] mm: pgtable: correct the wrong comment about ptdesc->__page_flags

[Qi Zheng]

On 2024/3/27 03:12, Vishal Moola wrote:
> On Mon, Mar 04, 2024 at 07:07:18PM +0800, Qi Zheng wrote:
>> The commit 32cc0b7c9d50 ("powerpc: add pte_free_defer() for pgtables
>> sharing page") introduced the use of PageActive flag to page table
>> fragments tracking, so the ptdesc->__page_flags is not unused, so
>> correct the wrong comment.
> 
> Thanks for catching this!
> 
> In regards to naming, we're trying to prefix unused variables with
> __underscores so I'd prefer to see the __ eliminated from the
> ptdesc->page_flags field here as well. This doesn't warrant a fix
> as it is already in 6.9-rc1, but is something to keep in
> mind for the future. 

Got it.

> Aside from that, LGTM.
> 
> Reviewed-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>

Thanks!

>> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
>> ---
>>   include/linux/mm_types.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
>> index a7223ba3ea1e..5ea77969daae 100644
>> --- a/include/linux/mm_types.h
>> +++ b/include/linux/mm_types.h
>> @@ -419,7 +419,7 @@ FOLIO_MATCH(compound_head, _head_2a);
>>   
>>   /**
>>    * struct ptdesc -    Memory descriptor for page tables.
>> - * @__page_flags:     Same as page flags. Unused for page tables.
>> + * @__page_flags:     Same as page flags. Powerpc only.
>>    * @pt_rcu_head:      For freeing page table pages.
>>    * @pt_list:          List of used page tables. Used for s390 and x86.
>>    * @_pt_pad_1:        Padding that aliases with page's compound head.
>> -- 
>> 2.30.2
>>

[PATCH 2/3] mm: pgtable: add missing pt_index to struct ptdesc

[Qi Zheng]

In s390, the page->index field is used for gmap (see gmap_shadow_pgt()),
so add the corresponding pt_index to struct ptdesc and add a comment to
clarify this.

Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
---
 include/linux/mm_types.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 5ea77969daae..5240bd7bca33 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -425,6 +425,7 @@ FOLIO_MATCH(compound_head, _head_2a);
  * @_pt_pad_1:        Padding that aliases with page's compound head.
  * @pmd_huge_pte:     Protected by ptdesc->ptl, used for THPs.
  * @__page_mapping:   Aliases with page->mapping. Unused for page tables.
+ * @pt_index:         Used for s390 gmap.
  * @pt_mm:            Used for x86 pgds.
  * @pt_frag_refcount: For fragmented page table tracking. Powerpc only.
  * @_pt_pad_2:        Padding to ensure proper alignment.
@@ -450,6 +451,7 @@ struct ptdesc {
 	unsigned long __page_mapping;
 
 	union {
+		pgoff_t pt_index;
 		struct mm_struct *pt_mm;
 		atomic_t pt_frag_refcount;
 	};
@@ -475,6 +477,7 @@ TABLE_MATCH(flags, __page_flags);
 TABLE_MATCH(compound_head, pt_list);
 TABLE_MATCH(compound_head, _pt_pad_1);
 TABLE_MATCH(mapping, __page_mapping);
+TABLE_MATCH(index, pt_index);
 TABLE_MATCH(rcu_head, pt_rcu_head);
 TABLE_MATCH(page_type, __page_type);
 TABLE_MATCH(_refcount, __page_refcount);
-- 
2.30.2

Re: [PATCH 2/3] mm: pgtable: add missing pt_index to struct ptdesc

[Vishal Moola]

On Mon, Mar 04, 2024 at 07:07:19PM +0800, Qi Zheng wrote:
> In s390, the page->index field is used for gmap (see gmap_shadow_pgt()),
> so add the corresponding pt_index to struct ptdesc and add a comment to
> clarify this.

Yes s390 gmap 'uses' page->index, but not for the purpose page->index is
supposed to hold. It's alright to have a variable here, but I'd rather
see it named something more appropriate to the purporse it serves.

You can take look at this patch from v5 of my ptdesc conversion series
for more info:
https://lore.kernel.org/linux-mm/20230622205745.79707-3-vishal.moola@gmail.com/

> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
> ---
>  include/linux/mm_types.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 5ea77969daae..5240bd7bca33 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -425,6 +425,7 @@ FOLIO_MATCH(compound_head, _head_2a);
>   * @_pt_pad_1:        Padding that aliases with page's compound head.
>   * @pmd_huge_pte:     Protected by ptdesc->ptl, used for THPs.
>   * @__page_mapping:   Aliases with page->mapping. Unused for page tables.
> + * @pt_index:         Used for s390 gmap.
>   * @pt_mm:            Used for x86 pgds.
>   * @pt_frag_refcount: For fragmented page table tracking. Powerpc only.
>   * @_pt_pad_2:        Padding to ensure proper alignment.
> @@ -450,6 +451,7 @@ struct ptdesc {
>  	unsigned long __page_mapping;
>  
>  	union {
> +		pgoff_t pt_index;
>  		struct mm_struct *pt_mm;
>  		atomic_t pt_frag_refcount;
>  	};
> @@ -475,6 +477,7 @@ TABLE_MATCH(flags, __page_flags);
>  TABLE_MATCH(compound_head, pt_list);
>  TABLE_MATCH(compound_head, _pt_pad_1);
>  TABLE_MATCH(mapping, __page_mapping);
> +TABLE_MATCH(index, pt_index);
>  TABLE_MATCH(rcu_head, pt_rcu_head);
>  TABLE_MATCH(page_type, __page_type);
>  TABLE_MATCH(_refcount, __page_refcount);
> -- 
> 2.30.2
> 

Re: [PATCH 2/3] mm: pgtable: add missing pt_index to struct ptdesc

[Qi Zheng]

On 2024/3/27 03:25, Vishal Moola wrote:
> On Mon, Mar 04, 2024 at 07:07:19PM +0800, Qi Zheng wrote:
>> In s390, the page->index field is used for gmap (see gmap_shadow_pgt()),
>> so add the corresponding pt_index to struct ptdesc and add a comment to
>> clarify this.
> 
> Yes s390 gmap 'uses' page->index, but not for the purpose page->index is
> supposed to hold. It's alright to have a variable here, but I'd rather
> see it named something more appropriate to the purporse it serves.

Make sense.

> 
> You can take look at this patch from v5 of my ptdesc conversion series
> for more info:
> https://lore.kernel.org/linux-mm/20230622205745.79707-3-vishal.moola@gmail.com/

Oh, but it seems that this patch has not been merged?

> 
>> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
>> ---
>>   include/linux/mm_types.h | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
>> index 5ea77969daae..5240bd7bca33 100644
>> --- a/include/linux/mm_types.h
>> +++ b/include/linux/mm_types.h
>> @@ -425,6 +425,7 @@ FOLIO_MATCH(compound_head, _head_2a);
>>    * @_pt_pad_1:        Padding that aliases with page's compound head.
>>    * @pmd_huge_pte:     Protected by ptdesc->ptl, used for THPs.
>>    * @__page_mapping:   Aliases with page->mapping. Unused for page tables.
>> + * @pt_index:         Used for s390 gmap.
>>    * @pt_mm:            Used for x86 pgds.
>>    * @pt_frag_refcount: For fragmented page table tracking. Powerpc only.
>>    * @_pt_pad_2:        Padding to ensure proper alignment.
>> @@ -450,6 +451,7 @@ struct ptdesc {
>>   	unsigned long __page_mapping;
>>   
>>   	union {
>> +		pgoff_t pt_index;
>>   		struct mm_struct *pt_mm;
>>   		atomic_t pt_frag_refcount;
>>   	};
>> @@ -475,6 +477,7 @@ TABLE_MATCH(flags, __page_flags);
>>   TABLE_MATCH(compound_head, pt_list);
>>   TABLE_MATCH(compound_head, _pt_pad_1);
>>   TABLE_MATCH(mapping, __page_mapping);
>> +TABLE_MATCH(index, pt_index);
>>   TABLE_MATCH(rcu_head, pt_rcu_head);
>>   TABLE_MATCH(page_type, __page_type);
>>   TABLE_MATCH(_refcount, __page_refcount);
>> -- 
>> 2.30.2
>>

[PATCH 3/3] s390: supplement for ptdesc conversion

[Qi Zheng]

After commit 6326c26c1514 ("s390: convert various pgalloc functions to use
ptdescs"), there are still some positions that use page->{lru, index}
instead of ptdesc->{pt_list, pt_index}. In order to make the use of
ptdesc->{pt_list, pt_index} clearer, it would be better to convert them
as well.

Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
Cc: Janosch Frank <frankja@linux.ibm.com>
Cc: Claudio Imbrenda <imbrenda@linux.ibm.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: kvm@vger.kernel.org
Cc: linux-s390@vger.kernel.org
---
 arch/s390/include/asm/pgalloc.h |  4 ++--
 arch/s390/mm/gmap.c             | 38 +++++++++++++++++----------------
 arch/s390/mm/pgalloc.c          |  8 +++----
 3 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/arch/s390/include/asm/pgalloc.h b/arch/s390/include/asm/pgalloc.h
index 502d655fe6ae..7b84ef6dc4b6 100644
--- a/arch/s390/include/asm/pgalloc.h
+++ b/arch/s390/include/asm/pgalloc.h
@@ -23,9 +23,9 @@ unsigned long *crst_table_alloc(struct mm_struct *);
 void crst_table_free(struct mm_struct *, unsigned long *);
 
 unsigned long *page_table_alloc(struct mm_struct *);
-struct page *page_table_alloc_pgste(struct mm_struct *mm);
+struct ptdesc *page_table_alloc_pgste(struct mm_struct *mm);
 void page_table_free(struct mm_struct *, unsigned long *);
-void page_table_free_pgste(struct page *page);
+void page_table_free_pgste(struct ptdesc *ptdesc);
 extern int page_table_allocate_pgste;
 
 static inline void crst_table_init(unsigned long *crst, unsigned long entry)
diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c
index 8da39deb56ca..4d2674f89322 100644
--- a/arch/s390/mm/gmap.c
+++ b/arch/s390/mm/gmap.c
@@ -206,9 +206,11 @@ static void gmap_free(struct gmap *gmap)
 
 	/* Free additional data for a shadow gmap */
 	if (gmap_is_shadow(gmap)) {
+		struct ptdesc *ptdesc;
+
 		/* Free all page tables. */
-		list_for_each_entry_safe(page, next, &gmap->pt_list, lru)
-			page_table_free_pgste(page);
+		list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
+			page_table_free_pgste(ptdesc);
 		gmap_rmap_radix_tree_free(&gmap->host_to_rmap);
 		/* Release reference to the parent */
 		gmap_put(gmap->parent);
@@ -1348,7 +1350,7 @@ static void gmap_unshadow_pgt(struct gmap *sg, unsigned long raddr)
 {
 	unsigned long *ste;
 	phys_addr_t sto, pgt;
-	struct page *page;
+	struct ptdesc *ptdesc;
 
 	BUG_ON(!gmap_is_shadow(sg));
 	ste = gmap_table_walk(sg, raddr, 1); /* get segment pointer */
@@ -1361,9 +1363,9 @@ static void gmap_unshadow_pgt(struct gmap *sg, unsigned long raddr)
 	*ste = _SEGMENT_ENTRY_EMPTY;
 	__gmap_unshadow_pgt(sg, raddr, __va(pgt));
 	/* Free page table */
-	page = phys_to_page(pgt);
-	list_del(&page->lru);
-	page_table_free_pgste(page);
+	ptdesc = page_ptdesc(phys_to_page(pgt));
+	list_del(&ptdesc->pt_list);
+	page_table_free_pgste(ptdesc);
 }
 
 /**
@@ -1377,7 +1379,7 @@ static void gmap_unshadow_pgt(struct gmap *sg, unsigned long raddr)
 static void __gmap_unshadow_sgt(struct gmap *sg, unsigned long raddr,
 				unsigned long *sgt)
 {
-	struct page *page;
+	struct ptdesc *ptdesc;
 	phys_addr_t pgt;
 	int i;
 
@@ -1389,9 +1391,9 @@ static void __gmap_unshadow_sgt(struct gmap *sg, unsigned long raddr,
 		sgt[i] = _SEGMENT_ENTRY_EMPTY;
 		__gmap_unshadow_pgt(sg, raddr, __va(pgt));
 		/* Free page table */
-		page = phys_to_page(pgt);
-		list_del(&page->lru);
-		page_table_free_pgste(page);
+		ptdesc = page_ptdesc(phys_to_page(pgt));
+		list_del(&ptdesc->pt_list);
+		page_table_free_pgste(ptdesc);
 	}
 }
 
@@ -2058,19 +2060,19 @@ int gmap_shadow_pgt(struct gmap *sg, unsigned long saddr, unsigned long pgt,
 {
 	unsigned long raddr, origin;
 	unsigned long *table;
-	struct page *page;
+	struct ptdesc *ptdesc;
 	phys_addr_t s_pgt;
 	int rc;
 
 	BUG_ON(!gmap_is_shadow(sg) || (pgt & _SEGMENT_ENTRY_LARGE));
 	/* Allocate a shadow page table */
-	page = page_table_alloc_pgste(sg->mm);
-	if (!page)
+	ptdesc = page_table_alloc_pgste(sg->mm);
+	if (!ptdesc)
 		return -ENOMEM;
-	page->index = pgt & _SEGMENT_ENTRY_ORIGIN;
+	ptdesc->pt_index = pgt & _SEGMENT_ENTRY_ORIGIN;
 	if (fake)
-		page->index |= GMAP_SHADOW_FAKE_TABLE;
-	s_pgt = page_to_phys(page);
+		ptdesc->pt_index |= GMAP_SHADOW_FAKE_TABLE;
+	s_pgt = page_to_phys(ptdesc_page(ptdesc));
 	/* Install shadow page table */
 	spin_lock(&sg->guest_table_lock);
 	table = gmap_table_walk(sg, saddr, 1); /* get segment pointer */
@@ -2088,7 +2090,7 @@ int gmap_shadow_pgt(struct gmap *sg, unsigned long saddr, unsigned long pgt,
 	/* mark as invalid as long as the parent table is not protected */
 	*table = (unsigned long) s_pgt | _SEGMENT_ENTRY |
 		 (pgt & _SEGMENT_ENTRY_PROTECT) | _SEGMENT_ENTRY_INVALID;
-	list_add(&page->lru, &sg->pt_list);
+	list_add(&ptdesc->pt_list, &sg->pt_list);
 	if (fake) {
 		/* nothing to protect for fake tables */
 		*table &= ~_SEGMENT_ENTRY_INVALID;
@@ -2114,7 +2116,7 @@ int gmap_shadow_pgt(struct gmap *sg, unsigned long saddr, unsigned long pgt,
 	return rc;
 out_free:
 	spin_unlock(&sg->guest_table_lock);
-	page_table_free_pgste(page);
+	page_table_free_pgste(ptdesc);
 	return rc;
 
 }
diff --git a/arch/s390/mm/pgalloc.c b/arch/s390/mm/pgalloc.c
index 008e487c94a6..abb629d7e131 100644
--- a/arch/s390/mm/pgalloc.c
+++ b/arch/s390/mm/pgalloc.c
@@ -135,7 +135,7 @@ int crst_table_upgrade(struct mm_struct *mm, unsigned long end)
 
 #ifdef CONFIG_PGSTE
 
-struct page *page_table_alloc_pgste(struct mm_struct *mm)
+struct ptdesc *page_table_alloc_pgste(struct mm_struct *mm)
 {
 	struct ptdesc *ptdesc;
 	u64 *table;
@@ -147,12 +147,12 @@ struct page *page_table_alloc_pgste(struct mm_struct *mm)
 		memset64(table, _PAGE_INVALID, PTRS_PER_PTE);
 		memset64(table + PTRS_PER_PTE, 0, PTRS_PER_PTE);
 	}
-	return ptdesc_page(ptdesc);
+	return ptdesc;
 }
 
-void page_table_free_pgste(struct page *page)
+void page_table_free_pgste(struct ptdesc *ptdesc)
 {
-	pagetable_free(page_ptdesc(page));
+	pagetable_free(ptdesc);
 }
 
 #endif /* CONFIG_PGSTE */
-- 
2.30.2

[PATCH v2 3/3] s390: supplement for ptdesc conversion

[Qi Zheng]

After commit 6326c26c1514 ("s390: convert various pgalloc functions to use
ptdescs"), there are still some positions that use page->{lru, index}
instead of ptdesc->{pt_list, pt_index}. In order to make the use of
ptdesc->{pt_list, pt_index} clearer, it would be better to convert them
as well.

Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
Cc: Janosch Frank <frankja@linux.ibm.com>
Cc: Claudio Imbrenda <imbrenda@linux.ibm.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: kvm@vger.kernel.org
Cc: linux-s390@vger.kernel.org
---
v1 -> v2: fix build failure (cross compilation successful)

 arch/s390/include/asm/pgalloc.h |  4 ++--
 arch/s390/mm/gmap.c             | 38 +++++++++++++++++----------------
 arch/s390/mm/pgalloc.c          |  8 +++----
 3 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/arch/s390/include/asm/pgalloc.h b/arch/s390/include/asm/pgalloc.h
index 502d655fe6ae..7b84ef6dc4b6 100644
--- a/arch/s390/include/asm/pgalloc.h
+++ b/arch/s390/include/asm/pgalloc.h
@@ -23,9 +23,9 @@ unsigned long *crst_table_alloc(struct mm_struct *);
 void crst_table_free(struct mm_struct *, unsigned long *);
 
 unsigned long *page_table_alloc(struct mm_struct *);
-struct page *page_table_alloc_pgste(struct mm_struct *mm);
+struct ptdesc *page_table_alloc_pgste(struct mm_struct *mm);
 void page_table_free(struct mm_struct *, unsigned long *);
-void page_table_free_pgste(struct page *page);
+void page_table_free_pgste(struct ptdesc *ptdesc);
 extern int page_table_allocate_pgste;
 
 static inline void crst_table_init(unsigned long *crst, unsigned long entry)
diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c
index 8da39deb56ca..e43a5a3befd4 100644
--- a/arch/s390/mm/gmap.c
+++ b/arch/s390/mm/gmap.c
@@ -206,9 +206,11 @@ static void gmap_free(struct gmap *gmap)
 
 	/* Free additional data for a shadow gmap */
 	if (gmap_is_shadow(gmap)) {
+		struct ptdesc *ptdesc, *n;
+
 		/* Free all page tables. */
-		list_for_each_entry_safe(page, next, &gmap->pt_list, lru)
-			page_table_free_pgste(page);
+		list_for_each_entry_safe(ptdesc, n, &gmap->pt_list, pt_list)
+			page_table_free_pgste(ptdesc);
 		gmap_rmap_radix_tree_free(&gmap->host_to_rmap);
 		/* Release reference to the parent */
 		gmap_put(gmap->parent);
@@ -1348,7 +1350,7 @@ static void gmap_unshadow_pgt(struct gmap *sg, unsigned long raddr)
 {
 	unsigned long *ste;
 	phys_addr_t sto, pgt;
-	struct page *page;
+	struct ptdesc *ptdesc;
 
 	BUG_ON(!gmap_is_shadow(sg));
 	ste = gmap_table_walk(sg, raddr, 1); /* get segment pointer */
@@ -1361,9 +1363,9 @@ static void gmap_unshadow_pgt(struct gmap *sg, unsigned long raddr)
 	*ste = _SEGMENT_ENTRY_EMPTY;
 	__gmap_unshadow_pgt(sg, raddr, __va(pgt));
 	/* Free page table */
-	page = phys_to_page(pgt);
-	list_del(&page->lru);
-	page_table_free_pgste(page);
+	ptdesc = page_ptdesc(phys_to_page(pgt));
+	list_del(&ptdesc->pt_list);
+	page_table_free_pgste(ptdesc);
 }
 
 /**
@@ -1377,7 +1379,7 @@ static void gmap_unshadow_pgt(struct gmap *sg, unsigned long raddr)
 static void __gmap_unshadow_sgt(struct gmap *sg, unsigned long raddr,
 				unsigned long *sgt)
 {
-	struct page *page;
+	struct ptdesc *ptdesc;
 	phys_addr_t pgt;
 	int i;
 
@@ -1389,9 +1391,9 @@ static void __gmap_unshadow_sgt(struct gmap *sg, unsigned long raddr,
 		sgt[i] = _SEGMENT_ENTRY_EMPTY;
 		__gmap_unshadow_pgt(sg, raddr, __va(pgt));
 		/* Free page table */
-		page = phys_to_page(pgt);
-		list_del(&page->lru);
-		page_table_free_pgste(page);
+		ptdesc = page_ptdesc(phys_to_page(pgt));
+		list_del(&ptdesc->pt_list);
+		page_table_free_pgste(ptdesc);
 	}
 }
 
@@ -2058,19 +2060,19 @@ int gmap_shadow_pgt(struct gmap *sg, unsigned long saddr, unsigned long pgt,
 {
 	unsigned long raddr, origin;
 	unsigned long *table;
-	struct page *page;
+	struct ptdesc *ptdesc;
 	phys_addr_t s_pgt;
 	int rc;
 
 	BUG_ON(!gmap_is_shadow(sg) || (pgt & _SEGMENT_ENTRY_LARGE));
 	/* Allocate a shadow page table */
-	page = page_table_alloc_pgste(sg->mm);
-	if (!page)
+	ptdesc = page_table_alloc_pgste(sg->mm);
+	if (!ptdesc)
 		return -ENOMEM;
-	page->index = pgt & _SEGMENT_ENTRY_ORIGIN;
+	ptdesc->pt_index = pgt & _SEGMENT_ENTRY_ORIGIN;
 	if (fake)
-		page->index |= GMAP_SHADOW_FAKE_TABLE;
-	s_pgt = page_to_phys(page);
+		ptdesc->pt_index |= GMAP_SHADOW_FAKE_TABLE;
+	s_pgt = page_to_phys(ptdesc_page(ptdesc));
 	/* Install shadow page table */
 	spin_lock(&sg->guest_table_lock);
 	table = gmap_table_walk(sg, saddr, 1); /* get segment pointer */
@@ -2088,7 +2090,7 @@ int gmap_shadow_pgt(struct gmap *sg, unsigned long saddr, unsigned long pgt,
 	/* mark as invalid as long as the parent table is not protected */
 	*table = (unsigned long) s_pgt | _SEGMENT_ENTRY |
 		 (pgt & _SEGMENT_ENTRY_PROTECT) | _SEGMENT_ENTRY_INVALID;
-	list_add(&page->lru, &sg->pt_list);
+	list_add(&ptdesc->pt_list, &sg->pt_list);
 	if (fake) {
 		/* nothing to protect for fake tables */
 		*table &= ~_SEGMENT_ENTRY_INVALID;
@@ -2114,7 +2116,7 @@ int gmap_shadow_pgt(struct gmap *sg, unsigned long saddr, unsigned long pgt,
 	return rc;
 out_free:
 	spin_unlock(&sg->guest_table_lock);
-	page_table_free_pgste(page);
+	page_table_free_pgste(ptdesc);
 	return rc;
 
 }
diff --git a/arch/s390/mm/pgalloc.c b/arch/s390/mm/pgalloc.c
index 008e487c94a6..abb629d7e131 100644
--- a/arch/s390/mm/pgalloc.c
+++ b/arch/s390/mm/pgalloc.c
@@ -135,7 +135,7 @@ int crst_table_upgrade(struct mm_struct *mm, unsigned long end)
 
 #ifdef CONFIG_PGSTE
 
-struct page *page_table_alloc_pgste(struct mm_struct *mm)
+struct ptdesc *page_table_alloc_pgste(struct mm_struct *mm)
 {
 	struct ptdesc *ptdesc;
 	u64 *table;
@@ -147,12 +147,12 @@ struct page *page_table_alloc_pgste(struct mm_struct *mm)
 		memset64(table, _PAGE_INVALID, PTRS_PER_PTE);
 		memset64(table + PTRS_PER_PTE, 0, PTRS_PER_PTE);
 	}
-	return ptdesc_page(ptdesc);
+	return ptdesc;
 }
 
-void page_table_free_pgste(struct page *page)
+void page_table_free_pgste(struct ptdesc *ptdesc)
 {
-	pagetable_free(page_ptdesc(page));
+	pagetable_free(ptdesc);
 }
 
 #endif /* CONFIG_PGSTE */
-- 
2.30.2

Re: [PATCH v2 3/3] s390: supplement for ptdesc conversion

[Heiko Carstens]

On Tue, Mar 05, 2024 at 03:21:54PM +0800, Qi Zheng wrote:
> After commit 6326c26c1514 ("s390: convert various pgalloc functions to use
> ptdescs"), there are still some positions that use page->{lru, index}
> instead of ptdesc->{pt_list, pt_index}. In order to make the use of
> ptdesc->{pt_list, pt_index} clearer, it would be better to convert them
> as well.
> 
> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
> Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
> Cc: Janosch Frank <frankja@linux.ibm.com>
> Cc: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: kvm@vger.kernel.org
> Cc: linux-s390@vger.kernel.org
> ---
> v1 -> v2: fix build failure (cross compilation successful)
> 
>  arch/s390/include/asm/pgalloc.h |  4 ++--
>  arch/s390/mm/gmap.c             | 38 +++++++++++++++++----------------
>  arch/s390/mm/pgalloc.c          |  8 +++----
>  3 files changed, 26 insertions(+), 24 deletions(-)

Same here: Christian, Janosch, or Claudio, please have a look and
provide an ACK if this is ok with you.

Re: [PATCH 3/3] s390: supplement for ptdesc conversion

[kernel test robot]

Hi Qi,

kernel test robot noticed the following build errors:

[auto build test ERROR on s390/features]
[also build test ERROR on kvms390/next linus/master v6.8-rc7]
[cannot apply to akpm-mm/mm-everything next-20240305]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Qi-Zheng/mm-pgtable-correct-the-wrong-comment-about-ptdesc-__page_flags/20240304-191006
base:   https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git features
patch link:    https://lore.kernel.org/r/04beaf3255056ffe131a5ea595736066c1e84756.1709541697.git.zhengqi.arch%40bytedance.com
patch subject: [PATCH 3/3] s390: supplement for ptdesc conversion
config: s390-allyesconfig (https://download.01.org/0day-ci/archive/20240306/202403060651.ld11yLEn-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240306/202403060651.ld11yLEn-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403060651.ld11yLEn-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/linux/smp.h:12,
                    from include/linux/lockdep.h:14,
                    from include/linux/spinlock.h:63,
                    from include/linux/mmzone.h:8,
                    from include/linux/gfp.h:7,
                    from include/linux/mm.h:7,
                    from include/linux/pagewalk.h:5,
                    from arch/s390/mm/gmap.c:12:
   arch/s390/mm/gmap.c: In function 'gmap_free':
>> include/linux/list.h:866:19: error: assignment to 'struct page *' from incompatible pointer type 'struct ptdesc *' [-Werror=incompatible-pointer-types]
     866 |                 n = list_next_entry(pos, member);                       \
         |                   ^
   arch/s390/mm/gmap.c:212:17: note: in expansion of macro 'list_for_each_entry_safe'
     212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~
>> include/linux/list.h:868:18: error: assignment to 'struct ptdesc *' from incompatible pointer type 'struct page *' [-Werror=incompatible-pointer-types]
     868 |              pos = n, n = list_next_entry(n, member))
         |                  ^
   arch/s390/mm/gmap.c:212:17: note: in expansion of macro 'list_for_each_entry_safe'
     212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/kernel.h:22,
                    from arch/s390/mm/gmap.c:11:
>> arch/s390/mm/gmap.c:212:72: error: 'struct page' has no member named 'pt_list'; did you mean 'pcp_list'?
     212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
         |                                                                        ^~~~~~~
   include/linux/container_of.h:19:33: note: in definition of macro 'container_of'
      19 |         void *__mptr = (void *)(ptr);                                   \
         |                                 ^~~
   include/linux/list.h:645:9: note: in expansion of macro 'list_entry'
     645 |         list_entry((pos)->member.next, typeof(*(pos)), member)
         |         ^~~~~~~~~~
   include/linux/list.h:868:27: note: in expansion of macro 'list_next_entry'
     868 |              pos = n, n = list_next_entry(n, member))
         |                           ^~~~~~~~~~~~~~~
   arch/s390/mm/gmap.c:212:17: note: in expansion of macro 'list_for_each_entry_safe'
     212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/container_of.h:5:
>> arch/s390/mm/gmap.c:212:72: error: 'struct page' has no member named 'pt_list'; did you mean 'pcp_list'?
     212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
         |                                                                        ^~~~~~~
   include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
      78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
         |                                                        ^~~~
   include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert'
      20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
         |         ^~~~~~~~~~~~~
   include/linux/container_of.h:20:23: note: in expansion of macro '__same_type'
      20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
         |                       ^~~~~~~~~~~
   include/linux/list.h:601:9: note: in expansion of macro 'container_of'
     601 |         container_of(ptr, type, member)
         |         ^~~~~~~~~~~~
   include/linux/list.h:645:9: note: in expansion of macro 'list_entry'
     645 |         list_entry((pos)->member.next, typeof(*(pos)), member)
         |         ^~~~~~~~~~
   include/linux/list.h:868:27: note: in expansion of macro 'list_next_entry'
     868 |              pos = n, n = list_next_entry(n, member))
         |                           ^~~~~~~~~~~~~~~
   arch/s390/mm/gmap.c:212:17: note: in expansion of macro 'list_for_each_entry_safe'
     212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~
>> arch/s390/mm/gmap.c:212:72: error: 'struct page' has no member named 'pt_list'; did you mean 'pcp_list'?
     212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
         |                                                                        ^~~~~~~
   include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
      78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
         |                                                        ^~~~
   include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert'
      20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
         |         ^~~~~~~~~~~~~
   include/linux/container_of.h:20:23: note: in expansion of macro '__same_type'
      20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
         |                       ^~~~~~~~~~~
   include/linux/list.h:601:9: note: in expansion of macro 'container_of'
     601 |         container_of(ptr, type, member)
         |         ^~~~~~~~~~~~
   include/linux/list.h:645:9: note: in expansion of macro 'list_entry'
     645 |         list_entry((pos)->member.next, typeof(*(pos)), member)
         |         ^~~~~~~~~~
   include/linux/list.h:868:27: note: in expansion of macro 'list_next_entry'
     868 |              pos = n, n = list_next_entry(n, member))
         |                           ^~~~~~~~~~~~~~~
   arch/s390/mm/gmap.c:212:17: note: in expansion of macro 'list_for_each_entry_safe'
     212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~
>> arch/s390/mm/gmap.c:212:72: error: 'struct page' has no member named 'pt_list'; did you mean 'pcp_list'?
     212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
         |                                                                        ^~~~~~~
   include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
      78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
         |                                                        ^~~~
   include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert'
      20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
         |         ^~~~~~~~~~~~~
   include/linux/container_of.h:21:23: note: in expansion of macro '__same_type'
      21 |                       __same_type(*(ptr), void),                        \
         |                       ^~~~~~~~~~~
   include/linux/list.h:601:9: note: in expansion of macro 'container_of'
     601 |         container_of(ptr, type, member)
         |         ^~~~~~~~~~~~
   include/linux/list.h:645:9: note: in expansion of macro 'list_entry'
     645 |         list_entry((pos)->member.next, typeof(*(pos)), member)
         |         ^~~~~~~~~~
   include/linux/list.h:868:27: note: in expansion of macro 'list_next_entry'
     868 |              pos = n, n = list_next_entry(n, member))
         |                           ^~~~~~~~~~~~~~~
   arch/s390/mm/gmap.c:212:17: note: in expansion of macro 'list_for_each_entry_safe'
     212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:376:27: error: expression in static assertion is not an integer
     376 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
         |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
      78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
         |                                                        ^~~~
   include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert'
      20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
         |         ^~~~~~~~~~~~~
   include/linux/container_of.h:20:23: note: in expansion of macro '__same_type'
      20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
         |                       ^~~~~~~~~~~
   include/linux/list.h:601:9: note: in expansion of macro 'container_of'
     601 |         container_of(ptr, type, member)
         |         ^~~~~~~~~~~~
   include/linux/list.h:645:9: note: in expansion of macro 'list_entry'
     645 |         list_entry((pos)->member.next, typeof(*(pos)), member)
         |         ^~~~~~~~~~
   include/linux/list.h:868:27: note: in expansion of macro 'list_next_entry'
     868 |              pos = n, n = list_next_entry(n, member))
         |                           ^~~~~~~~~~~~~~~
   arch/s390/mm/gmap.c:212:17: note: in expansion of macro 'list_for_each_entry_safe'
     212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/uapi/linux/posix_types.h:5,
                    from include/uapi/linux/types.h:14,
                    from include/linux/types.h:6,
                    from include/linux/kasan-checks.h:5,
                    from include/asm-generic/rwonce.h:26,
                    from arch/s390/include/asm/rwonce.h:29,
                    from include/linux/compiler.h:251,
                    from include/linux/array_size.h:5,
                    from include/linux/kernel.h:16:
>> arch/s390/mm/gmap.c:212:72: error: 'struct page' has no member named 'pt_list'; did you mean 'pcp_list'?
     212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
         |                                                                        ^~~~~~~
   include/linux/stddef.h:16:58: note: in definition of macro 'offsetof'
      16 | #define offsetof(TYPE, MEMBER)  __builtin_offsetof(TYPE, MEMBER)
         |                                                          ^~~~~~
   include/linux/list.h:601:9: note: in expansion of macro 'container_of'
     601 |         container_of(ptr, type, member)
         |         ^~~~~~~~~~~~
   include/linux/list.h:645:9: note: in expansion of macro 'list_entry'
     645 |         list_entry((pos)->member.next, typeof(*(pos)), member)
         |         ^~~~~~~~~~
   include/linux/list.h:868:27: note: in expansion of macro 'list_next_entry'
     868 |              pos = n, n = list_next_entry(n, member))
         |                           ^~~~~~~~~~~~~~~
   arch/s390/mm/gmap.c:212:17: note: in expansion of macro 'list_for_each_entry_safe'
     212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +212 arch/s390/mm/gmap.c

   187	
   188	/**
   189	 * gmap_free - free a guest address space
   190	 * @gmap: pointer to the guest address space structure
   191	 *
   192	 * No locks required. There are no references to this gmap anymore.
   193	 */
   194	static void gmap_free(struct gmap *gmap)
   195	{
   196		struct page *page, *next;
   197	
   198		/* Flush tlb of all gmaps (if not already done for shadows) */
   199		if (!(gmap_is_shadow(gmap) && gmap->removed))
   200			gmap_flush_tlb(gmap);
   201		/* Free all segment & region tables. */
   202		list_for_each_entry_safe(page, next, &gmap->crst_list, lru)
   203			__free_pages(page, CRST_ALLOC_ORDER);
   204		gmap_radix_tree_free(&gmap->guest_to_host);
   205		gmap_radix_tree_free(&gmap->host_to_guest);
   206	
   207		/* Free additional data for a shadow gmap */
   208		if (gmap_is_shadow(gmap)) {
   209			struct ptdesc *ptdesc;
   210	
   211			/* Free all page tables. */
 > 212			list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
   213				page_table_free_pgste(ptdesc);
   214			gmap_rmap_radix_tree_free(&gmap->host_to_rmap);
   215			/* Release reference to the parent */
   216			gmap_put(gmap->parent);
   217		}
   218	
   219		kfree(gmap);
   220	}
   221	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 3/3] s390: supplement for ptdesc conversion

[Qi Zheng]

Hi,

Thanks a lot for testing! And the following errors have been fixed by
https://lore.kernel.org/lkml/20240305072154.26168-1-zhengqi.arch@bytedance.com/.

Thanks,
Qi

On 2024/3/6 06:32, kernel test robot wrote:
> Hi Qi,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on s390/features]
> [also build test ERROR on kvms390/next linus/master v6.8-rc7]
> [cannot apply to akpm-mm/mm-everything next-20240305]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Qi-Zheng/mm-pgtable-correct-the-wrong-comment-about-ptdesc-__page_flags/20240304-191006
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git features
> patch link:    https://lore.kernel.org/r/04beaf3255056ffe131a5ea595736066c1e84756.1709541697.git.zhengqi.arch%40bytedance.com
> patch subject: [PATCH 3/3] s390: supplement for ptdesc conversion
> config: s390-allyesconfig (https://download.01.org/0day-ci/archive/20240306/202403060651.ld11yLEn-lkp@intel.com/config)
> compiler: s390-linux-gcc (GCC) 13.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240306/202403060651.ld11yLEn-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202403060651.ld11yLEn-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
>     In file included from include/linux/smp.h:12,
>                      from include/linux/lockdep.h:14,
>                      from include/linux/spinlock.h:63,
>                      from include/linux/mmzone.h:8,
>                      from include/linux/gfp.h:7,
>                      from include/linux/mm.h:7,
>                      from include/linux/pagewalk.h:5,
>                      from arch/s390/mm/gmap.c:12:
>     arch/s390/mm/gmap.c: In function 'gmap_free':
>>> include/linux/list.h:866:19: error: assignment to 'struct page *' from incompatible pointer type 'struct ptdesc *' [-Werror=incompatible-pointer-types]
>       866 |                 n = list_next_entry(pos, member);                       \
>           |                   ^
>     arch/s390/mm/gmap.c:212:17: note: in expansion of macro 'list_for_each_entry_safe'
>       212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
>           |                 ^~~~~~~~~~~~~~~~~~~~~~~~
>>> include/linux/list.h:868:18: error: assignment to 'struct ptdesc *' from incompatible pointer type 'struct page *' [-Werror=incompatible-pointer-types]
>       868 |              pos = n, n = list_next_entry(n, member))
>           |                  ^
>     arch/s390/mm/gmap.c:212:17: note: in expansion of macro 'list_for_each_entry_safe'
>       212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
>           |                 ^~~~~~~~~~~~~~~~~~~~~~~~
>     In file included from include/linux/kernel.h:22,
>                      from arch/s390/mm/gmap.c:11:
>>> arch/s390/mm/gmap.c:212:72: error: 'struct page' has no member named 'pt_list'; did you mean 'pcp_list'?
>       212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
>           |                                                                        ^~~~~~~
>     include/linux/container_of.h:19:33: note: in definition of macro 'container_of'
>        19 |         void *__mptr = (void *)(ptr);                                   \
>           |                                 ^~~
>     include/linux/list.h:645:9: note: in expansion of macro 'list_entry'
>       645 |         list_entry((pos)->member.next, typeof(*(pos)), member)
>           |         ^~~~~~~~~~
>     include/linux/list.h:868:27: note: in expansion of macro 'list_next_entry'
>       868 |              pos = n, n = list_next_entry(n, member))
>           |                           ^~~~~~~~~~~~~~~
>     arch/s390/mm/gmap.c:212:17: note: in expansion of macro 'list_for_each_entry_safe'
>       212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
>           |                 ^~~~~~~~~~~~~~~~~~~~~~~~
>     In file included from include/linux/container_of.h:5:
>>> arch/s390/mm/gmap.c:212:72: error: 'struct page' has no member named 'pt_list'; did you mean 'pcp_list'?
>       212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
>           |                                                                        ^~~~~~~
>     include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
>        78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
>           |                                                        ^~~~
>     include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert'
>        20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
>           |         ^~~~~~~~~~~~~
>     include/linux/container_of.h:20:23: note: in expansion of macro '__same_type'
>        20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
>           |                       ^~~~~~~~~~~
>     include/linux/list.h:601:9: note: in expansion of macro 'container_of'
>       601 |         container_of(ptr, type, member)
>           |         ^~~~~~~~~~~~
>     include/linux/list.h:645:9: note: in expansion of macro 'list_entry'
>       645 |         list_entry((pos)->member.next, typeof(*(pos)), member)
>           |         ^~~~~~~~~~
>     include/linux/list.h:868:27: note: in expansion of macro 'list_next_entry'
>       868 |              pos = n, n = list_next_entry(n, member))
>           |                           ^~~~~~~~~~~~~~~
>     arch/s390/mm/gmap.c:212:17: note: in expansion of macro 'list_for_each_entry_safe'
>       212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
>           |                 ^~~~~~~~~~~~~~~~~~~~~~~~
>>> arch/s390/mm/gmap.c:212:72: error: 'struct page' has no member named 'pt_list'; did you mean 'pcp_list'?
>       212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
>           |                                                                        ^~~~~~~
>     include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
>        78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
>           |                                                        ^~~~
>     include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert'
>        20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
>           |         ^~~~~~~~~~~~~
>     include/linux/container_of.h:20:23: note: in expansion of macro '__same_type'
>        20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
>           |                       ^~~~~~~~~~~
>     include/linux/list.h:601:9: note: in expansion of macro 'container_of'
>       601 |         container_of(ptr, type, member)
>           |         ^~~~~~~~~~~~
>     include/linux/list.h:645:9: note: in expansion of macro 'list_entry'
>       645 |         list_entry((pos)->member.next, typeof(*(pos)), member)
>           |         ^~~~~~~~~~
>     include/linux/list.h:868:27: note: in expansion of macro 'list_next_entry'
>       868 |              pos = n, n = list_next_entry(n, member))
>           |                           ^~~~~~~~~~~~~~~
>     arch/s390/mm/gmap.c:212:17: note: in expansion of macro 'list_for_each_entry_safe'
>       212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
>           |                 ^~~~~~~~~~~~~~~~~~~~~~~~
>>> arch/s390/mm/gmap.c:212:72: error: 'struct page' has no member named 'pt_list'; did you mean 'pcp_list'?
>       212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
>           |                                                                        ^~~~~~~
>     include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
>        78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
>           |                                                        ^~~~
>     include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert'
>        20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
>           |         ^~~~~~~~~~~~~
>     include/linux/container_of.h:21:23: note: in expansion of macro '__same_type'
>        21 |                       __same_type(*(ptr), void),                        \
>           |                       ^~~~~~~~~~~
>     include/linux/list.h:601:9: note: in expansion of macro 'container_of'
>       601 |         container_of(ptr, type, member)
>           |         ^~~~~~~~~~~~
>     include/linux/list.h:645:9: note: in expansion of macro 'list_entry'
>       645 |         list_entry((pos)->member.next, typeof(*(pos)), member)
>           |         ^~~~~~~~~~
>     include/linux/list.h:868:27: note: in expansion of macro 'list_next_entry'
>       868 |              pos = n, n = list_next_entry(n, member))
>           |                           ^~~~~~~~~~~~~~~
>     arch/s390/mm/gmap.c:212:17: note: in expansion of macro 'list_for_each_entry_safe'
>       212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
>           |                 ^~~~~~~~~~~~~~~~~~~~~~~~
>     include/linux/compiler_types.h:376:27: error: expression in static assertion is not an integer
>       376 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
>           |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
>        78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
>           |                                                        ^~~~
>     include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert'
>        20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
>           |         ^~~~~~~~~~~~~
>     include/linux/container_of.h:20:23: note: in expansion of macro '__same_type'
>        20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
>           |                       ^~~~~~~~~~~
>     include/linux/list.h:601:9: note: in expansion of macro 'container_of'
>       601 |         container_of(ptr, type, member)
>           |         ^~~~~~~~~~~~
>     include/linux/list.h:645:9: note: in expansion of macro 'list_entry'
>       645 |         list_entry((pos)->member.next, typeof(*(pos)), member)
>           |         ^~~~~~~~~~
>     include/linux/list.h:868:27: note: in expansion of macro 'list_next_entry'
>       868 |              pos = n, n = list_next_entry(n, member))
>           |                           ^~~~~~~~~~~~~~~
>     arch/s390/mm/gmap.c:212:17: note: in expansion of macro 'list_for_each_entry_safe'
>       212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
>           |                 ^~~~~~~~~~~~~~~~~~~~~~~~
>     In file included from include/uapi/linux/posix_types.h:5,
>                      from include/uapi/linux/types.h:14,
>                      from include/linux/types.h:6,
>                      from include/linux/kasan-checks.h:5,
>                      from include/asm-generic/rwonce.h:26,
>                      from arch/s390/include/asm/rwonce.h:29,
>                      from include/linux/compiler.h:251,
>                      from include/linux/array_size.h:5,
>                      from include/linux/kernel.h:16:
>>> arch/s390/mm/gmap.c:212:72: error: 'struct page' has no member named 'pt_list'; did you mean 'pcp_list'?
>       212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
>           |                                                                        ^~~~~~~
>     include/linux/stddef.h:16:58: note: in definition of macro 'offsetof'
>        16 | #define offsetof(TYPE, MEMBER)  __builtin_offsetof(TYPE, MEMBER)
>           |                                                          ^~~~~~
>     include/linux/list.h:601:9: note: in expansion of macro 'container_of'
>       601 |         container_of(ptr, type, member)
>           |         ^~~~~~~~~~~~
>     include/linux/list.h:645:9: note: in expansion of macro 'list_entry'
>       645 |         list_entry((pos)->member.next, typeof(*(pos)), member)
>           |         ^~~~~~~~~~
>     include/linux/list.h:868:27: note: in expansion of macro 'list_next_entry'
>       868 |              pos = n, n = list_next_entry(n, member))
>           |                           ^~~~~~~~~~~~~~~
>     arch/s390/mm/gmap.c:212:17: note: in expansion of macro 'list_for_each_entry_safe'
>       212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
>           |                 ^~~~~~~~~~~~~~~~~~~~~~~~
>     cc1: some warnings being treated as errors
> 
> 
> vim +212 arch/s390/mm/gmap.c
> 
>     187	
>     188	/**
>     189	 * gmap_free - free a guest address space
>     190	 * @gmap: pointer to the guest address space structure
>     191	 *
>     192	 * No locks required. There are no references to this gmap anymore.
>     193	 */
>     194	static void gmap_free(struct gmap *gmap)
>     195	{
>     196		struct page *page, *next;
>     197	
>     198		/* Flush tlb of all gmaps (if not already done for shadows) */
>     199		if (!(gmap_is_shadow(gmap) && gmap->removed))
>     200			gmap_flush_tlb(gmap);
>     201		/* Free all segment & region tables. */
>     202		list_for_each_entry_safe(page, next, &gmap->crst_list, lru)
>     203			__free_pages(page, CRST_ALLOC_ORDER);
>     204		gmap_radix_tree_free(&gmap->guest_to_host);
>     205		gmap_radix_tree_free(&gmap->host_to_guest);
>     206	
>     207		/* Free additional data for a shadow gmap */
>     208		if (gmap_is_shadow(gmap)) {
>     209			struct ptdesc *ptdesc;
>     210	
>     211			/* Free all page tables. */
>   > 212			list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
>     213				page_table_free_pgste(ptdesc);
>     214			gmap_rmap_radix_tree_free(&gmap->host_to_rmap);
>     215			/* Release reference to the parent */
>     216			gmap_put(gmap->parent);
>     217		}
>     218	
>     219		kfree(gmap);
>     220	}
>     221	
> 

Re: [PATCH 3/3] s390: supplement for ptdesc conversion

[kernel test robot]

Hi Qi,

kernel test robot noticed the following build errors:

[auto build test ERROR on s390/features]
[also build test ERROR on kvms390/next linus/master v6.8-rc7]
[cannot apply to akpm-mm/mm-everything next-20240305]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Qi-Zheng/mm-pgtable-correct-the-wrong-comment-about-ptdesc-__page_flags/20240304-191006
base:   https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git features
patch link:    https://lore.kernel.org/r/04beaf3255056ffe131a5ea595736066c1e84756.1709541697.git.zhengqi.arch%40bytedance.com
patch subject: [PATCH 3/3] s390: supplement for ptdesc conversion
config: s390-defconfig (https://download.01.org/0day-ci/archive/20240306/202403061249.4URm4Yxi-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 325f51237252e6dab8e4e1ea1fa7acbb4faee1cd)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240306/202403061249.4URm4Yxi-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403061249.4URm4Yxi-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from arch/s390/mm/gmap.c:12:
   In file included from include/linux/pagewalk.h:5:
   In file included from include/linux/mm.h:2188:
   include/linux/vmstat.h:508:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     508 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     509 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:515:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     515 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     516 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:522:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     522 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   include/linux/vmstat.h:527:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     527 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     528 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:536:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     536 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     537 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
>> arch/s390/mm/gmap.c:212:3: error: incompatible pointer types assigning to 'struct page *' from 'typeof (*(ptdesc)) *' (aka 'struct ptdesc *') [-Werror,-Wincompatible-pointer-types]
     212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/list.h:866:5: note: expanded from macro 'list_for_each_entry_safe'
     866 |                 n = list_next_entry(pos, member);                       \
         |                   ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> arch/s390/mm/gmap.c:212:3: error: incompatible pointer types assigning to 'struct ptdesc *' from 'struct page *' [-Werror,-Wincompatible-pointer-types]
     212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
         |                 ^                                ~~~~
   include/linux/list.h:868:11: note: expanded from macro 'list_for_each_entry_safe'
     868 |              pos = n, n = list_next_entry(n, member))
         |                  ^ ~
>> arch/s390/mm/gmap.c:212:58: error: no member named 'pt_list' in 'struct page'; did you mean 'pcp_list'?
     212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
         |                                                                        ^~~~~~~
         |                                                                        pcp_list
   include/linux/list.h:868:39: note: expanded from macro 'list_for_each_entry_safe'
     868 |              pos = n, n = list_next_entry(n, member))
         |                                              ^
   include/linux/list.h:645:20: note: expanded from macro 'list_next_entry'
     645 |         list_entry((pos)->member.next, typeof(*(pos)), member)
         |                           ^
   include/linux/list.h:601:15: note: expanded from macro 'list_entry'
     601 |         container_of(ptr, type, member)
         |                      ^
   include/linux/container_of.h:19:26: note: expanded from macro 'container_of'
      19 |         void *__mptr = (void *)(ptr);                                   \
         |                                 ^
   include/linux/mm_types.h:103:22: note: 'pcp_list' declared here
     103 |                                 struct list_head pcp_list;
         |                                                  ^
>> arch/s390/mm/gmap.c:212:58: error: no member named 'pt_list' in 'struct page'; did you mean 'pcp_list'?
     212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
         |                                                                        ^~~~~~~
         |                                                                        pcp_list
   include/linux/list.h:868:39: note: expanded from macro 'list_for_each_entry_safe'
     868 |              pos = n, n = list_next_entry(n, member))
         |                                              ^
   include/linux/list.h:645:20: note: expanded from macro 'list_next_entry'
     645 |         list_entry((pos)->member.next, typeof(*(pos)), member)
         |                           ^
   include/linux/list.h:601:15: note: expanded from macro 'list_entry'
     601 |         container_of(ptr, type, member)
         |                      ^
   note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:376:63: note: expanded from macro '__same_type'
     376 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
         |                                                               ^
   include/linux/build_bug.h:77:50: note: expanded from macro 'static_assert'
      77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
         |                                                  ^
   include/linux/build_bug.h:78:56: note: expanded from macro '__static_assert'
      78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
         |                                                        ^
   include/linux/mm_types.h:103:22: note: 'pcp_list' declared here
     103 |                                 struct list_head pcp_list;
         |                                                  ^
>> arch/s390/mm/gmap.c:212:58: error: no member named 'pt_list' in 'struct page'; did you mean 'pcp_list'?
     212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
         |                                                                        ^~~~~~~
         |                                                                        pcp_list
   include/linux/list.h:868:39: note: expanded from macro 'list_for_each_entry_safe'
     868 |              pos = n, n = list_next_entry(n, member))
         |                                              ^
   include/linux/list.h:645:49: note: expanded from macro 'list_next_entry'
     645 |         list_entry((pos)->member.next, typeof(*(pos)), member)
         |                                                        ^
   include/linux/list.h:601:26: note: expanded from macro 'list_entry'
     601 |         container_of(ptr, type, member)
         |                                 ^
   note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:376:74: note: expanded from macro '__same_type'
     376 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
         |                                                                          ^
   include/linux/build_bug.h:77:50: note: expanded from macro 'static_assert'
      77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
         |                                                  ^
   include/linux/build_bug.h:78:56: note: expanded from macro '__static_assert'
      78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
         |                                                        ^
   include/linux/mm_types.h:103:22: note: 'pcp_list' declared here
     103 |                                 struct list_head pcp_list;
         |                                                  ^
>> arch/s390/mm/gmap.c:212:58: error: no member named 'pt_list' in 'struct page'; did you mean 'pcp_list'?
     212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
         |                                                                        ^~~~~~~
         |                                                                        pcp_list
   include/linux/list.h:868:39: note: expanded from macro 'list_for_each_entry_safe'
     868 |              pos = n, n = list_next_entry(n, member))
         |                                              ^
   include/linux/list.h:645:20: note: expanded from macro 'list_next_entry'
     645 |         list_entry((pos)->member.next, typeof(*(pos)), member)
         |                           ^
   include/linux/list.h:601:15: note: expanded from macro 'list_entry'
     601 |         container_of(ptr, type, member)
         |                      ^
   note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:376:63: note: expanded from macro '__same_type'
     376 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
         |                                                               ^
   include/linux/build_bug.h:77:50: note: expanded from macro 'static_assert'
      77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
         |                                                  ^
   include/linux/build_bug.h:78:56: note: expanded from macro '__static_assert'
      78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
         |                                                        ^
   include/linux/mm_types.h:103:22: note: 'pcp_list' declared here
     103 |                                 struct list_head pcp_list;
         |                                                  ^
>> arch/s390/mm/gmap.c:212:3: error: no member named 'pt_list' in 'page'
     212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
         |                 ^                                                      ~~~~~~~
   include/linux/list.h:868:20: note: expanded from macro 'list_for_each_entry_safe'
     868 |              pos = n, n = list_next_entry(n, member))
         |                           ^                  ~~~~~~
   include/linux/list.h:645:2: note: expanded from macro 'list_next_entry'
     645 |         list_entry((pos)->member.next, typeof(*(pos)), member)
         |         ^                                              ~~~~~~
   include/linux/list.h:601:2: note: expanded from macro 'list_entry'
     601 |         container_of(ptr, type, member)
         |         ^                       ~~~~~~
   include/linux/container_of.h:23:21: note: expanded from macro 'container_of'
      23 |         ((type *)(__mptr - offsetof(type, member))); })
         |                            ^              ~~~~~~
   include/linux/stddef.h:16:32: note: expanded from macro 'offsetof'
      16 | #define offsetof(TYPE, MEMBER)  __builtin_offsetof(TYPE, MEMBER)
         |                                 ^                        ~~~~~~
>> arch/s390/mm/gmap.c:212:3: error: assigning to 'struct page *' from incompatible type 'void'
     212 |                 list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/list.h:868:18: note: expanded from macro 'list_for_each_entry_safe'
     868 |              pos = n, n = list_next_entry(n, member))
         |                         ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~
   5 warnings and 8 errors generated.


vim +212 arch/s390/mm/gmap.c

   187	
   188	/**
   189	 * gmap_free - free a guest address space
   190	 * @gmap: pointer to the guest address space structure
   191	 *
   192	 * No locks required. There are no references to this gmap anymore.
   193	 */
   194	static void gmap_free(struct gmap *gmap)
   195	{
   196		struct page *page, *next;
   197	
   198		/* Flush tlb of all gmaps (if not already done for shadows) */
   199		if (!(gmap_is_shadow(gmap) && gmap->removed))
   200			gmap_flush_tlb(gmap);
   201		/* Free all segment & region tables. */
   202		list_for_each_entry_safe(page, next, &gmap->crst_list, lru)
   203			__free_pages(page, CRST_ALLOC_ORDER);
   204		gmap_radix_tree_free(&gmap->guest_to_host);
   205		gmap_radix_tree_free(&gmap->host_to_guest);
   206	
   207		/* Free additional data for a shadow gmap */
   208		if (gmap_is_shadow(gmap)) {
   209			struct ptdesc *ptdesc;
   210	
   211			/* Free all page tables. */
 > 212			list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
   213				page_table_free_pgste(ptdesc);
   214			gmap_rmap_radix_tree_free(&gmap->host_to_rmap);
   215			/* Release reference to the parent */
   216			gmap_put(gmap->parent);
   217		}
   218	
   219		kfree(gmap);
   220	}
   221	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 3/3] s390: supplement for ptdesc conversion

[Vishal Moola]

On Mon, Mar 04, 2024 at 07:07:20PM +0800, Qi Zheng wrote:
> --- a/arch/s390/mm/gmap.c
> +++ b/arch/s390/mm/gmap.c
> @@ -206,9 +206,11 @@ static void gmap_free(struct gmap *gmap)
>  
>  	/* Free additional data for a shadow gmap */
>  	if (gmap_is_shadow(gmap)) {
> +		struct ptdesc *ptdesc;
> +
>  		/* Free all page tables. */
> -		list_for_each_entry_safe(page, next, &gmap->pt_list, lru)
> -			page_table_free_pgste(page);
> +		list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
> +			page_table_free_pgste(ptdesc);

An important note: ptdesc allocation/freeing is different than the
standard alloc_pages()/free_pages() routines architectures are used to.
Are we sure we don't have memory leaks here?

We always allocate and free ptdescs as compound pages; for page table
struct pages, most archictectures do not. s390 has CRST_ALLOC_ORDER
pagetables, meaning if we free anything using the ptdesc api, we better
be sure it was allocated using the ptdesc api as well.

Like you, I don't have a s390 to test on, so hopefully some s390 expert
can chime in to let us know if we need a fix for this.

Re: [PATCH 3/3] s390: supplement for ptdesc conversion

[Qi Zheng]

On 2024/3/27 03:48, Vishal Moola wrote:
> On Mon, Mar 04, 2024 at 07:07:20PM +0800, Qi Zheng wrote:
>> --- a/arch/s390/mm/gmap.c
>> +++ b/arch/s390/mm/gmap.c
>> @@ -206,9 +206,11 @@ static void gmap_free(struct gmap *gmap)
>>   
>>   	/* Free additional data for a shadow gmap */
>>   	if (gmap_is_shadow(gmap)) {
>> +		struct ptdesc *ptdesc;
>> +
>>   		/* Free all page tables. */
>> -		list_for_each_entry_safe(page, next, &gmap->pt_list, lru)
>> -			page_table_free_pgste(page);
>> +		list_for_each_entry_safe(ptdesc, next, &gmap->pt_list, pt_list)
>> +			page_table_free_pgste(ptdesc);
> 
> An important note: ptdesc allocation/freeing is different than the
> standard alloc_pages()/free_pages() routines architectures are used to.
> Are we sure we don't have memory leaks here?
> 
> We always allocate and free ptdescs as compound pages; for page table
> struct pages, most archictectures do not. s390 has CRST_ALLOC_ORDER
> pagetables, meaning if we free anything using the ptdesc api, we better
> be sure it was allocated using the ptdesc api as well.

According to the code inspection, all ptdescs added to the pmap->pt_list
are allocated via page_table_alloc_pgste().

> 
> Like you, I don't have a s390 to test on, so hopefully some s390 expert
> can chime in to let us know if we need a fix for this.

Yes, hope so!

Re: [PATCH 0/3] minor fixes and supplement for ptdesc

[Vishal Moola]

On Mon, Mar 04, 2024 at 07:07:17PM +0800, Qi Zheng wrote:
> Hi all,

Sorry for the late review. Thanks for looking at doing some ptdesc
conversions. This patchset has the right idea and looks *mostly* fine.

> In this series, the [PATCH 1/3] and [PATCH 2/3] are fixes for some issues
> discovered during code inspection.
> 
> The [PATCH 3/3] is a supplement to ptdesc conversion in s390, I don't know
> why this is not done in the commit 6326c26c1514 ("s390: convert various pgalloc
> functions to use ptdescs"), maybe I missed something. And since I don't have an

It's important to keep in mind the end goal of ptdescs, cleaning up much
of the struct page field misuse by standardizing their usage. s390 page
tables and gmap are similar but not the same, so the conversions require
deeper thought. 

My initial "Split ptdesc from struct page" patchset tried to focus on the
most straightforward, simple conversions in order to introduce the
descriptor and lay a foundation for future conversions - you can see some
more complicated iterations v6 and prior.

When converting to ptdescs (and any other newer descriptors), we should
be careful about generating superficial code churn instead of using
them to solve the problems they are trying to solve.

Re: [PATCH 0/3] minor fixes and supplement for ptdesc

[David Hildenbrand]

On 26.03.24 20:07, Vishal Moola wrote:
> On Mon, Mar 04, 2024 at 07:07:17PM +0800, Qi Zheng wrote:
>> Hi all,
> 
> Sorry for the late review. Thanks for looking at doing some ptdesc
> conversions. This patchset has the right idea and looks *mostly* fine.
> 
>> In this series, the [PATCH 1/3] and [PATCH 2/3] are fixes for some issues
>> discovered during code inspection.
>>
>> The [PATCH 3/3] is a supplement to ptdesc conversion in s390, I don't know
>> why this is not done in the commit 6326c26c1514 ("s390: convert various pgalloc
>> functions to use ptdescs"), maybe I missed something. And since I don't have an
> 
> It's important to keep in mind the end goal of ptdescs, cleaning up much
> of the struct page field misuse by standardizing their usage. s390 page
> tables and gmap are similar but not the same, so the conversions require
> deeper thought.
> 
> My initial "Split ptdesc from struct page" patchset tried to focus on the
> most straightforward, simple conversions in order to introduce the
> descriptor and lay a foundation for future conversions - you can see some
> more complicated iterations v6 and prior.
> 
> When converting to ptdescs (and any other newer descriptors), we should
> be careful about generating superficial code churn instead of using
> them to solve the problems they are trying to solve.

The gmap shadow pages are page tables that are not linked into the user 
page tables.

I recall I raised in the past that using ptdesc from them is confusing.

-- 
Cheers,

David / dhildenb