[PATCH] mm: Fix pud_alloc_track()

Subsystems: generic include/asm header files, memory management - core, the rest

STALE2260d REVIEWED: 2 (0M)

2 review trailers.

5 messages, 5 authors, 2020-06-04 · open the first message on its own page

[PATCH] mm: Fix pud_alloc_track()

From: Joerg Roedel <joro@8bytes.org>
Date: 2020-06-04 07:44:52

From: Joerg Roedel <redacted>

The pud_alloc_track() needs to do different checks based on whether
__ARCH_HAS_5LEVEL_HACK is defined, like it already does in
pud_alloc(). Otherwise it causes boot failures on PowerPC.

Provide the correct implementations for both possible settings of
__ARCH_HAS_5LEVEL_HACK to fix the boot problems.

Reported-by: Abdul Haleem <redacted>
Tested-by: Abdul Haleem <redacted>
Tested-by: Satheesh Rajendran <redacted>
Fixes: d8626138009b ("mm: add functions to track page directory modifications")
Signed-off-by: Joerg Roedel <redacted>
---
 include/asm-generic/5level-fixup.h |  5 +++++
 include/linux/mm.h                 | 26 +++++++++++++-------------
 2 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/include/asm-generic/5level-fixup.h b/include/asm-generic/5level-fixup.h
index 58046ddc08d0..afbab31fbd7e 100644
--- a/include/asm-generic/5level-fixup.h
+++ b/include/asm-generic/5level-fixup.h
@@ -17,6 +17,11 @@
 	((unlikely(pgd_none(*(p4d))) && __pud_alloc(mm, p4d, address)) ? \
 		NULL : pud_offset(p4d, address))
 
+#define pud_alloc_track(mm, p4d, address, mask)					\
+	((unlikely(pgd_none(*(p4d))) &&						\
+	  (__pud_alloc(mm, p4d, address) || ({*(mask)|=PGTBL_P4D_MODIFIED;0;})))?	\
+	  NULL : pud_offset(p4d, address))
+
 #define p4d_alloc(mm, pgd, address)		(pgd)
 #define p4d_alloc_track(mm, pgd, address, mask)	(pgd)
 #define p4d_offset(pgd, start)			(pgd)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 66e0977f970a..ad3b31c5bcc3 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2088,35 +2088,35 @@ static inline pud_t *pud_alloc(struct mm_struct *mm, p4d_t *p4d,
 		NULL : pud_offset(p4d, address);
 }
 
