* [PATCH 0/3] minor fixes and supplement for ptdesc
@ 2024-03-04 11:07 Qi Zheng
2024-03-04 11:07 ` [PATCH 1/3] mm: pgtable: correct the wrong comment about ptdesc->__page_flags Qi Zheng
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Qi Zheng @ 2024-03-04 11:07 UTC (permalink / raw
To: akpm, vishal.moola, hughd, david, rppt, willy, muchun.song
Cc: linux-mm, linux-kernel, 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] mm: pgtable: correct the wrong comment about ptdesc->__page_flags
2024-03-04 11:07 [PATCH 0/3] minor fixes and supplement for ptdesc Qi Zheng
@ 2024-03-04 11:07 ` Qi Zheng
2024-03-26 19:12 ` Vishal Moola
2024-03-04 11:07 ` [PATCH 2/3] mm: pgtable: add missing pt_index to struct ptdesc Qi Zheng
` (2 subsequent siblings)
3 siblings, 1 reply; 17+ messages in thread
From: Qi Zheng @ 2024-03-04 11:07 UTC (permalink / raw
To: akpm, vishal.moola, hughd, david, rppt, willy, muchun.song
Cc: linux-mm, linux-kernel, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/3] mm: pgtable: add missing pt_index to struct ptdesc
2024-03-04 11:07 [PATCH 0/3] minor fixes and supplement for ptdesc Qi Zheng
2024-03-04 11:07 ` [PATCH 1/3] mm: pgtable: correct the wrong comment about ptdesc->__page_flags Qi Zheng
@ 2024-03-04 11:07 ` Qi Zheng
2024-03-26 19:25 ` Vishal Moola
2024-03-04 11:07 ` [PATCH 3/3] s390: supplement for ptdesc conversion Qi Zheng
2024-03-26 19:07 ` [PATCH 0/3] minor fixes and supplement for ptdesc Vishal Moola
3 siblings, 1 reply; 17+ messages in thread
From: Qi Zheng @ 2024-03-04 11:07 UTC (permalink / raw
To: akpm, vishal.moola, hughd, david, rppt, willy, muchun.song
Cc: linux-mm, linux-kernel, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/3] s390: supplement for ptdesc conversion
2024-03-04 11:07 [PATCH 0/3] minor fixes and supplement for ptdesc Qi Zheng
2024-03-04 11:07 ` [PATCH 1/3] mm: pgtable: correct the wrong comment about ptdesc->__page_flags Qi Zheng
2024-03-04 11:07 ` [PATCH 2/3] mm: pgtable: add missing pt_index to struct ptdesc Qi Zheng
@ 2024-03-04 11:07 ` Qi Zheng
2024-03-05 7:21 ` [PATCH v2 " Qi Zheng
` (3 more replies)
2024-03-26 19:07 ` [PATCH 0/3] minor fixes and supplement for ptdesc Vishal Moola
3 siblings, 4 replies; 17+ messages in thread
From: Qi Zheng @ 2024-03-04 11:07 UTC (permalink / raw
To: akpm, vishal.moola, hughd, david, rppt, willy, muchun.song
Cc: linux-mm, linux-kernel, Qi Zheng, Christian Borntraeger,
Janosch Frank, Claudio Imbrenda, kvm, linux-s390
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 3/3] s390: supplement for ptdesc conversion
2024-03-04 11:07 ` [PATCH 3/3] s390: supplement for ptdesc conversion Qi Zheng
@ 2024-03-05 7:21 ` Qi Zheng
2024-03-26 7:46 ` Heiko Carstens
2024-03-05 22:32 ` [PATCH " kernel test robot
` (2 subsequent siblings)
3 siblings, 1 reply; 17+ messages in thread
From: Qi Zheng @ 2024-03-05 7:21 UTC (permalink / raw
To: akpm, vishal.moola, hughd, david, rppt, willy, muchun.song
Cc: linux-mm, linux-kernel, Qi Zheng, Christian Borntraeger,
Janosch Frank, Claudio Imbrenda, kvm, linux-s390
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] s390: supplement for ptdesc conversion
2024-03-04 11:07 ` [PATCH 3/3] s390: supplement for ptdesc conversion Qi Zheng
2024-03-05 7:21 ` [PATCH v2 " Qi Zheng
@ 2024-03-05 22:32 ` kernel test robot
2024-03-06 2:46 ` Qi Zheng
2024-03-06 4:53 ` kernel test robot
2024-03-26 19:48 ` Vishal Moola
3 siblings, 1 reply; 17+ messages in thread
From: kernel test robot @ 2024-03-05 22:32 UTC (permalink / raw
To: Qi Zheng; +Cc: oe-kbuild-all
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] s390: supplement for ptdesc conversion
2024-03-05 22:32 ` [PATCH " kernel test robot
@ 2024-03-06 2:46 ` Qi Zheng
0 siblings, 0 replies; 17+ messages in thread
From: Qi Zheng @ 2024-03-06 2:46 UTC (permalink / raw
To: kernel test robot; +Cc: oe-kbuild-all
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
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] s390: supplement for ptdesc conversion
2024-03-04 11:07 ` [PATCH 3/3] s390: supplement for ptdesc conversion Qi Zheng
2024-03-05 7:21 ` [PATCH v2 " Qi Zheng
2024-03-05 22:32 ` [PATCH " kernel test robot
@ 2024-03-06 4:53 ` kernel test robot
2024-03-26 19:48 ` Vishal Moola
3 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2024-03-06 4:53 UTC (permalink / raw
To: Qi Zheng; +Cc: llvm, oe-kbuild-all
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/3] s390: supplement for ptdesc conversion
2024-03-05 7:21 ` [PATCH v2 " Qi Zheng
@ 2024-03-26 7:46 ` Heiko Carstens
0 siblings, 0 replies; 17+ messages in thread
From: Heiko Carstens @ 2024-03-26 7:46 UTC (permalink / raw
To: Qi Zheng
Cc: akpm, vishal.moola, hughd, david, rppt, willy, muchun.song,
linux-mm, linux-kernel, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, kvm, linux-s390, Alexander Gordeev,
Vasily Gorbik
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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] minor fixes and supplement for ptdesc
2024-03-04 11:07 [PATCH 0/3] minor fixes and supplement for ptdesc Qi Zheng
` (2 preceding siblings ...)
2024-03-04 11:07 ` [PATCH 3/3] s390: supplement for ptdesc conversion Qi Zheng
@ 2024-03-26 19:07 ` Vishal Moola
2024-03-27 8:52 ` David Hildenbrand
3 siblings, 1 reply; 17+ messages in thread
From: Vishal Moola @ 2024-03-26 19:07 UTC (permalink / raw
To: Qi Zheng
Cc: akpm, hughd, david, rppt, willy, muchun.song, linux-mm,
linux-kernel
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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] mm: pgtable: correct the wrong comment about ptdesc->__page_flags
2024-03-04 11:07 ` [PATCH 1/3] mm: pgtable: correct the wrong comment about ptdesc->__page_flags Qi Zheng
@ 2024-03-26 19:12 ` Vishal Moola
2024-03-27 2:00 ` Qi Zheng
0 siblings, 1 reply; 17+ messages in thread
From: Vishal Moola @ 2024-03-26 19:12 UTC (permalink / raw
To: Qi Zheng
Cc: akpm, hughd, david, rppt, willy, muchun.song, linux-mm,
linux-kernel
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
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] mm: pgtable: add missing pt_index to struct ptdesc
2024-03-04 11:07 ` [PATCH 2/3] mm: pgtable: add missing pt_index to struct ptdesc Qi Zheng
@ 2024-03-26 19:25 ` Vishal Moola
2024-03-27 2:06 ` Qi Zheng
0 siblings, 1 reply; 17+ messages in thread
From: Vishal Moola @ 2024-03-26 19:25 UTC (permalink / raw
To: Qi Zheng
Cc: akpm, hughd, david, rppt, willy, muchun.song, linux-mm,
linux-kernel
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
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] s390: supplement for ptdesc conversion
2024-03-04 11:07 ` [PATCH 3/3] s390: supplement for ptdesc conversion Qi Zheng
` (2 preceding siblings ...)
2024-03-06 4:53 ` kernel test robot
@ 2024-03-26 19:48 ` Vishal Moola
2024-03-27 2:11 ` Qi Zheng
3 siblings, 1 reply; 17+ messages in thread
From: Vishal Moola @ 2024-03-26 19:48 UTC (permalink / raw
To: Qi Zheng
Cc: akpm, hughd, david, rppt, willy, muchun.song, linux-mm,
linux-kernel, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, kvm, linux-s390
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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] mm: pgtable: correct the wrong comment about ptdesc->__page_flags
2024-03-26 19:12 ` Vishal Moola
@ 2024-03-27 2:00 ` Qi Zheng
0 siblings, 0 replies; 17+ messages in thread
From: Qi Zheng @ 2024-03-27 2:00 UTC (permalink / raw
To: Vishal Moola
Cc: akpm, hughd, david, rppt, willy, muchun.song, linux-mm,
linux-kernel
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
>>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] mm: pgtable: add missing pt_index to struct ptdesc
2024-03-26 19:25 ` Vishal Moola
@ 2024-03-27 2:06 ` Qi Zheng
0 siblings, 0 replies; 17+ messages in thread
From: Qi Zheng @ 2024-03-27 2:06 UTC (permalink / raw
To: Vishal Moola
Cc: akpm, hughd, david, rppt, willy, muchun.song, linux-mm,
linux-kernel
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
>>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] s390: supplement for ptdesc conversion
2024-03-26 19:48 ` Vishal Moola
@ 2024-03-27 2:11 ` Qi Zheng
0 siblings, 0 replies; 17+ messages in thread
From: Qi Zheng @ 2024-03-27 2:11 UTC (permalink / raw
To: Vishal Moola
Cc: akpm, hughd, david, rppt, willy, muchun.song, linux-mm,
linux-kernel, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, kvm, linux-s390
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!
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] minor fixes and supplement for ptdesc
2024-03-26 19:07 ` [PATCH 0/3] minor fixes and supplement for ptdesc Vishal Moola
@ 2024-03-27 8:52 ` David Hildenbrand
0 siblings, 0 replies; 17+ messages in thread
From: David Hildenbrand @ 2024-03-27 8:52 UTC (permalink / raw
To: Vishal Moola, Qi Zheng
Cc: akpm, hughd, rppt, willy, muchun.song, linux-mm, linux-kernel
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
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2024-03-27 8:52 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-04 11:07 [PATCH 0/3] minor fixes and supplement for ptdesc Qi Zheng
2024-03-04 11:07 ` [PATCH 1/3] mm: pgtable: correct the wrong comment about ptdesc->__page_flags Qi Zheng
2024-03-26 19:12 ` Vishal Moola
2024-03-27 2:00 ` Qi Zheng
2024-03-04 11:07 ` [PATCH 2/3] mm: pgtable: add missing pt_index to struct ptdesc Qi Zheng
2024-03-26 19:25 ` Vishal Moola
2024-03-27 2:06 ` Qi Zheng
2024-03-04 11:07 ` [PATCH 3/3] s390: supplement for ptdesc conversion Qi Zheng
2024-03-05 7:21 ` [PATCH v2 " Qi Zheng
2024-03-26 7:46 ` Heiko Carstens
2024-03-05 22:32 ` [PATCH " kernel test robot
2024-03-06 2:46 ` Qi Zheng
2024-03-06 4:53 ` kernel test robot
2024-03-26 19:48 ` Vishal Moola
2024-03-27 2:11 ` Qi Zheng
2024-03-26 19:07 ` [PATCH 0/3] minor fixes and supplement for ptdesc Vishal Moola
2024-03-27 8:52 ` David Hildenbrand
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.