-static inline p4d_t *p4d_alloc_track(struct mm_struct *mm, pgd_t *pgd,
+static inline pud_t *pud_alloc_track(struct mm_struct *mm, p4d_t *p4d,
 				     unsigned long address,
 				     pgtbl_mod_mask *mod_mask)
-
 {
-	if (unlikely(pgd_none(*pgd))) {
-		if (__p4d_alloc(mm, pgd, address))
+	if (unlikely(p4d_none(*p4d))) {
+		if (__pud_alloc(mm, p4d, address))
 			return NULL;
-		*mod_mask |= PGTBL_PGD_MODIFIED;
+		*mod_mask |= PGTBL_P4D_MODIFIED;
 	}
 
-	return p4d_offset(pgd, address);
+	return pud_offset(p4d, address);
 }
 
-#endif /* !__ARCH_HAS_5LEVEL_HACK */
-
-static inline pud_t *pud_alloc_track(struct mm_struct *mm, p4d_t *p4d,
+static inline p4d_t *p4d_alloc_track(struct mm_struct *mm, pgd_t *pgd,
 				     unsigned long address,
 				     pgtbl_mod_mask *mod_mask)
+
 {
-	if (unlikely(p4d_none(*p4d))) {
-		if (__pud_alloc(mm, p4d, address))
+	if (unlikely(pgd_none(*pgd))) {
+		if (__p4d_alloc(mm, pgd, address))
 			return NULL;
-		*mod_mask |= PGTBL_P4D_MODIFIED;
+		*mod_mask |= PGTBL_PGD_MODIFIED;
 	}
 
-	return pud_offset(p4d, address);
+	return p4d_offset(pgd, address);
 }
 
+#endif /* !__ARCH_HAS_5LEVEL_HACK */
+
 static inline pmd_t *pmd_alloc(struct mm_struct *mm, pud_t *pud, unsigned long address)
 {
 	return (unlikely(pud_none(*pud)) && __pmd_alloc(mm, pud, address))?
-- 
2.26.2

Re: [PATCH] mm: Fix pud_alloc_track()

From: Mike Rapoport <rppt@kernel.org>
Date: 2020-06-04 16:48:28

On Thu, Jun 04, 2020 at 09:44:46AM +0200, Joerg Roedel wrote:
From: Joerg Roedel <redacted>

The pud_alloc_track() needs to do different checks based on whether
__ARCH_HAS_5LEVEL_HACK is defined, like it already does in
pud_alloc(). Otherwise it causes boot failures on PowerPC.

Provide the correct implementations for both possible settings of
__ARCH_HAS_5LEVEL_HACK to fix the boot problems.
There is a patch in mmotm [1] that completely removes
__ARCH_HAS_5LEVEL_HACK which is a part of the series [2] that updates
p4d folding accross architectures. This should fix boot on PowerPC and
the addition of pXd_alloc_track() for __ARCH_HAS_5LEVEL_HACK wouldn't be
necessary.


[1] https://github.com/hnaz/linux-mm/commit/cfae68792af3731ac902ea6ba5ed8df5a0f6bd2f
[2] https://lore.kernel.org/kvmarm/20200414153455.21744-1-rppt@kernel.org/
quoted hunk
Reported-by: Abdul Haleem <redacted>
Tested-by: Abdul Haleem <redacted>
Tested-by: Satheesh Rajendran <redacted>
Fixes: d8626138009b ("mm: add functions to track page directory modifications")
Signed-off-by: Joerg Roedel <redacted>
---
 include/asm-generic/5level-fixup.h |  5 +++++
 include/linux/mm.h                 | 26 +++++++++++++-------------
 2 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/include/asm-generic/5level-fixup.h b/include/asm-generic/5level-fixup.h
index 58046ddc08d0..afbab31fbd7e 100644
--- a/include/asm-generic/5level-fixup.h
+++ b/include/asm-generic/5level-fixup.h
@@ -17,6 +17,11 @@
 	((unlikely(pgd_none(*(p4d))) && __pud_alloc(mm, p4d, address)) ? \
 		NULL : pud_offset(p4d, address))
 
+#define pud_alloc_track(mm, p4d, address, mask)					\
+	((unlikely(pgd_none(*(p4d))) &&						\
+	  (__pud_alloc(mm, p4d, address) || ({*(mask)|=PGTBL_P4D_MODIFIED;0;})))?	\
+	  NULL : pud_offset(p4d, address))
+
 #define p4d_alloc(mm, pgd, address)		(pgd)
 #define p4d_alloc_track(mm, pgd, address, mask)	(pgd)
 #define p4d_offset(pgd, start)			(pgd)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 66e0977f970a..ad3b31c5bcc3 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2088,35 +2088,35 @@ static inline pud_t *pud_alloc(struct mm_struct *mm, p4d_t *p4d,
 		NULL : pud_offset(p4d, address);
 }
 
-static inline p4d_t *p4d_alloc_track(struct mm_struct *mm, pgd_t *pgd,
+static inline pud_t *pud_alloc_track(struct mm_struct *mm, p4d_t *p4d,
 				     unsigned long address,
 				     pgtbl_mod_mask *mod_mask)
-
 {
-	if (unlikely(pgd_none(*pgd))) {
-		if (__p4d_alloc(mm, pgd, address))
+	if (unlikely(p4d_none(*p4d))) {
+		if (__pud_alloc(mm, p4d, address))
 			return NULL;
-		*mod_mask |= PGTBL_PGD_MODIFIED;
+		*mod_mask |= PGTBL_P4D_MODIFIED;
 	}
 
-	return p4d_offset(pgd, address);
+	return pud_offset(p4d, address);
 }
 
-#endif /* !__ARCH_HAS_5LEVEL_HACK */
-
-static inline pud_t *pud_alloc_track(struct mm_struct *mm, p4d_t *p4d,
+static inline p4d_t *p4d_alloc_track(struct mm_struct *mm, pgd_t *pgd,
 				     unsigned long address,
 				     pgtbl_mod_mask *mod_mask)
+
 {
-	if (unlikely(p4d_none(*p4d))) {
-		if (__pud_alloc(mm, p4d, address))
+	if (unlikely(pgd_none(*pgd))) {
+		if (__p4d_alloc(mm, pgd, address))
 			return NULL;
-		*mod_mask |= PGTBL_P4D_MODIFIED;
+		*mod_mask |= PGTBL_PGD_MODIFIED;
 	}
 
-	return pud_offset(p4d, address);
+	return p4d_offset(pgd, address);
 }
 
+#endif /* !__ARCH_HAS_5LEVEL_HACK */
+
 static inline pmd_t *pmd_alloc(struct mm_struct *mm, pud_t *pud, unsigned long address)
 {
 	return (unlikely(pud_none(*pud)) && __pmd_alloc(mm, pud, address))?
-- 
2.26.2
-- 
Sincerely yours,
Mike.

Re: [PATCH] mm: Fix pud_alloc_track()

From: Sedat Dilek <hidden>
Date: 2020-06-04 16:58:43

On Thu, Jun 4, 2020 at 6:49 PM Mike Rapoport [off-list ref] wrote:
On Thu, Jun 04, 2020 at 09:44:46AM +0200, Joerg Roedel wrote:
quoted
From: Joerg Roedel <redacted>

The pud_alloc_track() needs to do different checks based on whether
__ARCH_HAS_5LEVEL_HACK is defined, like it already does in
pud_alloc(). Otherwise it causes boot failures on PowerPC.

Provide the correct implementations for both possible settings of
__ARCH_HAS_5LEVEL_HACK to fix the boot problems.
There is a patch in mmotm [1] that completely removes
__ARCH_HAS_5LEVEL_HACK which is a part of the series [2] that updates
p4d folding accross architectures. This should fix boot on PowerPC and
the addition of pXd_alloc_track() for __ARCH_HAS_5LEVEL_HACK wouldn't be
necessary.


[1] https://github.com/hnaz/linux-mm/commit/cfae68792af3731ac902ea6ba5ed8df5a0f6bd2f
[2] https://lore.kernel.org/kvmarm/20200414153455.21744-1-rppt@kernel.org/
That link shows an overview of v4 and is easily downloadable as a
single mbox file.
See " Series = mm: remove __ARCH_HAS_5LEVEL_HACK"

- Sedat -

[1] https://lore.kernel.org/patchwork/project/lkml/list/?series=438627
quoted
Reported-by: Abdul Haleem <redacted>
Tested-by: Abdul Haleem <redacted>
Tested-by: Satheesh Rajendran <redacted>
Fixes: d8626138009b ("mm: add functions to track page directory modifications")
Signed-off-by: Joerg Roedel <redacted>
---
 include/asm-generic/5level-fixup.h |  5 +++++
 include/linux/mm.h                 | 26 +++++++++++++-------------
 2 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/include/asm-generic/5level-fixup.h b/include/asm-generic/5level-fixup.h
index 58046ddc08d0..afbab31fbd7e 100644
--- a/include/asm-generic/5level-fixup.h
+++ b/include/asm-generic/5level-fixup.h
@@ -17,6 +17,11 @@
      ((unlikely(pgd_none(*(p4d))) && __pud_alloc(mm, p4d, address)) ? \
              NULL : pud_offset(p4d, address))

+#define pud_alloc_track(mm, p4d, address, mask)                                      \
+     ((unlikely(pgd_none(*(p4d))) &&                                         \
+       (__pud_alloc(mm, p4d, address) || ({*(mask)|=PGTBL_P4D_MODIFIED;0;})))?       \
+       NULL : pud_offset(p4d, address))
+
 #define p4d_alloc(mm, pgd, address)          (pgd)
 #define p4d_alloc_track(mm, pgd, address, mask)      (pgd)
 #define p4d_offset(pgd, start)                       (pgd)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 66e0977f970a..ad3b31c5bcc3 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2088,35 +2088,35 @@ static inline pud_t *pud_alloc(struct mm_struct *mm, p4d_t *p4d,
              NULL : pud_offset(p4d, address);
 }

-static inline p4d_t *p4d_alloc_track(struct mm_struct *mm, pgd_t *pgd,
+static inline pud_t *pud_alloc_track(struct mm_struct *mm, p4d_t *p4d,
                                   unsigned long address,
                                   pgtbl_mod_mask *mod_mask)
-
 {
-     if (unlikely(pgd_none(*pgd))) {
-             if (__p4d_alloc(mm, pgd, address))
+     if (unlikely(p4d_none(*p4d))) {
+             if (__pud_alloc(mm, p4d, address))
                      return NULL;
-             *mod_mask |= PGTBL_PGD_MODIFIED;
+             *mod_mask |= PGTBL_P4D_MODIFIED;
      }

-     return p4d_offset(pgd, address);
+     return pud_offset(p4d, address);
 }

-#endif /* !__ARCH_HAS_5LEVEL_HACK */
-
-static inline pud_t *pud_alloc_track(struct mm_struct *mm, p4d_t *p4d,
+static inline p4d_t *p4d_alloc_track(struct mm_struct *mm, pgd_t *pgd,
                                   unsigned long address,
                                   pgtbl_mod_mask *mod_mask)
+
 {
-     if (unlikely(p4d_none(*p4d))) {
-             if (__pud_alloc(mm, p4d, address))
+     if (unlikely(pgd_none(*pgd))) {
+             if (__p4d_alloc(mm, pgd, address))
                      return NULL;
-             *mod_mask |= PGTBL_P4D_MODIFIED;
+             *mod_mask |= PGTBL_PGD_MODIFIED;
      }

-     return pud_offset(p4d, address);
+     return p4d_offset(pgd, address);
 }

+#endif /* !__ARCH_HAS_5LEVEL_HACK */
+
 static inline pmd_t *pmd_alloc(struct mm_struct *mm, pud_t *pud, unsigned long address)
 {
      return (unlikely(pud_none(*pud)) && __pmd_alloc(mm, pud, address))?
--
2.26.2
--
Sincerely yours,
Mike.

Re: [PATCH] mm: Fix pud_alloc_track()

From: Guenter Roeck <linux@roeck-us.net>
Date: 2020-06-04 18:04:40

On Thu, Jun 04, 2020 at 09:44:46AM +0200, Joerg Roedel wrote:
From: Joerg Roedel <redacted>

The pud_alloc_track() needs to do different checks based on whether
__ARCH_HAS_5LEVEL_HACK is defined, like it already does in
pud_alloc(). Otherwise it causes boot failures on PowerPC.

Provide the correct implementations for both possible settings of
__ARCH_HAS_5LEVEL_HACK to fix the boot problems.

Reported-by: Abdul Haleem <redacted>
Tested-by: Abdul Haleem <redacted>
Tested-by: Satheesh Rajendran <redacted>
Fixes: d8626138009b ("mm: add functions to track page directory modifications")
Signed-off-by: Joerg Roedel <redacted>
Tested-by: Guenter Roeck <linux@roeck-us.net>
quoted hunk
---
 include/asm-generic/5level-fixup.h |  5 +++++
 include/linux/mm.h                 | 26 +++++++++++++-------------
 2 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/include/asm-generic/5level-fixup.h b/include/asm-generic/5level-fixup.h
index 58046ddc08d0..afbab31fbd7e 100644
--- a/include/asm-generic/5level-fixup.h
+++ b/include/asm-generic/5level-fixup.h
@@ -17,6 +17,11 @@
 	((unlikely(pgd_none(*(p4d))) && __pud_alloc(mm, p4d, address)) ? \
 		NULL : pud_offset(p4d, address))
 
+#define pud_alloc_track(mm, p4d, address, mask)					\
+	((unlikely(pgd_none(*(p4d))) &&						\
+	  (__pud_alloc(mm, p4d, address) || ({*(mask)|=PGTBL_P4D_MODIFIED;0;})))?	\
+	  NULL : pud_offset(p4d, address))
+
 #define p4d_alloc(mm, pgd, address)		(pgd)
 #define p4d_alloc_track(mm, pgd, address, mask)	(pgd)
 #define p4d_offset(pgd, start)			(pgd)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 66e0977f970a..ad3b31c5bcc3 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2088,35 +2088,35 @@ static inline pud_t *pud_alloc(struct mm_struct *mm, p4d_t *p4d,
 		NULL : pud_offset(p4d, address);
 }
 
-static inline p4d_t *p4d_alloc_track(struct mm_struct *mm, pgd_t *pgd,
+static inline pud_t *pud_alloc_track(struct mm_struct *mm, p4d_t *p4d,
 				     unsigned long address,
 				     pgtbl_mod_mask *mod_mask)
-
 {
-	if (unlikely(pgd_none(*pgd))) {
-		if (__p4d_alloc(mm, pgd, address))
+	if (unlikely(p4d_none(*p4d))) {
+		if (__pud_alloc(mm, p4d, address))
 			return NULL;
-		*mod_mask |= PGTBL_PGD_MODIFIED;
+		*mod_mask |= PGTBL_P4D_MODIFIED;
 	}
 
-	return p4d_offset(pgd, address);
+	return pud_offset(p4d, address);
 }
 
-#endif /* !__ARCH_HAS_5LEVEL_HACK */
-
-static inline pud_t *pud_alloc_track(struct mm_struct *mm, p4d_t *p4d,
+static inline p4d_t *p4d_alloc_track(struct mm_struct *mm, pgd_t *pgd,
 				     unsigned long address,
 				     pgtbl_mod_mask *mod_mask)
+
 {
-	if (unlikely(p4d_none(*p4d))) {
-		if (__pud_alloc(mm, p4d, address))
+	if (unlikely(pgd_none(*pgd))) {
+		if (__p4d_alloc(mm, pgd, address))
 			return NULL;
-		*mod_mask |= PGTBL_P4D_MODIFIED;
+		*mod_mask |= PGTBL_PGD_MODIFIED;
 	}
 
-	return pud_offset(p4d, address);
+	return p4d_offset(pgd, address);
 }
 
+#endif /* !__ARCH_HAS_5LEVEL_HACK */
+
 static inline pmd_t *pmd_alloc(struct mm_struct *mm, pud_t *pud, unsigned long address)
 {
 	return (unlikely(pud_none(*pud)) && __pmd_alloc(mm, pud, address))?
-- 
2.26.2

Re: [PATCH] mm: Fix pud_alloc_track()

From: Andrew Morton <akpm@linux-foundation.org>
Date: 2020-06-04 20:36:25

On Thu, 4 Jun 2020 19:48:14 +0300 Mike Rapoport [off-list ref] wrote:
On Thu, Jun 04, 2020 at 09:44:46AM +0200, Joerg Roedel wrote:
quoted
From: Joerg Roedel <redacted>

The pud_alloc_track() needs to do different checks based on whether
__ARCH_HAS_5LEVEL_HACK is defined, like it already does in
pud_alloc(). Otherwise it causes boot failures on PowerPC.

Provide the correct implementations for both possible settings of
__ARCH_HAS_5LEVEL_HACK to fix the boot problems.
There is a patch in mmotm [1] that completely removes
__ARCH_HAS_5LEVEL_HACK which is a part of the series [2] that updates
p4d folding accross architectures. This should fix boot on PowerPC and
the addition of pXd_alloc_track() for __ARCH_HAS_5LEVEL_HACK wouldn't be
necessary.


[1] https://github.com/hnaz/linux-mm/commit/cfae68792af3731ac902ea6ba5ed8df5a0f6bd2f
[2] https://lore.kernel.org/kvmarm/20200414153455.21744-1-rppt@kernel.org/
That patchset is stacked up behind many other patches, including all
the powerpc stuff in linux-next :(

As it's a big bug fix, I'll pull those patches forward, hopefully send it
all Linuswards later today...
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help