LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2 1/2] selftest/vm: Use correct PAGE_SHIFT value for ppc64
From: Mike Rapoport @ 2022-02-11 13:28 UTC (permalink / raw)
  To: Aneesh Kumar K V
  Cc: Shuah Khan, linux-mm, linux-kselftest, akpm, linuxppc-dev
In-Reply-To: <27a71056-489a-4099-b094-64766bea84db@linux.ibm.com>

Hi Aneesh,

On Fri, Feb 11, 2022 at 05:22:13PM +0530, Aneesh Kumar K V wrote:
> On 2/11/22 16:03, Mike Rapoport wrote:
> > On Fri, Feb 11, 2022 at 12:03:28PM +0530, Aneesh Kumar K.V wrote:
> > > Keep it simple by using a #define and limiting hugepage size to 2M.
> > > This keeps the test simpler instead of dynamically finding the page size
> > > and huge page size.
> > > 
> > > Without this tests are broken w.r.t reading /proc/self/pagemap
> > > 
> > > 	if (pread(pagemap_fd, ent, sizeof(ent),
> > > 			(uintptr_t)ptr >> (PAGE_SHIFT - 3)) != sizeof(ent))
> > > 		err(2, "read pagemap");
> > > 
> > > Cc: Shuah Khan <shuah@kernel.org>
> > > Cc: linux-kselftest@vger.kernel.org
> > > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> > > ---
> > >   tools/testing/selftests/vm/ksm_tests.c        | 9 ++++++++-
> > >   tools/testing/selftests/vm/transhuge-stress.c | 9 ++++++++-
> > >   2 files changed, 16 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tools/testing/selftests/vm/ksm_tests.c b/tools/testing/selftests/vm/ksm_tests.c
> > > index 1436e1a9a3d3..cae72872152b 100644
> > > --- a/tools/testing/selftests/vm/ksm_tests.c
> > > +++ b/tools/testing/selftests/vm/ksm_tests.c
> > > @@ -22,7 +22,14 @@
> > >   #define KSM_MERGE_ACROSS_NODES_DEFAULT true
> > >   #define MB (1ul << 20)
> > > -#define PAGE_SHIFT 12
> > > +#ifdef __powerpc64__
> > > +#define PAGE_SHIFT	16
> > > +#else
> > > +#define PAGE_SHIFT	12
> > > +#endif
> > 
> > Page size can be other than 4096 for other configurations as well. And even
> > on ppc64 it's not necessarily 64k.
> > 
> 
> But most common test config is with 64K page size.
> 
> > Ideally page size in selftests/vm should be sysconf(_SC_PAGESIZE)
> 
> 
> yes. As explained in commit message, the idea was to keep it simpler.

I think it's simple enough (compile tested on x86 only):

From 219577d87041f19f2c00dc7c23e0fd5aad8b02d5 Mon Sep 17 00:00:00 2001
From: Mike Rapoport <rppt@linux.ibm.com>
Date: Fri, 11 Feb 2022 15:24:13 +0200
Subject: [PATCH] selftest/vm: add helpers to detect PAGE_SIZE and PAGE_SHIFT

PAGE_SIZE is not 4096 in many configurations, particularily ppc64 uses
64K pages in majority of cases.

Add helpers to detect PAGE_SIZE and PAGE_SHIFT dynamically.

Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
---
 tools/testing/selftests/vm/gup_test.c         |  3 +-
 tools/testing/selftests/vm/ksm_tests.c        |  8 +----
 tools/testing/selftests/vm/transhuge-stress.c |  9 ++----
 tools/testing/selftests/vm/util.h             | 29 +++++++++++++++++++
 4 files changed, 34 insertions(+), 15 deletions(-)
 create mode 100644 tools/testing/selftests/vm/util.h

diff --git a/tools/testing/selftests/vm/gup_test.c b/tools/testing/selftests/vm/gup_test.c
index fe043f67798b..cda837a14736 100644
--- a/tools/testing/selftests/vm/gup_test.c
+++ b/tools/testing/selftests/vm/gup_test.c
@@ -10,8 +10,9 @@
 #include <assert.h>
 #include "../../../../mm/gup_test.h"
 
+#include "util.h"
+
 #define MB (1UL << 20)
-#define PAGE_SIZE sysconf(_SC_PAGESIZE)
 
 /* Just the flags we need, copied from mm.h: */
 #define FOLL_WRITE	0x01	/* check pte is writable */
diff --git a/tools/testing/selftests/vm/ksm_tests.c b/tools/testing/selftests/vm/ksm_tests.c
index cae72872152b..7faafd24446f 100644
--- a/tools/testing/selftests/vm/ksm_tests.c
+++ b/tools/testing/selftests/vm/ksm_tests.c
@@ -12,6 +12,7 @@
 
 #include "../kselftest.h"
 #include "../../../../include/vdso/time64.h"
+#include "util.h"
 
 #define KSM_SYSFS_PATH "/sys/kernel/mm/ksm/"
 #define KSM_FP(s) (KSM_SYSFS_PATH s)
@@ -22,17 +23,10 @@
 #define KSM_MERGE_ACROSS_NODES_DEFAULT true
 #define MB (1ul << 20)
 
-#ifdef __powerpc64__
-#define PAGE_SHIFT	16
-#else
-#define PAGE_SHIFT	12
-#endif
 /*
  * On ppc64 this will only work with radix 2M hugepage size
  */
 #define HPAGE_SHIFT 21
-
-#define PAGE_SIZE (1 << PAGE_SHIFT)
 #define HPAGE_SIZE (1 << HPAGE_SHIFT)
 
 #define PAGEMAP_PRESENT(ent)	(((ent) & (1ull << 63)) != 0)
diff --git a/tools/testing/selftests/vm/transhuge-stress.c b/tools/testing/selftests/vm/transhuge-stress.c
index b1f8d98355c5..baf90a745d28 100644
--- a/tools/testing/selftests/vm/transhuge-stress.c
+++ b/tools/testing/selftests/vm/transhuge-stress.c
@@ -16,17 +16,12 @@
 #include <string.h>
 #include <sys/mman.h>
 
-#ifdef __powerpc64__
-#define PAGE_SHIFT	16
-#else
-#define PAGE_SHIFT	12
-#endif
+#include "util.h"
+
 /*
  * On ppc64 this will only work with radix 2M hugepage size
  */
 #define HPAGE_SHIFT 21
-
-#define PAGE_SIZE (1 << PAGE_SHIFT)
 #define HPAGE_SIZE (1 << HPAGE_SHIFT)
 
 #define PAGEMAP_PRESENT(ent)	(((ent) & (1ull << 63)) != 0)
diff --git a/tools/testing/selftests/vm/util.h b/tools/testing/selftests/vm/util.h
new file mode 100644
index 000000000000..1c85d7583bac
--- /dev/null
+++ b/tools/testing/selftests/vm/util.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __KSELFTEST_VM_UTIL_H
+#define __KSELFTEST_VM_UTIL_H
+
+#include <string.h> /* ffsl() */
+#include <unistd.h> /* _SC_PAGESIZE */
+
+static unsigned __page_size;
+static unsigned __page_shift;
+
+static inline unsigned page_size(void)
+{
+	if (!__page_size)
+		__page_size = sysconf(_SC_PAGESIZE);
+	return __page_size;
+}
+
+static inline unsigned page_shift(void)
+{
+	if (!__page_shift)
+		__page_shift = (ffsl(page_size()) - 1);
+	return __page_shift;
+}
+
+#define PAGE_SHIFT	(page_shift())
+#define PAGE_SIZE	(page_size())
+
+#endif /* __KSELFTEST_VM_UTIL_H */
-- 
2.34.1

-- 
Sincerely yours,
Mike.

^ permalink raw reply related

* Re: [PATCH v2 1/2] selftest/vm: Use correct PAGE_SHIFT value for ppc64
From: Aneesh Kumar K V @ 2022-02-11 13:55 UTC (permalink / raw)
  To: Mike Rapoport; +Cc: Shuah Khan, linux-mm, linux-kselftest, akpm, linuxppc-dev
In-Reply-To: <YgZke383drSXTekO@kernel.org>

On 2/11/22 18:58, Mike Rapoport wrote:
> Hi Aneesh,
> 
> On Fri, Feb 11, 2022 at 05:22:13PM +0530, Aneesh Kumar K V wrote:
>> On 2/11/22 16:03, Mike Rapoport wrote:
>>> On Fri, Feb 11, 2022 at 12:03:28PM +0530, Aneesh Kumar K.V wrote:
>>>> Keep it simple by using a #define and limiting hugepage size to 2M.
>>>> This keeps the test simpler instead of dynamically finding the page size
>>>> and huge page size.
>>>>
>>>> Without this tests are broken w.r.t reading /proc/self/pagemap
>>>>
>>>> 	if (pread(pagemap_fd, ent, sizeof(ent),
>>>> 			(uintptr_t)ptr >> (PAGE_SHIFT - 3)) != sizeof(ent))
>>>> 		err(2, "read pagemap");
>>>>
>>>> Cc: Shuah Khan <shuah@kernel.org>
>>>> Cc: linux-kselftest@vger.kernel.org
>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>>>> ---
>>>>    tools/testing/selftests/vm/ksm_tests.c        | 9 ++++++++-
>>>>    tools/testing/selftests/vm/transhuge-stress.c | 9 ++++++++-
>>>>    2 files changed, 16 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/tools/testing/selftests/vm/ksm_tests.c b/tools/testing/selftests/vm/ksm_tests.c
>>>> index 1436e1a9a3d3..cae72872152b 100644
>>>> --- a/tools/testing/selftests/vm/ksm_tests.c
>>>> +++ b/tools/testing/selftests/vm/ksm_tests.c
>>>> @@ -22,7 +22,14 @@
>>>>    #define KSM_MERGE_ACROSS_NODES_DEFAULT true
>>>>    #define MB (1ul << 20)
>>>> -#define PAGE_SHIFT 12
>>>> +#ifdef __powerpc64__
>>>> +#define PAGE_SHIFT	16
>>>> +#else
>>>> +#define PAGE_SHIFT	12
>>>> +#endif
>>>
>>> Page size can be other than 4096 for other configurations as well. And even
>>> on ppc64 it's not necessarily 64k.
>>>
>>
>> But most common test config is with 64K page size.
>>
>>> Ideally page size in selftests/vm should be sysconf(_SC_PAGESIZE)
>>
>>
>> yes. As explained in commit message, the idea was to keep it simpler.
> 
> I think it's simple enough (compile tested on x86 only):
> 
>  From 219577d87041f19f2c00dc7c23e0fd5aad8b02d5 Mon Sep 17 00:00:00 2001
> From: Mike Rapoport <rppt@linux.ibm.com>
> Date: Fri, 11 Feb 2022 15:24:13 +0200
> Subject: [PATCH] selftest/vm: add helpers to detect PAGE_SIZE and PAGE_SHIFT
> 
> PAGE_SIZE is not 4096 in many configurations, particularily ppc64 uses
> 64K pages in majority of cases.
> 
> Add helpers to detect PAGE_SIZE and PAGE_SHIFT dynamically.
> 
> Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> ---
>   tools/testing/selftests/vm/gup_test.c         |  3 +-
>   tools/testing/selftests/vm/ksm_tests.c        |  8 +----
>   tools/testing/selftests/vm/transhuge-stress.c |  9 ++----
>   tools/testing/selftests/vm/util.h             | 29 +++++++++++++++++++
>   4 files changed, 34 insertions(+), 15 deletions(-)
>   create mode 100644 tools/testing/selftests/vm/util.h
> 
> diff --git a/tools/testing/selftests/vm/gup_test.c b/tools/testing/selftests/vm/gup_test.c
> index fe043f67798b..cda837a14736 100644
> --- a/tools/testing/selftests/vm/gup_test.c
> +++ b/tools/testing/selftests/vm/gup_test.c
> @@ -10,8 +10,9 @@
>   #include <assert.h>
>   #include "../../../../mm/gup_test.h"
>   
> +#include "util.h"
> +
>   #define MB (1UL << 20)
> -#define PAGE_SIZE sysconf(_SC_PAGESIZE)
>   
>   /* Just the flags we need, copied from mm.h: */
>   #define FOLL_WRITE	0x01	/* check pte is writable */
> diff --git a/tools/testing/selftests/vm/ksm_tests.c b/tools/testing/selftests/vm/ksm_tests.c
> index cae72872152b..7faafd24446f 100644
> --- a/tools/testing/selftests/vm/ksm_tests.c
> +++ b/tools/testing/selftests/vm/ksm_tests.c
> @@ -12,6 +12,7 @@
>   
>   #include "../kselftest.h"
>   #include "../../../../include/vdso/time64.h"
> +#include "util.h"
>   
>   #define KSM_SYSFS_PATH "/sys/kernel/mm/ksm/"
>   #define KSM_FP(s) (KSM_SYSFS_PATH s)
> @@ -22,17 +23,10 @@
>   #define KSM_MERGE_ACROSS_NODES_DEFAULT true
>   #define MB (1ul << 20)
>   
> -#ifdef __powerpc64__
> -#define PAGE_SHIFT	16
> -#else
> -#define PAGE_SHIFT	12
> -#endif
>   /*
>    * On ppc64 this will only work with radix 2M hugepage size
>    */
>   #define HPAGE_SHIFT 21
> -
> -#define PAGE_SIZE (1 << PAGE_SHIFT)
>   #define HPAGE_SIZE (1 << HPAGE_SHIFT)
>   
>   #define PAGEMAP_PRESENT(ent)	(((ent) & (1ull << 63)) != 0)
> diff --git a/tools/testing/selftests/vm/transhuge-stress.c b/tools/testing/selftests/vm/transhuge-stress.c
> index b1f8d98355c5..baf90a745d28 100644
> --- a/tools/testing/selftests/vm/transhuge-stress.c
> +++ b/tools/testing/selftests/vm/transhuge-stress.c
> @@ -16,17 +16,12 @@
>   #include <string.h>
>   #include <sys/mman.h>
>   
> -#ifdef __powerpc64__
> -#define PAGE_SHIFT	16
> -#else
> -#define PAGE_SHIFT	12
> -#endif
> +#include "util.h"
> +
>   /*
>    * On ppc64 this will only work with radix 2M hugepage size
>    */
>   #define HPAGE_SHIFT 21
> -
> -#define PAGE_SIZE (1 << PAGE_SHIFT)
>   #define HPAGE_SIZE (1 << HPAGE_SHIFT)
>   
>   #define PAGEMAP_PRESENT(ent)	(((ent) & (1ull << 63)) != 0)
> diff --git a/tools/testing/selftests/vm/util.h b/tools/testing/selftests/vm/util.h
> new file mode 100644
> index 000000000000..1c85d7583bac
> --- /dev/null
> +++ b/tools/testing/selftests/vm/util.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __KSELFTEST_VM_UTIL_H
> +#define __KSELFTEST_VM_UTIL_H
> +
> +#include <string.h> /* ffsl() */
> +#include <unistd.h> /* _SC_PAGESIZE */
> +
> +static unsigned __page_size;
> +static unsigned __page_shift;
> +
> +static inline unsigned page_size(void)
> +{
> +	if (!__page_size)
> +		__page_size = sysconf(_SC_PAGESIZE);
> +	return __page_size;
> +}
> +
> +static inline unsigned page_shift(void)
> +{
> +	if (!__page_shift)
> +		__page_shift = (ffsl(page_size()) - 1);
> +	return __page_shift;
> +}
> +
> +#define PAGE_SHIFT	(page_shift())
> +#define PAGE_SIZE	(page_size())
> +
> +#endif /* __KSELFTEST_VM_UTIL_H */

This is on top of patch 1 I sent i guess. Should I add it to the series 
because v2 also consolidate allocate_transhuge()? Or do you want to sent 
it as an independent patch?

-aneesh


^ permalink raw reply

* [PATCH] powerpc/64: Force inlining of prevent_user_access() and set_kuap()
From: Christophe Leroy @ 2022-02-11 14:16 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

A ppc64_defconfig build exhibits about 10 copied of
prevent_user_access(). It also have one copy of set_kuap().

	c000000000017340 <.prevent_user_access.constprop.0>:
	c00000000001a038:	4b ff d3 09 	bl      c000000000017340 <.prevent_user_access.constprop.0>
	c00000000001aabc:	4b ff c8 85 	bl      c000000000017340 <.prevent_user_access.constprop.0>
	c00000000001ab38:	4b ff c8 09 	bl      c000000000017340 <.prevent_user_access.constprop.0>
	c00000000001ade0:	4b ff c5 61 	bl      c000000000017340 <.prevent_user_access.constprop.0>
	c000000000039b90 <.prevent_user_access.constprop.0>:
	c00000000003ac08:	4b ff ef 89 	bl      c000000000039b90 <.prevent_user_access.constprop.0>
	c00000000003b9d0:	4b ff e1 c1 	bl      c000000000039b90 <.prevent_user_access.constprop.0>
	c00000000003ba54:	4b ff e1 3d 	bl      c000000000039b90 <.prevent_user_access.constprop.0>
	c00000000003bbfc:	4b ff df 95 	bl      c000000000039b90 <.prevent_user_access.constprop.0>
	c00000000015dde0 <.prevent_user_access.constprop.0>:
	c0000000001612c0:	4b ff cb 21 	bl      c00000000015dde0 <.prevent_user_access.constprop.0>
	c000000000161b54:	4b ff c2 8d 	bl      c00000000015dde0 <.prevent_user_access.constprop.0>
	c000000000188cf0 <.prevent_user_access.constprop.0>:
	c00000000018d658:	4b ff b6 99 	bl      c000000000188cf0 <.prevent_user_access.constprop.0>
	c00000000030fe20 <.prevent_user_access.constprop.0>:
	c0000000003123d4:	4b ff da 4d 	bl      c00000000030fe20 <.prevent_user_access.constprop.0>
	c000000000313970:	4b ff c4 b1 	bl      c00000000030fe20 <.prevent_user_access.constprop.0>
	c0000000005e6bd0 <.prevent_user_access.constprop.0>:
	c0000000005e7d8c:	4b ff ee 45 	bl      c0000000005e6bd0 <.prevent_user_access.constprop.0>
	c0000000007bcae0 <.prevent_user_access.constprop.0>:
	c0000000007bda10:	4b ff f0 d1 	bl      c0000000007bcae0 <.prevent_user_access.constprop.0>
	c0000000007bda54:	4b ff f0 8d 	bl      c0000000007bcae0 <.prevent_user_access.constprop.0>
	c0000000007bdd28:	4b ff ed b9 	bl      c0000000007bcae0 <.prevent_user_access.constprop.0>
	c0000000007c0390:	4b ff c7 51 	bl      c0000000007bcae0 <.prevent_user_access.constprop.0>
	c00000000094e4f0 <.prevent_user_access.constprop.0>:
	c000000000950e40:	4b ff d6 b1 	bl      c00000000094e4f0 <.prevent_user_access.constprop.0>
	c00000000097d2d0 <.prevent_user_access.constprop.0>:
	c0000000009813fc:	4b ff be d5 	bl      c00000000097d2d0 <.prevent_user_access.constprop.0>
	c000000000acd540 <.prevent_user_access.constprop.0>:
	c000000000ad1d60:	4b ff b7 e1 	bl      c000000000acd540 <.prevent_user_access.constprop.0>
	c000000000e5d680 <.prevent_user_access.constprop.0>:
	c000000000e64b60:	4b ff 8b 21 	bl      c000000000e5d680 <.prevent_user_access.constprop.0>
	c000000000e64b6c:	4b ff 8b 15 	bl      c000000000e5d680 <.prevent_user_access.constprop.0>
	c000000000e64c38:	4b ff 8a 49 	bl      c000000000e5d680 <.prevent_user_access.constprop.0>

When building signal_64.c with -Winline the following messages appear:

	./arch/powerpc/include/asm/book3s/64/kup.h:331:20: error: inlining failed in call to 'set_kuap': call is unlikely and code size would grow [-Werror=inline]
	./arch/powerpc/include/asm/book3s/64/kup.h:401:20: error: inlining failed in call to 'prevent_user_access.constprop': call is unlikely and code size would grow [-Werror=inline]

Those functions are used on hot pathes and have been
expected to be inlined at all time.

Force them inline.

This patch reduces the kernel text size by 700 bytes, confirming
that not inlining those functions is not worth it.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/book3s/64/kup.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/kup.h b/arch/powerpc/include/asm/book3s/64/kup.h
index 69fcf63eec94..54cf46808157 100644
--- a/arch/powerpc/include/asm/book3s/64/kup.h
+++ b/arch/powerpc/include/asm/book3s/64/kup.h
@@ -328,7 +328,7 @@ static inline unsigned long get_kuap(void)
 	return mfspr(SPRN_AMR);
 }
 
-static inline void set_kuap(unsigned long value)
+static __always_inline void set_kuap(unsigned long value)
 {
 	if (!mmu_has_feature(MMU_FTR_BOOK3S_KUAP))
 		return;
@@ -398,7 +398,7 @@ static __always_inline void allow_user_access(void __user *to, const void __user
 
 #endif /* !CONFIG_PPC_KUAP */
 
-static inline void prevent_user_access(unsigned long dir)
+static __always_inline void prevent_user_access(unsigned long dir)
 {
 	set_kuap(AMR_KUAP_BLOCKED);
 	if (static_branch_unlikely(&uaccess_flush_key))
-- 
2.34.1


^ permalink raw reply related

* Re: rcutorture’s init segfaults in ppc64le VM
From: Paul Menzel @ 2022-02-11 14:19 UTC (permalink / raw)
  To: Michael Ellerman, Paul E. McKenney
  Cc: rcu, Zhouyi Zhou, linuxppc-dev, Willy Tarreau
In-Reply-To: <87y22irx5k.fsf@mpe.ellerman.id.au>

Dear Michael,


Am 11.02.22 um 02:48 schrieb Michael Ellerman:
> Paul Menzel writes:
>> Am 08.02.22 um 11:09 schrieb Michael Ellerman:
>>> Paul Menzel writes:
>>
>> […]
>>
>>>> On the POWER8 server IBM S822LC running Ubuntu 21.10, building Linux
>>>> 5.17-rc2+ with rcutorture tests
>>>
>>> I'm not sure if that's the host kernel version or the version you're
>>> using of rcutorture? Can you tell us the sha1 of your host kernel and of
>>> the tree you're running rcutorture from?
>>
>> The host system runs Linux 5.17-rc1+ started with kexec. Unfortunately,
>> I am unable to find the exact sha1.
>>
>>       $ more /proc/version
>>       Linux version 5.17.0-rc1+ (x@eddb.molgen.mpg.de) (Ubuntu clang version 13.0.0-2, LLD 13.0.0) #1 SMP Fri Jan 28 17:13:04 CET 2022
> 
> OK. In general rc1 kernels can have issues, so it might be worth
> rebooting the host into either v5.17-rc3 or a distro or stable kernel.
> Just to rule out any issues on the host.

Yes, that was a good test. It works with Ubuntu’s 5.13 Linux kernel.

     $ more /proc/version
     Linux version 5.13.0-28-generic (buildd@bos02-ppc64el-013) (gcc 
(Ubuntu 11.2.0-7ubuntu2) 11.2.0, GNU ld (GNU Binutils for Ubuntu) 2.37) 
#31-Ubuntu SMP Thu Jan 13 17:40:19 UTC 2022

I have to do more tests, but it could be LLVM/clang related.

>> The Linux tree, from where I run rcutorture from, is at commit
>> dfd42facf1e4 (Linux 5.17-rc3) with four patches on top:
>>
>>       $ git log --oneline -6
>>       207cec79e752 (HEAD -> master, origin/master, origin/HEAD) Problems with rcutorture on ppc64le: allmodconfig(2) and other failures
>>       8c82f96fbe57 ata: libata-sata: improve sata_link_debounce()
>>       a447541d925f ata: libata-sata: remove debounce delay by default
>>       afd84e1eeafc ata: libata-sata: introduce struct sata_deb_timing
>>       f4caf7e48b75 ata: libata-sata: Simplify sata_link_resume() interface
>>       dfd42facf1e4 (tag: v5.17-rc3) Linux 5.17-rc3
>>
>>>>        $ tools/testing/selftests/rcutorture/bin/torture.sh --duration 10
>>>>
>>>> the built init
>>>>
>>>>        $ file tools/testing/selftests/rcutorture/initrd/init
>>>>        tools/testing/selftests/rcutorture/initrd/init: ELF 64-bit LSB executable, 64-bit PowerPC or cisco 7500, version 1 (SYSV), statically linked, BuildID[sha1]=0ded0e45649184a296f30d611f7a03cc51ecb616, for GNU/Linux 3.10.0, stripped
>>>
>>> Mine looks pretty much identical:
>>>
>>>     $ file tools/testing/selftests/rcutorture/initrd/init
>>>     tools/testing/selftests/rcutorture/initrd/init: ELF 64-bit LSB executable, 64-bit PowerPC or cisco 7500, version 1 (SYSV), statically linked, BuildID[sha1]=86078bf6e5d54ab0860d36aa9a65d52818b972c8, for GNU/Linux 3.10.0, stripped
>>>
>>>> segfaults in QEMU. From one of the log files
>>>
>>> But mine doesn't segfault, it runs fine and the test completes.
>>>
>>> What qemu version are you using?
>>>
>>> I tried 4.2.1 and 6.2.0, both worked.
>>
>>       $ qemu-system-ppc64le --version
>>       QEMU emulator version 6.0.0 (Debian 1:6.0+dfsg-2expubuntu1.1)
>>       Copyright (c) 2003-2021 Fabrice Bellard and the QEMU Project developers
> 
> OK, that's one difference between our setups, but I'd be surprised if it
> explains this bug, but I guess anything's possible.
> 
>>>> /dev/shm/linux/tools/testing/selftests/rcutorture/res/2022.02.01-21.52.37-torture/results-rcutorture/TREE03/console.log
>>
>> Sorry, that was the wrong path/test. The correct one for the excerpt
>> below is:
>>
>>   
>> /dev/shm/linux/tools/testing/selftests/rcutorture/res/2022.02.01-21.52.37-torture/results-locktorture-kasan/LOCK01/console.log
>>
>> (For TREE03, QEMU does not start the Linux kernel at all, that means no
>> output after:
>>
>>       Booting Linux via __start() @ 0x0000000000400000 ...
> 
> OK yeah I see that too.
> 
> Removing "threadirqs" from tools/testing/selftests/rcutorture/configs/rcu/TREE03.boot
> seems to fix it.

Nice find. I have no idea, what that means though.

> I still see some preempt related warnings, we clearly have some bugs
> with preempt enabled.
> 
>> You can now download the content of
>> `/dev/shm/linux/tools/testing/selftests/rcutorture/res/2022.02.01-21.52.37-torture/results-locktorture-kasan/LOCK01`
>> [1, 65 MB].
>>
>> Can you reproduce the segmentation fault with the line below?
>>
>>       $ qemu-system-ppc64 -enable-kvm -nographic -smp cores=1,threads=8 \
>>       -net none -enable-kvm -M pseries -nodefaults -device spapr-vscsi -serial stdio -m 512 \
>>       -kernel /dev/shm/linux/tools/testing/selftests/rcutorture/res/2022.02.01-21.52.37-torture/results-locktorture-kasan/LOCK01/vmlinux \
>>       -append "debug_boot_weak_hash panic=-1 console=ttyS0 \
>>       torture.disable_onoff_at_boot locktorture.onoff_interval=3 \
>>       locktorture.onoff_holdoff=30 locktorture.stat_interval=15 \
>>       locktorture.shutdown_secs=60 locktorture.verbose=1"
> 
> That works fine for me, boots and runs the test, then shuts down.
> 
> I assume you see the segfault on every boot, not intermittently?
> 
> So the differences between our setups are the host kernel and the qemu
> version. Can you try a different host kernel easily?
> 
> The other thing would be to try a different qemu version, you might need
> to build from source, but it's not that hard :)

Indeed. I needed to find a current Meson, but then it didn’t make a 
difference, as found out above, it’s related to the Linux kernel.


Kind regards,

Paul

^ permalink raw reply

* [PATCH] powerpc/64: Rewrite loading of AMR_KUAP_BLOCKED in assembly
From: Christophe Leroy @ 2022-02-11 14:29 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

Constant loading of AMR_KUAP_BLOCKED takes 5 instructions:

	c000000000016a40:       4c 00 01 2c     isync
	c000000000016a44:       3d 20 fc ff     lis     r9,-769
	c000000000016a48:       61 29 ff ff     ori     r9,r9,65535
	c000000000016a4c:       79 29 07 c6     rldicr  r9,r9,32,31
	c000000000016a50:       65 29 ff ff     oris    r9,r9,65535
	c000000000016a54:       61 29 ff ff     ori     r9,r9,65535
	c000000000016a58:       7d 3d 03 a6     mtamr   r9
	c000000000016a5c:       4c 00 01 2c     isync

Until GCC is fixed, implement it in assembly using 2 instructions:

	c000000000016a50:       4c 00 01 2c     isync
	c000000000016a54:       39 20 fc ff     li      r9,-769
	c000000000016a58:       79 29 80 02     rotldi  r9,r9,48
	c000000000016a5c:       7d 3d 03 a6     mtamr   r9
	c000000000016a60:       4c 00 01 2c     isync

With this change a ppc64_defconfig build is reduced by 15 kbytes.

Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94395
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/book3s/64/kup.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/include/asm/book3s/64/kup.h b/arch/powerpc/include/asm/book3s/64/kup.h
index 54cf46808157..35c017ba29e1 100644
--- a/arch/powerpc/include/asm/book3s/64/kup.h
+++ b/arch/powerpc/include/asm/book3s/64/kup.h
@@ -338,6 +338,10 @@ static __always_inline void set_kuap(unsigned long value)
 	 * before and after the move to AMR. See table 6 on page 1134.
 	 */
 	isync();
+
+	if (__builtin_constant_p(value) && value == 0xfcffffffffffffff)
+		asm("li %0, %1 ; rotldi %0, %0, 48" : "=r"(value) : "i"(0xfffffffffffffcff));
+
 	mtspr(SPRN_AMR, value);
 	isync();
 }
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH v2] powerpc/mm: Update default hugetlb size early
From: Aneesh Kumar K.V @ 2022-02-11 14:40 UTC (permalink / raw)
  To: David Hildenbrand, linuxppc-dev, mpe, Christophe Leroy; +Cc: linux-mm
In-Reply-To: <87tud5a8x5.fsf@linux.ibm.com>

Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> writes:

> David Hildenbrand <david@redhat.com> writes:
>
>> On 11.02.22 10:16, Aneesh Kumar K V wrote:
>>> On 2/11/22 14:00, David Hildenbrand wrote:
>>>> On 11.02.22 07:52, Aneesh Kumar K.V wrote:
>>>>> commit: d9c234005227 ("Do not depend on MAX_ORDER when grouping pages by mobility")
....
....

> I could build a kernel with FORCE_MAX_ZONEORDER=8 and pageblock_order =
> 8. We need to disable THP for such a kernel to boot, because THP do
> check for PMD_ORDER < MAX_ORDER. I was able to boot that kernel on a
> virtualized platform, but then gigantic_page_runtime_supported is not
> supported on such config with hash translation.
>
> On non virtualized platform I am hitting crashes like below during boot.
>
> [   47.637865][   C42] =============================================================================                                                                                                                                                                                                              
> [   47.637907][   C42] BUG pgtable-2^11 (Not tainted): Object already free                                                                                     
> [   47.637925][   C42] -----------------------------------------------------------------------------                                                           
> [   47.637925][   C42]                                                                                                                                         
> [   47.637945][   C42] Allocated in __pud_alloc+0x84/0x2a0 age=278 cpu=40 pid=1409                                                                             
> [   47.637974][   C42]  __slab_alloc.isra.0+0x40/0x60                                                                                                          
> [   47.637995][   C42]  kmem_cache_alloc+0x1a8/0x510                                                                                                           
> [   47.638010][   C42]  __pud_alloc+0x84/0x2a0                                                                                                                 
> [   47.638024][   C42]  copy_page_range+0x38c/0x1b90                                                                                                           
> [   47.638040][   C42]  dup_mm+0x548/0x880                                                                                                                     
> [   47.638058][   C42]  copy_process+0xdc0/0x1e90                                                                                                              
> [   47.638076][   C42]  kernel_clone+0xd4/0x9d0                                                                                                                
> [   47.638094][   C42]  __do_sys_clone+0x88/0xe0                                                                                                               
> [   47.638112][   C42]  system_call_exception+0x368/0x3a0                                                                                                      
> [   47.638128][   C42]  system_call_common+0xec/0x250                                                                                                          
> [   47.638147][   C42] Freed in __tlb_remove_table+0x1d4/0x200 age=263 cpu=57 pid=326                                                                          
> [   47.638172][   C42]  kmem_cache_free+0x44c/0x680                                                                                                            
> [   47.638187][   C42]  __tlb_remove_table+0x1d4/0x200                                                                                                         
> [   47.638204][   C42]  tlb_remove_table_rcu+0x54/0xa0                                                                                                         
> [   47.638222][   C42]  rcu_core+0xdd4/0x15d0                                                                                                                  
> [   47.638239][   C42]  __do_softirq+0x360/0x69c                                                                                                               
> [   47.638257][   C42]  run_ksoftirqd+0x54/0xc0                                                                                                                
> [   47.638273][   C42]  smpboot_thread_fn+0x28c/0x2f0                                                                                                          
> [   47.638290][   C42]  kthread+0x1a4/0x1b0                                                                                                                    
> [   47.638305][   C42]  ret_from_kernel_thread+0x5c/0x64                                                                                                       
> [   47.638320][   C42] Slab 0xc00c00000000d600 objects=10 used=9 fp=0xc0000000035a8000 flags=0x7ffff000010201(locked|slab|head|node=0|zone=0|lastcpupid=0x7ffff)                                                                                                                                                              
> [   47.638352][   C42] Object 0xc0000000035a8000 @offset=163840 fp=0x0000000000000000                                                                          
> [   47.638352][   C42]                                                                                                                                         
> [   47.638373][   C42] Redzone  c0000000035a4000: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb  ................                                            
> [   47.638394][   C42] Redzone  c0000000035a4010: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb  ................                                            
> [   47.638414][   C42] Redzone  c0000000035a4020: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb  ................                                            
> [   47.638435][   C42] Redzone  c0000000035a4030: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb  ................                                            
> [   47.638455][   C42] Redzone  c0000000035a4040: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb  ................                                            
> [   47.638474][   C42] Redzone  c0000000035a4050: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb  ................                                            
> [   47.638494][   C42] Redzone  c0000000035a4060: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb  ................                                            
> [   47.638514][   C42] Redzone  c0000000035a4070: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb  ................                                            
> [   47.638534][   C42] Redzone  c0000000035a4080: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb  ................                                            

Ok that turned out to be unrelated. I was using a wrong kernel. I can
boot kernel with pageblock_order > MAX_ORDER and run hugetlb related
test fine. I do get the below warning which you had already called out
in your patch.

[    3.952124] WARNING: CPU: 16 PID: 719 at mm/vmstat.c:1103 __fragmentation_index+0x14/0x70                                                                   
[    3.952136] Modules linked in:                                                                                                                              
[    3.952141] CPU: 16 PID: 719 Comm: kswapd0 Tainted: G    B             5.17.0-rc3-00044-g69052ffa0e08 #68                                                   
[    3.952149] NIP:  c000000000465264 LR: c000000000468544 CTR: 0000000000000000                                                                               
[    3.952154] REGS: c000000014a4f7e0 TRAP: 0700   Tainted: G    B              (5.17.0-rc3-00044-g69052ffa0e08)
[    3.952161] MSR:  9000000000029033 <SF,HV,EE,ME,IR,DR,RI,LE>  CR: 44042422  XER: 20000000
[    3.952174] CFAR: c000000000468540 IRQMASK: 0                  
               GPR00: c000000000468544 c000000014a4fa80 c000000001ea9500 0000000000000008 
               GPR04: c000000014a4faa0 00000000001fd700 0000000000004003 00000000001fd92d 
               GPR08: c000001fffd1c7a0 0000000000000008 0000000000000008 0000000000000000 
               GPR12: 0000000000002200 c000001fffff2880 0000000000000000 c000000013cfd240                                                                      
               GPR16: c000000011940600 c000001fffd21058 0000000000000d00 c000000001407d30                                                                      
               GPR20: ffffffffffffffaf c000001fffd21098 0000000000000000 c000000002ab7328                                                                      
               GPR24: c000000011940600 c000001fffd21300 0000000000000000 0000000000000008 
               GPR28: c000001fffd1c280 0000000000000008 0000000000000000 0000000000000004                                                                      
[    3.952231] NIP [c000000000465264] __fragmentation_index+0x14/0x70                                                                                          
[    3.952237] LR [c000000000468544] fragmentation_index+0xb4/0xe0                                                                                             
[    3.952244] Call Trace:                                        
[    3.952247] [c000000014a4fa80] [c00000000023e248] lock_release+0x138/0x470 (unreliable)
[    3.952256] [c000000014a4fac0] [c00000000047cd84] compaction_suitable+0x94/0x270
[    3.952263] [c000000014a4fb10] [c0000000004802b8] wakeup_kcompactd+0xc8/0x2a0
[    3.952270] [c000000014a4fb60] [c000000000457568] balance_pgdat+0x798/0x8d0
[    3.952277] [c000000014a4fca0] [c000000000457d14] kswapd+0x674/0x7b0                                                                                        
[    3.952283] [c000000014a4fdc0] [c0000000001d7e84] kthread+0x144/0x150                                                                                       
[    3.952290] [c000000014a4fe10] [c00000000000cd74] ret_from_kernel_thread+0x5c/0x64
[    3.952297] Instruction dump:                                      
[    3.952301] 7d2021ad 40c2fff4 e8ed0030 38a00000 7caa39ae 4e800020 60000000 7c0802a6 
[    3.952311] 60000000 28030007 7c6a1b78 40810010 <0fe00000> 60000000 60000000 e9040008 
[    3.952322] irq event stamp: 0                                        
[    3.952325] hardirqs last  enabled at (0): [<0000000000000000>] 0x0                                                                                         
[    3.952331] hardirqs last disabled at (0): [<c000000000196030>] copy_process+0x970/0x1de0                                                                   
[    3.952339] softirqs last  enabled at (0): [<c000000000196030>] copy_process+0x970/0x1de0                                                                   
[    3.952345] softirqs last disabled at (0): [<0000000000000000>] 0x0                                                                                         

I am not sure whether there is any value in selecting MAX_ORDER = 8 on
ppc64. If not we could do a patch as below for ppc64.

commit 09ed79c4fda92418914546f36c2750670503d7a0
Author: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Date:   Fri Feb 11 17:15:10 2022 +0530

    powerpc/mm: Disable MAX_ORDER value 8 on book3s64 with 64K pagesize
    
    With transparent hugepage support we expect HPAGE_PMD_ORDER < MAX_ORDER.
    Without this we BUG() during boot as below
    
    cpu 0x6: Vector: 700 (Program Check) at [c000000012143880]
        pc: c000000001b4ddbc: hugepage_init+0x108/0x2c4
        lr: c000000001b4dd98: hugepage_init+0xe4/0x2c4
        sp: c000000012143b20
       msr: 8000000002029033
      current = 0xc0000000120d0f80
      paca    = 0xc00000001ec7e900   irqmask: 0x03   irq_happened: 0x01
        pid   = 1, comm = swapper/0
    kernel BUG at mm/huge_memory.c:413!
    [c000000012143b20] c0000000022c0468 blacklisted_initcalls+0x120/0x1c8 (unreliable)
    [c000000012143bb0] c000000000012104 do_one_initcall+0x94/0x520
    [c000000012143c90] c000000001b04da0 kernel_init_freeable+0x444/0x508
    [c000000012143da0] c000000000012d8c kernel_init+0x44/0x188
    [c000000012143e10] c00000000000cbf4 ret_from_kernel_thread+0x5c/0x64
    
    Hence a FORCE_MAX_ZONEORDER of value < 9 doesn't make sense with THP
    enabled. We also cannot have value > 9 because we are limitted by
    SECTION_SIZE_BITS
    
     #if (MAX_ORDER - 1 + PAGE_SHIFT) > SECTION_SIZE_BITS
     #error Allocator MAX_ORDER exceeds SECTION_SIZE
     #endif
    
    We can select MAX_ORDER value 8 by disabling THP support but then that
    results in pageblock_order > MAX_ORDER - 1 which is not fully tested/supported.
    
    Cc: David Hildenbrand <david@redhat.com>
    Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index b779603978e1..a050f5f46df3 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -807,7 +807,7 @@ config DATA_SHIFT
 
 config FORCE_MAX_ZONEORDER
 	int "Maximum zone order"
-	range 8 9 if PPC64 && PPC_64K_PAGES
+	range 9 9 if PPC64 && PPC_64K_PAGES
 	default "9" if PPC64 && PPC_64K_PAGES
 	range 13 13 if PPC64 && !PPC_64K_PAGES
 	default "13" if PPC64 && !PPC_64K_PAGES


^ permalink raw reply related

* Re: [RFC PATCH 0/3] powerpc64/bpf: Add support for BPF Trampolines
From: Christophe Leroy @ 2022-02-11 14:40 UTC (permalink / raw)
  To: Naveen N. Rao, Daniel Borkmann, Alexei Starovoitov,
	Michael Ellerman, Steven Rostedt
  Cc: Yauheni Kaliuta, Jordan Niethe, Jiri Olsa, bpf@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org, Hari Bathini
In-Reply-To: <cover.1644216043.git.naveen.n.rao@linux.vnet.ibm.com>



Le 07/02/2022 à 08:07, Naveen N. Rao a écrit :
> This is an early RFC series that adds support for BPF Trampolines on
> powerpc64. Some of the selftests are passing for me, but this needs more
> testing and I've likely missed a few things as well. A review of the
> patches and feedback about the overall approach will be great.
> 
> This series depends on some of the other BPF JIT fixes and enhancements
> posted previously, as well as on ftrace direct enablement on powerpc
> which has also been posted in the past.

Is there any reason to limit this to powerpc64 ?

Christophe

^ permalink raw reply

* Re: [PATCH v5 2/6] powerpc/kexec_file: Add KEXEC_SIG support.
From: Paul Menzel @ 2022-02-11 15:31 UTC (permalink / raw)
  To: Michal Suchánek
  Cc: Nayna, Mimi Zohar, Sven Schnelle, David Howells, keyrings,
	Paul Mackerras, Alexander Gordeev, Rob Herring, Herbert Xu,
	Baoquan He, Christian Borntraeger, James Morris,
	Lakshmi Ramasubramanian, Christian Borntraeger, Serge E. Hallyn,
	Vasily Gorbik, linux-s390, Heiko Carstens, Dmitry Kasatkin,
	Hari Bathini, Daniel Axtens, Philipp Rudo, Frank van der Linden,
	kexec, linux-kernel, Luis Chamberlain, linux-crypto,
	linux-security-module, Jessica Yu, linux-integrity, linuxppc-dev,
	David S. Miller, Thiago Jung Bauermann, buendgen
In-Reply-To: <20220209120154.GC3113@kunlun.suse.cz>

Dear Michal,


Am 09.02.22 um 13:01 schrieb Michal Suchánek:

> On Wed, Feb 09, 2022 at 07:44:15AM +0100, Paul Menzel wrote:

>> Am 11.01.22 um 12:37 schrieb Michal Suchanek:

[…]

>> How can this be tested?
> 
> Apparently KEXEC_SIG_FORCE is x86 only although the use of the option is
> arch neutral:
> 
> arch/x86/Kconfig:config KEXEC_SIG_FORCE
> kernel/kexec_file.c:            if (IS_ENABLED(CONFIG_KEXEC_SIG_FORCE))
> {
> 
> Maybe it should be moved?

Sounds good.

> I used a patched kernel that enables lockdown in secure boot, and then
> verified that signed kernel can be loaded by kexec and unsigned not,
> with KEXEC_SIG enabled and IMA_KEXEC disabled.
> 
> The lockdown support can be enabled on any platform, and although I
> can't find it documented anywhere there appears to be code in kexec_file
> to take it into account:
> kernel/kexec.c: result = security_locked_down(LOCKDOWN_KEXEC);
> kernel/kexec_file.c:                security_locked_down(LOCKDOWN_KEXEC))
> kernel/module.c:        return security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
> kernel/params.c:            security_locked_down(LOCKDOWN_MODULE_PARAMETERS))
> and lockdown can be enabled with a buildtime option, a kernel parameter, or a
> debugfs file.
> 
> Still for testing lifting KEXEC_SIG_FORCE to some arch-neutral Kconfig file is
> probably the simplest option.
> 
> kexec -s option should be used to select kexec_file rather than the old
> style kexec which would either fail always or succeed always regardelss
> of signature.

Thank you.

>>> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
>>> ---
>>> v3: - Philipp Rudo <prudo@redhat.com>: Update the comit message with
>>>         explanation why the s390 code is usable on powerpc.
>>>       - Include correct header for mod_check_sig
>>>       - Nayna <nayna@linux.vnet.ibm.com>: Mention additional IMA features
>>>         in kconfig text
>>> ---
>>>    arch/powerpc/Kconfig        | 16 ++++++++++++++++
>>>    arch/powerpc/kexec/elf_64.c | 36 ++++++++++++++++++++++++++++++++++++
>>>    2 files changed, 52 insertions(+)
>>>
>>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>>> index dea74d7717c0..1cde9b6c5987 100644
>>> --- a/arch/powerpc/Kconfig
>>> +++ b/arch/powerpc/Kconfig
>>> @@ -560,6 +560,22 @@ config KEXEC_FILE
>>>    config ARCH_HAS_KEXEC_PURGATORY
>>>    	def_bool KEXEC_FILE
>>> +config KEXEC_SIG
>>> +	bool "Verify kernel signature during kexec_file_load() syscall"
>>> +	depends on KEXEC_FILE && MODULE_SIG_FORMAT
>>> +	help
>>> +	  This option makes kernel signature verification mandatory for
>>> +	  the kexec_file_load() syscall.
>>> +
>>> +	  In addition to that option, you need to enable signature
>>> +	  verification for the corresponding kernel image type being
>>> +	  loaded in order for this to work.
>>> +
>>> +	  Note: on powerpc IMA_ARCH_POLICY also implements kexec'ed kernel
>>> +	  verification. In addition IMA adds kernel hashes to the measurement
>>> +	  list, extends IMA PCR in the TPM, and implements kernel image
>>> +	  blacklist by hash.
>>
>> So, what is the takeaway for the user? IMA_ARCH_POLICY is preferred? What is
>> the disadvantage, and two implementations(?) needed then? More overhead?
> 
> IMA_KEXEC does more than KEXEC_SIG. The overhead is probably not big
> unless you are trying to really minimize the kernel code size.
> 
> Arguably the simpler implementation has less potential for bugs, too.
> Both in code and in user configuration required to enable the feature.
> 
> Interestingly IMA_ARCH_POLICY depends on KEXEC_SIG rather than
> IMA_KEXEC. Just mind-boggling.

I have not looked into that.

> The main problem with IMA_KEXEC from my point of view is it is not portable.
> To record the measurements TPM support is requireed which is not available on
> all platforms. It does not support PE so it cannot be used on platforms
> that use PE kernel signature format.

Could you add that to the comment please?

>>> +
>>>    config RELOCATABLE
>>>    	bool "Build a relocatable kernel"
>>>    	depends on PPC64 || (FLATMEM && (44x || FSL_BOOKE))
>>> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
>>> index eeb258002d1e..98d1cb5135b4 100644
>>> --- a/arch/powerpc/kexec/elf_64.c
>>> +++ b/arch/powerpc/kexec/elf_64.c
>>> @@ -23,6 +23,7 @@
>>>    #include <linux/of_fdt.h>
>>>    #include <linux/slab.h>
>>>    #include <linux/types.h>
>>> +#include <linux/module_signature.h>
>>>    static void *elf64_load(struct kimage *image, char *kernel_buf,
>>>    			unsigned long kernel_len, char *initrd,
>>> @@ -151,7 +152,42 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>>>    	return ret ? ERR_PTR(ret) : NULL;
>>>    }
>>> +#ifdef CONFIG_KEXEC_SIG
>>> +int elf64_verify_sig(const char *kernel, unsigned long kernel_len)
>>> +{
>>> +	const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
>>> +	struct module_signature *ms;
>>> +	unsigned long sig_len;
>>
>> Use size_t to match the signature of `verify_pkcs7_signature()`?
> 
> Nope. struct module_signature uses unsigned long, and this needs to be
> matched to avoid type errors on 32bit.

I meant for `sig_len`.

> Technically using size_t for in-memory buffers is misguided because
> AFAICT no memory buffer can be bigger than ULONG_MAX, and size_t is
> non-native type on 32bit.
> 
> Sure, the situation with ssize_t/int is different but that's not what we
> are dealing with here.
True. In my experience it prevents compiler warnings when building for 
32 bit or 64 bit. Anyway, not that important.


Kind regards,

Paul

^ permalink raw reply

* Re: rcutorture’s init segfaults in ppc64le VM
From: Paul Menzel @ 2022-02-11 15:42 UTC (permalink / raw)
  To: Michael Ellerman, Paul E. McKenney
  Cc: rcu, Zhouyi Zhou, linuxppc-dev, Willy Tarreau
In-Reply-To: <6ae23d59-fe88-6f14-7d9f-648afa3dc298@molgen.mpg.de>

Dear Michael,


Am 11.02.22 um 15:19 schrieb Paul Menzel:

> Am 11.02.22 um 02:48 schrieb Michael Ellerman:
>> Paul Menzel writes:
>>> Am 08.02.22 um 11:09 schrieb Michael Ellerman:
>>>> Paul Menzel writes:
>>>
>>> […]
>>>
>>>>> On the POWER8 server IBM S822LC running Ubuntu 21.10, building Linux
>>>>> 5.17-rc2+ with rcutorture tests
>>>>
>>>> I'm not sure if that's the host kernel version or the version you're
>>>> using of rcutorture? Can you tell us the sha1 of your host kernel 
>>>> and of the tree you're running rcutorture from?
>>>
>>> The host system runs Linux 5.17-rc1+ started with kexec. Unfortunately,
>>> I am unable to find the exact sha1.
>>>
>>>       $ more /proc/version
>>>       Linux version 5.17.0-rc1+ (x@eddb.molgen.mpg.de) (Ubuntu clang version 13.0.0-2, LLD 13.0.0) #1 SMP Fri Jan 28 17:13:04 CET 2022
>>
>> OK. In general rc1 kernels can have issues, so it might be worth
>> rebooting the host into either v5.17-rc3 or a distro or stable kernel.
>> Just to rule out any issues on the host.
> 
> Yes, that was a good test. It works with Ubuntu’s 5.13 Linux kernel.
> 
>      $ more /proc/version
>      Linux version 5.13.0-28-generic (buildd@bos02-ppc64el-013) (gcc (Ubuntu 11.2.0-7ubuntu2) 11.2.0, GNU ld (GNU Binutils for Ubuntu) 2.37) #31-Ubuntu SMP Thu Jan 13 17:40:19 UTC 2022
> 
> I have to do more tests, but it could be LLVM/clang related.

Building commit f1baf68e1383 (Merge tag 'net-5.17-rc4' of 
git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net) with the ata 
patches on top with GCC, I am unable to reproduce the issue. Before I 
built it with

     make -j100 LLVM=1 LLVM_IAS=0 bindeb-pkg

[…]


Kind regards,

Paul

^ permalink raw reply

* [PATCH v5 2/6] mm: page_isolation: check specified range for unmovable pages
From: Zi Yan @ 2022-02-11 16:41 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm
  Cc: Mel Gorman, Zi Yan, Oscar Salvador, Robin Murphy, linux-kernel,
	iommu, Mike Rapoport, Eric Ren, virtualization, linuxppc-dev,
	Christoph Hellwig, Vlastimil Babka, Marek Szyprowski
In-Reply-To: <20220211164135.1803616-1-zi.yan@sent.com>

From: Zi Yan <ziy@nvidia.com>

Enable set_migratetype_isolate() to check specified sub-range for
unmovable pages during isolation. Page isolation is done
at max(MAX_ORDER_NR_PAEGS, pageblock_nr_pages) granularity, but not all
pages within that granularity are intended to be isolated. For example,
alloc_contig_range(), which uses page isolation, allows ranges without
alignment. This commit makes unmovable page check only look for
interesting pages, so that page isolation can succeed for any
non-overlapping ranges.

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 include/linux/page-isolation.h | 12 +++++++++
 mm/page_alloc.c                | 15 +----------
 mm/page_isolation.c            | 46 +++++++++++++++++++++-------------
 3 files changed, 41 insertions(+), 32 deletions(-)

diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index e14eddf6741a..4ef7be6def83 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -15,6 +15,18 @@ static inline bool is_migrate_isolate(int migratetype)
 {
 	return migratetype == MIGRATE_ISOLATE;
 }
+static inline unsigned long pfn_max_align_down(unsigned long pfn)
+{
+	return ALIGN_DOWN(pfn, max_t(unsigned long, MAX_ORDER_NR_PAGES,
+				     pageblock_nr_pages));
+}
+
+static inline unsigned long pfn_max_align_up(unsigned long pfn)
+{
+	return ALIGN(pfn, max_t(unsigned long, MAX_ORDER_NR_PAGES,
+				pageblock_nr_pages));
+}
+
 #else
 static inline bool has_isolate_pageblock(struct zone *zone)
 {
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index e2c6a67fc386..62ef78f3d771 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -8963,18 +8963,6 @@ void *__init alloc_large_system_hash(const char *tablename,
 }
 
 #ifdef CONFIG_CONTIG_ALLOC
-static unsigned long pfn_max_align_down(unsigned long pfn)
-{
-	return pfn & ~(max_t(unsigned long, MAX_ORDER_NR_PAGES,
-			     pageblock_nr_pages) - 1);
-}
-
-static unsigned long pfn_max_align_up(unsigned long pfn)
-{
-	return ALIGN(pfn, max_t(unsigned long, MAX_ORDER_NR_PAGES,
-				pageblock_nr_pages));
-}
-
 #if defined(CONFIG_DYNAMIC_DEBUG) || \
 	(defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DYNAMIC_DEBUG_MODULE))
 /* Usage: See admin-guide/dynamic-debug-howto.rst */
@@ -9119,8 +9107,7 @@ int alloc_contig_range(unsigned long start, unsigned long end,
 	 * put back to page allocator so that buddy can use them.
 	 */
 
-	ret = start_isolate_page_range(pfn_max_align_down(start),
-				       pfn_max_align_up(end), migratetype, 0);
+	ret = start_isolate_page_range(start, end, migratetype, 0);
 	if (ret)
 		return ret;
 
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index b34f1310aeaa..64d093ab83ec 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -16,7 +16,8 @@
 #include <trace/events/page_isolation.h>
 
 /*
- * This function checks whether pageblock includes unmovable pages or not.
+ * This function checks whether pageblock within [start_pfn, end_pfn) includes
+ * unmovable pages or not.
  *
  * PageLRU check without isolation or lru_lock could race so that
  * MIGRATE_MOVABLE block might include unmovable pages. And __PageMovable
@@ -29,11 +30,14 @@
  *
  */
 static struct page *has_unmovable_pages(struct zone *zone, struct page *page,
-				 int migratetype, int flags)
+				 int migratetype, int flags,
+				 unsigned long start_pfn, unsigned long end_pfn)
 {
-	unsigned long iter = 0;
-	unsigned long pfn = page_to_pfn(page);
-	unsigned long offset = pfn % pageblock_nr_pages;
+	unsigned long first_pfn = max(page_to_pfn(page), start_pfn);
+	unsigned long pfn = first_pfn;
+	unsigned long last_pfn = min(ALIGN(pfn + 1, pageblock_nr_pages), end_pfn);
+
+	page = pfn_to_page(pfn);
 
 	if (is_migrate_cma_page(page)) {
 		/*
@@ -47,8 +51,8 @@ static struct page *has_unmovable_pages(struct zone *zone, struct page *page,
 		return page;
 	}
 
-	for (; iter < pageblock_nr_pages - offset; iter++) {
-		page = pfn_to_page(pfn + iter);
+	for (pfn = first_pfn; pfn < last_pfn; pfn++) {
+		page = pfn_to_page(pfn);
 
 		/*
 		 * Both, bootmem allocations and memory holes are marked
@@ -85,7 +89,7 @@ static struct page *has_unmovable_pages(struct zone *zone, struct page *page,
 			}
 
 			skip_pages = compound_nr(head) - (page - head);
-			iter += skip_pages - 1;
+			pfn += skip_pages - 1;
 			continue;
 		}
 
@@ -97,7 +101,7 @@ static struct page *has_unmovable_pages(struct zone *zone, struct page *page,
 		 */
 		if (!page_ref_count(page)) {
 			if (PageBuddy(page))
-				iter += (1 << buddy_order(page)) - 1;
+				pfn += (1 << buddy_order(page)) - 1;
 			continue;
 		}
 
@@ -134,7 +138,13 @@ static struct page *has_unmovable_pages(struct zone *zone, struct page *page,
 	return NULL;
 }
 
-static int set_migratetype_isolate(struct page *page, int migratetype, int isol_flags)
+/*
+ * This function set pageblock migratetype to isolate if no unmovable page is
+ * present in [start_pfn, end_pfn). The pageblock must be within
+ * [start_pfn, end_pfn).
+ */
+static int set_migratetype_isolate(struct page *page, int migratetype, int isol_flags,
+			unsigned long start_pfn, unsigned long end_pfn)
 {
 	struct zone *zone = page_zone(page);
 	struct page *unmovable;
@@ -156,7 +166,7 @@ static int set_migratetype_isolate(struct page *page, int migratetype, int isol_
 	 * FIXME: Now, memory hotplug doesn't call shrink_slab() by itself.
 	 * We just check MOVABLE pages.
 	 */
-	unmovable = has_unmovable_pages(zone, page, migratetype, isol_flags);
+	unmovable = has_unmovable_pages(zone, page, migratetype, isol_flags, start_pfn, end_pfn);
 	if (!unmovable) {
 		unsigned long nr_pages;
 		int mt = get_pageblock_migratetype(page);
@@ -267,7 +277,6 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
  * be MIGRATE_ISOLATE.
  * @start_pfn:		The lower PFN of the range to be isolated.
  * @end_pfn:		The upper PFN of the range to be isolated.
- *			start_pfn/end_pfn must be aligned to pageblock_order.
  * @migratetype:	Migrate type to set in error recovery.
  * @flags:		The following flags are allowed (they can be combined in
  *			a bit mask)
@@ -309,15 +318,16 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
 	unsigned long pfn;
 	struct page *page;
 
-	BUG_ON(!IS_ALIGNED(start_pfn, pageblock_nr_pages));
-	BUG_ON(!IS_ALIGNED(end_pfn, pageblock_nr_pages));
+	unsigned long isolate_start = pfn_max_align_down(start_pfn);
+	unsigned long isolate_end = pfn_max_align_up(end_pfn);
 
-	for (pfn = start_pfn;
-	     pfn < end_pfn;
+	for (pfn = isolate_start;
+	     pfn < isolate_end;
 	     pfn += pageblock_nr_pages) {
 		page = __first_valid_page(pfn, pageblock_nr_pages);
-		if (page && set_migratetype_isolate(page, migratetype, flags)) {
-			undo_isolate_page_range(start_pfn, pfn, migratetype);
+		if (page && set_migratetype_isolate(page, migratetype, flags,
+					start_pfn, end_pfn)) {
+			undo_isolate_page_range(isolate_start, pfn, migratetype);
 			return -EBUSY;
 		}
 	}
-- 
2.34.1


^ permalink raw reply related

* [PATCH v5 1/6] mm: page_isolation: move has_unmovable_pages() to mm/page_isolation.c
From: Zi Yan @ 2022-02-11 16:41 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm
  Cc: Mel Gorman, Zi Yan, Oscar Salvador, Robin Murphy, linux-kernel,
	iommu, Mike Rapoport, Eric Ren, virtualization, linuxppc-dev,
	Christoph Hellwig, Vlastimil Babka, Marek Szyprowski
In-Reply-To: <20220211164135.1803616-1-zi.yan@sent.com>

From: Zi Yan <ziy@nvidia.com>

has_unmovable_pages() is only used in mm/page_isolation.c. Move it from
mm/page_alloc.c and make it static.

Signed-off-by: Zi Yan <ziy@nvidia.com>
Reviewed-by: Oscar Salvador <osalvador@suse.de>
---
 include/linux/page-isolation.h |   2 -
 mm/page_alloc.c                | 119 ---------------------------------
 mm/page_isolation.c            | 119 +++++++++++++++++++++++++++++++++
 3 files changed, 119 insertions(+), 121 deletions(-)

diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index 572458016331..e14eddf6741a 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -33,8 +33,6 @@ static inline bool is_migrate_isolate(int migratetype)
 #define MEMORY_OFFLINE	0x1
 #define REPORT_FAILURE	0x2
 
-struct page *has_unmovable_pages(struct zone *zone, struct page *page,
-				 int migratetype, int flags);
 void set_pageblock_migratetype(struct page *page, int migratetype);
 int move_freepages_block(struct zone *zone, struct page *page,
 				int migratetype, int *num_movable);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index cface1d38093..e2c6a67fc386 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -8962,125 +8962,6 @@ void *__init alloc_large_system_hash(const char *tablename,
 	return table;
 }
 
-/*
- * This function checks whether pageblock includes unmovable pages or not.
- *
- * PageLRU check without isolation or lru_lock could race so that
- * MIGRATE_MOVABLE block might include unmovable pages. And __PageMovable
- * check without lock_page also may miss some movable non-lru pages at
- * race condition. So you can't expect this function should be exact.
- *
- * Returns a page without holding a reference. If the caller wants to
- * dereference that page (e.g., dumping), it has to make sure that it
- * cannot get removed (e.g., via memory unplug) concurrently.
- *
- */
-struct page *has_unmovable_pages(struct zone *zone, struct page *page,
-				 int migratetype, int flags)
-{
-	unsigned long iter = 0;
-	unsigned long pfn = page_to_pfn(page);
-	unsigned long offset = pfn % pageblock_nr_pages;
-
-	if (is_migrate_cma_page(page)) {
-		/*
-		 * CMA allocations (alloc_contig_range) really need to mark
-		 * isolate CMA pageblocks even when they are not movable in fact
-		 * so consider them movable here.
-		 */
-		if (is_migrate_cma(migratetype))
-			return NULL;
-
-		return page;
-	}
-
-	for (; iter < pageblock_nr_pages - offset; iter++) {
-		page = pfn_to_page(pfn + iter);
-
-		/*
-		 * Both, bootmem allocations and memory holes are marked
-		 * PG_reserved and are unmovable. We can even have unmovable
-		 * allocations inside ZONE_MOVABLE, for example when
-		 * specifying "movablecore".
-		 */
-		if (PageReserved(page))
-			return page;
-
-		/*
-		 * If the zone is movable and we have ruled out all reserved
-		 * pages then it should be reasonably safe to assume the rest
-		 * is movable.
-		 */
-		if (zone_idx(zone) == ZONE_MOVABLE)
-			continue;
-
-		/*
-		 * Hugepages are not in LRU lists, but they're movable.
-		 * THPs are on the LRU, but need to be counted as #small pages.
-		 * We need not scan over tail pages because we don't
-		 * handle each tail page individually in migration.
-		 */
-		if (PageHuge(page) || PageTransCompound(page)) {
-			struct page *head = compound_head(page);
-			unsigned int skip_pages;
-
-			if (PageHuge(page)) {
-				if (!hugepage_migration_supported(page_hstate(head)))
-					return page;
-			} else if (!PageLRU(head) && !__PageMovable(head)) {
-				return page;
-			}
-
-			skip_pages = compound_nr(head) - (page - head);
-			iter += skip_pages - 1;
-			continue;
-		}
-
-		/*
-		 * We can't use page_count without pin a page
-		 * because another CPU can free compound page.
-		 * This check already skips compound tails of THP
-		 * because their page->_refcount is zero at all time.
-		 */
-		if (!page_ref_count(page)) {
-			if (PageBuddy(page))
-				iter += (1 << buddy_order(page)) - 1;
-			continue;
-		}
-
-		/*
-		 * The HWPoisoned page may be not in buddy system, and
-		 * page_count() is not 0.
-		 */
-		if ((flags & MEMORY_OFFLINE) && PageHWPoison(page))
-			continue;
-
-		/*
-		 * We treat all PageOffline() pages as movable when offlining
-		 * to give drivers a chance to decrement their reference count
-		 * in MEM_GOING_OFFLINE in order to indicate that these pages
-		 * can be offlined as there are no direct references anymore.
-		 * For actually unmovable PageOffline() where the driver does
-		 * not support this, we will fail later when trying to actually
-		 * move these pages that still have a reference count > 0.
-		 * (false negatives in this function only)
-		 */
-		if ((flags & MEMORY_OFFLINE) && PageOffline(page))
-			continue;
-
-		if (__PageMovable(page) || PageLRU(page))
-			continue;
-
-		/*
-		 * If there are RECLAIMABLE pages, we need to check
-		 * it.  But now, memory offline itself doesn't call
-		 * shrink_node_slabs() and it still to be fixed.
-		 */
-		return page;
-	}
-	return NULL;
-}
-
 #ifdef CONFIG_CONTIG_ALLOC
 static unsigned long pfn_max_align_down(unsigned long pfn)
 {
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index f67c4c70f17f..b34f1310aeaa 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -15,6 +15,125 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/page_isolation.h>
 
+/*
+ * This function checks whether pageblock includes unmovable pages or not.
+ *
+ * PageLRU check without isolation or lru_lock could race so that
+ * MIGRATE_MOVABLE block might include unmovable pages. And __PageMovable
+ * check without lock_page also may miss some movable non-lru pages at
+ * race condition. So you can't expect this function should be exact.
+ *
+ * Returns a page without holding a reference. If the caller wants to
+ * dereference that page (e.g., dumping), it has to make sure that it
+ * cannot get removed (e.g., via memory unplug) concurrently.
+ *
+ */
+static struct page *has_unmovable_pages(struct zone *zone, struct page *page,
+				 int migratetype, int flags)
+{
+	unsigned long iter = 0;
+	unsigned long pfn = page_to_pfn(page);
+	unsigned long offset = pfn % pageblock_nr_pages;
+
+	if (is_migrate_cma_page(page)) {
+		/*
+		 * CMA allocations (alloc_contig_range) really need to mark
+		 * isolate CMA pageblocks even when they are not movable in fact
+		 * so consider them movable here.
+		 */
+		if (is_migrate_cma(migratetype))
+			return NULL;
+
+		return page;
+	}
+
+	for (; iter < pageblock_nr_pages - offset; iter++) {
+		page = pfn_to_page(pfn + iter);
+
+		/*
+		 * Both, bootmem allocations and memory holes are marked
+		 * PG_reserved and are unmovable. We can even have unmovable
+		 * allocations inside ZONE_MOVABLE, for example when
+		 * specifying "movablecore".
+		 */
+		if (PageReserved(page))
+			return page;
+
+		/*
+		 * If the zone is movable and we have ruled out all reserved
+		 * pages then it should be reasonably safe to assume the rest
+		 * is movable.
+		 */
+		if (zone_idx(zone) == ZONE_MOVABLE)
+			continue;
+
+		/*
+		 * Hugepages are not in LRU lists, but they're movable.
+		 * THPs are on the LRU, but need to be counted as #small pages.
+		 * We need not scan over tail pages because we don't
+		 * handle each tail page individually in migration.
+		 */
+		if (PageHuge(page) || PageTransCompound(page)) {
+			struct page *head = compound_head(page);
+			unsigned int skip_pages;
+
+			if (PageHuge(page)) {
+				if (!hugepage_migration_supported(page_hstate(head)))
+					return page;
+			} else if (!PageLRU(head) && !__PageMovable(head)) {
+				return page;
+			}
+
+			skip_pages = compound_nr(head) - (page - head);
+			iter += skip_pages - 1;
+			continue;
+		}
+
+		/*
+		 * We can't use page_count without pin a page
+		 * because another CPU can free compound page.
+		 * This check already skips compound tails of THP
+		 * because their page->_refcount is zero at all time.
+		 */
+		if (!page_ref_count(page)) {
+			if (PageBuddy(page))
+				iter += (1 << buddy_order(page)) - 1;
+			continue;
+		}
+
+		/*
+		 * The HWPoisoned page may be not in buddy system, and
+		 * page_count() is not 0.
+		 */
+		if ((flags & MEMORY_OFFLINE) && PageHWPoison(page))
+			continue;
+
+		/*
+		 * We treat all PageOffline() pages as movable when offlining
+		 * to give drivers a chance to decrement their reference count
+		 * in MEM_GOING_OFFLINE in order to indicate that these pages
+		 * can be offlined as there are no direct references anymore.
+		 * For actually unmovable PageOffline() where the driver does
+		 * not support this, we will fail later when trying to actually
+		 * move these pages that still have a reference count > 0.
+		 * (false negatives in this function only)
+		 */
+		if ((flags & MEMORY_OFFLINE) && PageOffline(page))
+			continue;
+
+		if (__PageMovable(page) || PageLRU(page))
+			continue;
+
+		/*
+		 * If there are RECLAIMABLE pages, we need to check
+		 * it.  But now, memory offline itself doesn't call
+		 * shrink_node_slabs() and it still to be fixed.
+		 */
+		return page;
+	}
+	return NULL;
+}
+
 static int set_migratetype_isolate(struct page *page, int migratetype, int isol_flags)
 {
 	struct zone *zone = page_zone(page);
-- 
2.34.1


^ permalink raw reply related

* [PATCH v5 0/6] Use pageblock_order for cma and alloc_contig_range alignment.
From: Zi Yan @ 2022-02-11 16:41 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm
  Cc: Mel Gorman, Zi Yan, Oscar Salvador, Robin Murphy, linux-kernel,
	iommu, Mike Rapoport, Eric Ren, virtualization, linuxppc-dev,
	Christoph Hellwig, Vlastimil Babka, Marek Szyprowski

From: Zi Yan <ziy@nvidia.com>

Hi all,

This patchset tries to remove the MAX_ORDER-1 alignment requirement for CMA
and alloc_contig_range(). It prepares for my upcoming changes to make
MAX_ORDER adjustable at boot time[1]. It is on top of mmotm-2022-02-08-15-31.

Changelog
===
V5
---
1. Moved isolation address alignment handling in start_isolate_page_range().
2. Rewrote and simplified how alloc_contig_range() works at pageblock
   granularity (Patch 3). Only two pageblock migratetypes need to be saved and
   restored. start_isolate_page_range() might need to migrate pages in this
   version, but it prevents the caller from worrying about
   max(MAX_ORDER_NR_PAEGS, pageblock_nr_pages) alignment after the page range
   is isolated.

V4
---
1. Dropped two irrelevant patches on non-lru compound page handling, as
   it is not supported upstream.
2. Renamed migratetype_has_fallback() to migratetype_is_mergeable().
3. Always check whether two pageblocks can be merged in
   __free_one_page() when order is >= pageblock_order, as the case (not
   mergeable pageblocks are isolated, CMA, and HIGHATOMIC) becomes more common.
3. Moving has_unmovable_pages() is now a separate patch.
4. Removed MAX_ORDER-1 alignment requirement in the comment in virtio_mem code.

Description
===

The MAX_ORDER - 1 alignment requirement comes from that alloc_contig_range()
isolates pageblocks to remove free memory from buddy allocator but isolating
only a subset of pageblocks within a page spanning across multiple pageblocks
causes free page accounting issues. Isolated page might not be put into the
right free list, since the code assumes the migratetype of the first pageblock
as the whole free page migratetype. This is based on the discussion at [2].

To remove the requirement, this patchset:
1. isolates pages at pageblock granularity instead of
   max(MAX_ORDER_NR_PAEGS, pageblock_nr_pages);
2. splits free pages across the specified range or migrates in-use pages
   across the specified range then splits the freed page to avoid free page
   accounting issues (it happens when multiple pageblocks within a single page
   have different migratetypes);
3. only checks unmovable pages within the range instead of MAX_ORDER - 1 aligned
   range during isolation to avoid alloc_contig_range() failure when pageblocks
   within a MAX_ORDER - 1 aligned range are allocated separately.
4. returns pages not in the range as it did before.

One optimization might come later:
1. make MIGRATE_ISOLATE a separate bit to be able to restore the original
   migratetypes when isolation fails in the middle of the range.

Feel free to give comments and suggestions. Thanks.

[1] https://lore.kernel.org/linux-mm/20210805190253.2795604-1-zi.yan@sent.com/
[2] https://lore.kernel.org/linux-mm/d19fb078-cb9b-f60f-e310-fdeea1b947d2@redhat.com/

Zi Yan (6):
  mm: page_isolation: move has_unmovable_pages() to mm/page_isolation.c
  mm: page_isolation: check specified range for unmovable pages
  mm: make alloc_contig_range work at pageblock granularity
  mm: cma: use pageblock_order as the single alignment
  drivers: virtio_mem: use pageblock size as the minimum virtio_mem
    size.
  arch: powerpc: adjust fadump alignment to be pageblock aligned.

 arch/powerpc/include/asm/fadump-internal.h |   4 +-
 drivers/virtio/virtio_mem.c                |   7 +-
 include/linux/mmzone.h                     |   5 +-
 include/linux/page-isolation.h             |  16 +-
 kernel/dma/contiguous.c                    |   2 +-
 mm/cma.c                                   |   6 +-
 mm/internal.h                              |   3 +
 mm/memory_hotplug.c                        |   3 +-
 mm/page_alloc.c                            | 371 ++++++++++-----------
 mm/page_isolation.c                        | 172 +++++++++-
 10 files changed, 367 insertions(+), 222 deletions(-)

-- 
2.34.1


^ permalink raw reply

* [PATCH v5 3/6] mm: make alloc_contig_range work at pageblock granularity
From: Zi Yan @ 2022-02-11 16:41 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm
  Cc: Mel Gorman, Zi Yan, Oscar Salvador, Robin Murphy, linux-kernel,
	iommu, Mike Rapoport, Eric Ren, virtualization, linuxppc-dev,
	Christoph Hellwig, Vlastimil Babka, Marek Szyprowski
In-Reply-To: <20220211164135.1803616-1-zi.yan@sent.com>

From: Zi Yan <ziy@nvidia.com>

alloc_contig_range() worked at MAX_ORDER-1 granularity to avoid merging
pageblocks with different migratetypes. It might unnecessarily convert
extra pageblocks at the beginning and at the end of the range. Change
alloc_contig_range() to work at pageblock granularity.

Special handling is needed for free pages and in-use pages across the
boundaries of the range specified alloc_contig_range(). Because these
partially isolated pages causes free page accounting issues. The free
pages will be split and freed into separate migratetype lists; the
in-use pages will be migrated then the freed pages will be handled.

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 include/linux/page-isolation.h |   2 +-
 mm/internal.h                  |   3 +
 mm/memory_hotplug.c            |   3 +-
 mm/page_alloc.c                | 235 +++++++++++++++++++++++++--------
 mm/page_isolation.c            |  33 ++++-
 5 files changed, 211 insertions(+), 65 deletions(-)

diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index 4ef7be6def83..78ff940cc169 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -54,7 +54,7 @@ int move_freepages_block(struct zone *zone, struct page *page,
  */
 int
 start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
-			 unsigned migratetype, int flags);
+			 unsigned migratetype, int flags, gfp_t gfp_flags);
 
 /*
  * Changes MIGRATE_ISOLATE to MIGRATE_MOVABLE.
diff --git a/mm/internal.h b/mm/internal.h
index 0d240e876831..509cbdc25992 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -319,6 +319,9 @@ isolate_freepages_range(struct compact_control *cc,
 int
 isolate_migratepages_range(struct compact_control *cc,
 			   unsigned long low_pfn, unsigned long end_pfn);
+
+int
+isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags, int isolate_before_boundary);
 #endif
 int find_suitable_fallback(struct free_area *area, unsigned int order,
 			int migratetype, bool only_stealable, bool *can_steal);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index ce68098832aa..82406d2f3e46 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1863,7 +1863,8 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages,
 	/* set above range as isolated */
 	ret = start_isolate_page_range(start_pfn, end_pfn,
 				       MIGRATE_MOVABLE,
-				       MEMORY_OFFLINE | REPORT_FAILURE);
+				       MEMORY_OFFLINE | REPORT_FAILURE,
+				       GFP_USER | __GFP_MOVABLE | __GFP_RETRY_MAYFAIL);
 	if (ret) {
 		reason = "failure to isolate range";
 		goto failed_removal_pcplists_disabled;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 62ef78f3d771..7a4fa21aea5c 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -8985,7 +8985,7 @@ static inline void alloc_contig_dump_pages(struct list_head *page_list)
 #endif
 
 /* [start, end) must belong to a single zone. */
-static int __alloc_contig_migrate_range(struct compact_control *cc,
+int __alloc_contig_migrate_range(struct compact_control *cc,
 					unsigned long start, unsigned long end)
 {
 	/* This function is based on compact_zone() from compaction.c. */
@@ -9043,6 +9043,167 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
 	return 0;
 }
 
+/**
+ * split_free_page() -- split a free page at split_pfn_offset
+ * @free_page:		the original free page
+ * @order:		the order of the page
+ * @split_pfn_offset:	split offset within the page
+ *
+ * It is used when the free page crosses two pageblocks with different migratetypes
+ * at split_pfn_offset within the page. The split free page will be put into
+ * separate migratetype lists afterwards. Otherwise, the function achieves
+ * nothing.
+ */
+static inline void split_free_page(struct page *free_page,
+				int order, unsigned long split_pfn_offset)
+{
+	struct zone *zone = page_zone(free_page);
+	unsigned long free_page_pfn = page_to_pfn(free_page);
+	unsigned long pfn;
+	unsigned long flags;
+	int free_page_order;
+
+	spin_lock_irqsave(&zone->lock, flags);
+	del_page_from_free_list(free_page, zone, order);
+	for (pfn = free_page_pfn;
+	     pfn < free_page_pfn + (1UL << order);) {
+		int mt = get_pfnblock_migratetype(pfn_to_page(pfn), pfn);
+
+		free_page_order = order_base_2(split_pfn_offset);
+		__free_one_page(pfn_to_page(pfn), pfn, zone, free_page_order,
+				mt, FPI_NONE);
+		pfn += 1UL << free_page_order;
+		split_pfn_offset -= (1UL << free_page_order);
+		/* we have done the first part, now switch to second part */
+		if (split_pfn_offset == 0)
+			split_pfn_offset = (1UL << order) - (pfn - free_page_pfn);
+	}
+	spin_unlock_irqrestore(&zone->lock, flags);
+}
+
+/**
+ * isolate_single_pageblock() -- tries to isolate a pageblock that might be
+ * within a free or in-use page.
+ * @boundary_pfn:		pageblock-aligned pfn that a page might cross
+ * @gfp_flags:			GFP flags used for migrating pages
+ * @isolate_before_boundary:	isolate the pageblock before (1) or after (0)
+ *				the boundary_pfn
+ *
+ * Free and in-use pages can be as big as MAX_ORDER-1 and contain more than one
+ * pageblock. When not all pageblocks within a page are isolated at the same
+ * time, free page accounting can go wrong. For example, in the case of
+ * MAX_ORDER-1 = pageblock_order + 1, a MAX_ORDER-1 page has two pagelbocks.
+ * [        MAX_ORDER-1          ]
+ * [  pageblock0  |  pageblock1  ]
+ * When either pageblock is isolated, if it is a free page, the page is not
+ * split into separate migratetype lists, which is supposed to; if it is an
+ * in-use page and freed later, __free_one_page() does not split the free page
+ * either. The function handles this by splitting the free page or migrating
+ * the in-use page then splitting the free page.
+ */
+int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
+			int isolate_before_boundary)
+{
+	unsigned char saved_mt;
+	/*
+	 * scan at max(MAX_ORDER_NR_PAGES, pageblock_nr_pages) aligned range to
+	 * avoid isolate pageblocks belonging to a bigger free or in-use page
+	 */
+	unsigned long start_pfn = pfn_max_align_down(boundary_pfn);
+	unsigned long isolated_pageblock_pfn;
+	unsigned long pfn;
+
+	VM_BUG_ON(!IS_ALIGNED(boundary_pfn, pageblock_nr_pages));
+
+	if (isolate_before_boundary)
+		isolated_pageblock_pfn = boundary_pfn - pageblock_nr_pages;
+	else
+		isolated_pageblock_pfn = boundary_pfn;
+
+	saved_mt = get_pageblock_migratetype(pfn_to_page(isolated_pageblock_pfn));
+	set_pageblock_migratetype(pfn_to_page(isolated_pageblock_pfn), MIGRATE_ISOLATE);
+
+	for (pfn = start_pfn; pfn < boundary_pfn;) {
+		struct page *page = pfn_to_page(pfn);
+
+		/*
+		 * start_pfn is max(MAX_ORDER_NR_PAGES, pageblock_nr_pages)
+		 * aligned, if there is any free pages in [start_pfn, boundary_pfn),
+		 * its head page will always be in the range.
+		 */
+		if (PageBuddy(page)) {
+			int order = buddy_order(page);
+
+			if (pfn + (1UL << order) > boundary_pfn)
+				split_free_page(page, order, boundary_pfn - pfn);
+			pfn += (1UL << order);
+			continue;
+		}
+		/*
+		 * migrate compound pages then let the free page handling code
+		 * above do the rest
+		 */
+		if (PageHuge(page) || PageTransCompound(page)) {
+			unsigned long nr_pages = compound_nr(page);
+			int order = compound_order(page);
+			struct page *head = compound_head(page);
+			unsigned long head_pfn = page_to_pfn(head);
+
+			if (head_pfn + nr_pages >= boundary_pfn) {
+				int ret;
+				struct compact_control cc = {
+					.nr_migratepages = 0,
+					.order = -1,
+					.zone = page_zone(pfn_to_page(head_pfn)),
+					.mode = MIGRATE_SYNC,
+					.ignore_skip_hint = true,
+					.no_set_skip_hint = true,
+					.gfp_mask = current_gfp_context(gfp_flags),
+					.alloc_contig = true,
+				};
+
+				INIT_LIST_HEAD(&cc.migratepages);
+
+				ret = __alloc_contig_migrate_range(&cc, head_pfn,
+							head_pfn + nr_pages);
+
+				if (ret) {
+					/* restore the original migratetype */
+					set_pageblock_migratetype(
+						pfn_to_page(isolated_pageblock_pfn),
+						saved_mt);
+					return -EBUSY;
+				}
+				/*
+				 * reset pfn, let the free page handling code
+				 * above split the free page to the right
+				 * migratetype list.
+				 *
+				 * head_pfn is not used here as a hugetlb page
+				 * order can be bigger than MAX_ORDER-1, but
+				 * after it is freed, the free page order is not.
+				 * Use pfn within the range to find the head of
+				 * the free page and reset order to 0 if a hugetlb
+				 * page with >MAX_ORDER-1 order is encountered.
+				 */
+				if (order > MAX_ORDER-1)
+					order = 0;
+				while (!PageBuddy(pfn_to_page(pfn))) {
+					order++;
+					pfn &= ~0UL << order;
+				}
+				continue;
+			}
+			pfn += nr_pages;
+			continue;
+		}
+
+		pfn++;
+	}
+	return 0;
+}
+
+
 /**
  * alloc_contig_range() -- tries to allocate given range of pages
  * @start:	start PFN to allocate
@@ -9067,8 +9228,9 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
 int alloc_contig_range(unsigned long start, unsigned long end,
 		       unsigned migratetype, gfp_t gfp_mask)
 {
-	unsigned long outer_start, outer_end;
-	unsigned int order;
+	unsigned long outer_end;
+	unsigned long alloc_start = ALIGN_DOWN(start, pageblock_nr_pages);
+	unsigned long alloc_end = ALIGN(end, pageblock_nr_pages);
 	int ret = 0;
 
 	struct compact_control cc = {
@@ -9087,14 +9249,11 @@ int alloc_contig_range(unsigned long start, unsigned long end,
 	 * What we do here is we mark all pageblocks in range as
 	 * MIGRATE_ISOLATE.  Because pageblock and max order pages may
 	 * have different sizes, and due to the way page allocator
-	 * work, we align the range to biggest of the two pages so
-	 * that page allocator won't try to merge buddies from
-	 * different pageblocks and change MIGRATE_ISOLATE to some
-	 * other migration type.
+	 * work, start_isolate_page_range() has special handlings for this.
 	 *
 	 * Once the pageblocks are marked as MIGRATE_ISOLATE, we
 	 * migrate the pages from an unaligned range (ie. pages that
-	 * we are interested in).  This will put all the pages in
+	 * we are interested in). This will put all the pages in
 	 * range back to page allocator as MIGRATE_ISOLATE.
 	 *
 	 * When this is done, we take the pages in range from page
@@ -9107,9 +9266,9 @@ int alloc_contig_range(unsigned long start, unsigned long end,
 	 * put back to page allocator so that buddy can use them.
 	 */
 
-	ret = start_isolate_page_range(start, end, migratetype, 0);
+	ret = start_isolate_page_range(start, end, migratetype, 0, gfp_mask);
 	if (ret)
-		return ret;
+		goto done;
 
 	drain_all_pages(cc.zone);
 
@@ -9128,68 +9287,28 @@ int alloc_contig_range(unsigned long start, unsigned long end,
 		goto done;
 	ret = 0;
 
-	/*
-	 * Pages from [start, end) are within a MAX_ORDER_NR_PAGES
-	 * aligned blocks that are marked as MIGRATE_ISOLATE.  What's
-	 * more, all pages in [start, end) are free in page allocator.
-	 * What we are going to do is to allocate all pages from
-	 * [start, end) (that is remove them from page allocator).
-	 *
-	 * The only problem is that pages at the beginning and at the
-	 * end of interesting range may be not aligned with pages that
-	 * page allocator holds, ie. they can be part of higher order
-	 * pages.  Because of this, we reserve the bigger range and
-	 * once this is done free the pages we are not interested in.
-	 *
-	 * We don't have to hold zone->lock here because the pages are
-	 * isolated thus they won't get removed from buddy.
-	 */
-
-	order = 0;
-	outer_start = start;
-	while (!PageBuddy(pfn_to_page(outer_start))) {
-		if (++order >= MAX_ORDER) {
-			outer_start = start;
-			break;
-		}
-		outer_start &= ~0UL << order;
-	}
-
-	if (outer_start != start) {
-		order = buddy_order(pfn_to_page(outer_start));
-
-		/*
-		 * outer_start page could be small order buddy page and
-		 * it doesn't include start page. Adjust outer_start
-		 * in this case to report failed page properly
-		 * on tracepoint in test_pages_isolated()
-		 */
-		if (outer_start + (1UL << order) <= start)
-			outer_start = start;
-	}
-
 	/* Make sure the range is really isolated. */
-	if (test_pages_isolated(outer_start, end, 0)) {
+	if (test_pages_isolated(alloc_start, alloc_end, 0)) {
 		ret = -EBUSY;
 		goto done;
 	}
 
 	/* Grab isolated pages from freelists. */
-	outer_end = isolate_freepages_range(&cc, outer_start, end);
+	outer_end = isolate_freepages_range(&cc, alloc_start, alloc_end);
 	if (!outer_end) {
 		ret = -EBUSY;
 		goto done;
 	}
 
 	/* Free head and tail (if any) */
-	if (start != outer_start)
-		free_contig_range(outer_start, start - outer_start);
-	if (end != outer_end)
-		free_contig_range(end, outer_end - end);
+	if (start != alloc_start)
+		free_contig_range(alloc_start, start - alloc_start);
+	if (end != alloc_end)
+		free_contig_range(end, alloc_end - end);
 
 done:
-	undo_isolate_page_range(pfn_max_align_down(start),
-				pfn_max_align_up(end), migratetype);
+	undo_isolate_page_range(alloc_start,
+				alloc_end, migratetype);
 	return ret;
 }
 EXPORT_SYMBOL(alloc_contig_range);
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 64d093ab83ec..0256d5e1032c 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -285,6 +285,8 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
  *					 and PageOffline() pages.
  *			REPORT_FAILURE - report details about the failure to
  *			isolate the range
+ * @gfp_flags:		GFP flags used for migrating pages that sit across the
+ *			range boundaries.
  *
  * Making page-allocation-type to be MIGRATE_ISOLATE means free pages in
  * the range will never be allocated. Any free pages and pages freed in the
@@ -293,6 +295,10 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
  * pages in the range finally, the caller have to free all pages in the range.
  * test_page_isolated() can be used for test it.
  *
+ * The function first tries to isolate the pageblocks at the beginning and end
+ * of the range, since there might be pages across the range boundaries.
+ * Afterwards, it isolates the rest of the range.
+ *
  * There is no high level synchronization mechanism that prevents two threads
  * from trying to isolate overlapping ranges. If this happens, one thread
  * will notice pageblocks in the overlapping range already set to isolate.
@@ -313,21 +319,38 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
  * Return: 0 on success and -EBUSY if any part of range cannot be isolated.
  */
 int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
-			     unsigned migratetype, int flags)
+			     unsigned migratetype, int flags, gfp_t gfp_flags)
 {
 	unsigned long pfn;
 	struct page *page;
+	/* isolation is done at page block granularity */
+	unsigned long isolate_start = ALIGN_DOWN(start_pfn, pageblock_nr_pages);
+	unsigned long isolate_end = ALIGN(end_pfn, pageblock_nr_pages);
+	int ret;
 
-	unsigned long isolate_start = pfn_max_align_down(start_pfn);
-	unsigned long isolate_end = pfn_max_align_up(end_pfn);
+	/* isolate [isolate_start, isolate_start + pageblock_nr_pages] pageblock */
+	ret = isolate_single_pageblock(isolate_start, gfp_flags, 0);
+	if (ret)
+		return ret;
+
+	/* isolate [isolate_end - pageblock_nr_pages, isolate_end] pageblock */
+	ret = isolate_single_pageblock(isolate_end, gfp_flags, 1);
+	if (ret) {
+		unset_migratetype_isolate(pfn_to_page(isolate_start), migratetype);
+		return ret;
+	}
 
-	for (pfn = isolate_start;
-	     pfn < isolate_end;
+	/* skip isolated pageblocks at the beginning and end */
+	for (pfn = isolate_start + pageblock_nr_pages;
+	     pfn < isolate_end - pageblock_nr_pages;
 	     pfn += pageblock_nr_pages) {
 		page = __first_valid_page(pfn, pageblock_nr_pages);
 		if (page && set_migratetype_isolate(page, migratetype, flags,
 					start_pfn, end_pfn)) {
 			undo_isolate_page_range(isolate_start, pfn, migratetype);
+			unset_migratetype_isolate(
+				pfn_to_page(isolate_end - pageblock_nr_pages),
+				migratetype);
 			return -EBUSY;
 		}
 	}
-- 
2.34.1


^ permalink raw reply related

* [PATCH v5 4/6] mm: cma: use pageblock_order as the single alignment
From: Zi Yan @ 2022-02-11 16:41 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm
  Cc: Mel Gorman, Zi Yan, Oscar Salvador, Robin Murphy, linux-kernel,
	iommu, Mike Rapoport, Eric Ren, virtualization, linuxppc-dev,
	Christoph Hellwig, Vlastimil Babka, Marek Szyprowski
In-Reply-To: <20220211164135.1803616-1-zi.yan@sent.com>

From: Zi Yan <ziy@nvidia.com>

Now alloc_contig_range() works at pageblock granularity. Change CMA
allocation, which uses alloc_contig_range(), to use pageblock_order
alignment.

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 include/linux/mmzone.h  | 5 +----
 kernel/dma/contiguous.c | 2 +-
 mm/cma.c                | 6 ++----
 mm/page_alloc.c         | 4 ++--
 4 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 3fff6deca2c0..da38c8436493 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -54,10 +54,7 @@ enum migratetype {
 	 *
 	 * The way to use it is to change migratetype of a range of
 	 * pageblocks to MIGRATE_CMA which can be done by
-	 * __free_pageblock_cma() function.  What is important though
-	 * is that a range of pageblocks must be aligned to
-	 * MAX_ORDER_NR_PAGES should biggest page be bigger than
-	 * a single pageblock.
+	 * __free_pageblock_cma() function.
 	 */
 	MIGRATE_CMA,
 #endif
diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
index 3d63d91cba5c..ac35b14b0786 100644
--- a/kernel/dma/contiguous.c
+++ b/kernel/dma/contiguous.c
@@ -399,7 +399,7 @@ static const struct reserved_mem_ops rmem_cma_ops = {
 
 static int __init rmem_cma_setup(struct reserved_mem *rmem)
 {
-	phys_addr_t align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
+	phys_addr_t align = PAGE_SIZE << pageblock_order;
 	phys_addr_t mask = align - 1;
 	unsigned long node = rmem->fdt_node;
 	bool default_cma = of_get_flat_dt_prop(node, "linux,cma-default", NULL);
diff --git a/mm/cma.c b/mm/cma.c
index 766f1b82b532..b2e927fab7b5 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -187,8 +187,7 @@ int __init cma_init_reserved_mem(phys_addr_t base, phys_addr_t size,
 		return -EINVAL;
 
 	/* ensure minimal alignment required by mm core */
-	alignment = PAGE_SIZE <<
-			max_t(unsigned long, MAX_ORDER - 1, pageblock_order);
+	alignment = PAGE_SIZE << pageblock_order;
 
 	/* alignment should be aligned with order_per_bit */
 	if (!IS_ALIGNED(alignment >> PAGE_SHIFT, 1 << order_per_bit))
@@ -275,8 +274,7 @@ int __init cma_declare_contiguous_nid(phys_addr_t base,
 	 * migratetype page by page allocator's buddy algorithm. In the case,
 	 * you couldn't get a contiguous memory, which is not what we want.
 	 */
-	alignment = max(alignment,  (phys_addr_t)PAGE_SIZE <<
-			  max_t(unsigned long, MAX_ORDER - 1, pageblock_order));
+	alignment = max(alignment,  (phys_addr_t)PAGE_SIZE << pageblock_order);
 	if (fixed && base & (alignment - 1)) {
 		ret = -EINVAL;
 		pr_err("Region at %pa must be aligned to %pa bytes\n",
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 7a4fa21aea5c..ac9432e63ce1 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -9214,8 +9214,8 @@ int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
  *			be either of the two.
  * @gfp_mask:	GFP mask to use during compaction
  *
- * The PFN range does not have to be pageblock or MAX_ORDER_NR_PAGES
- * aligned.  The PFN range must belong to a single zone.
+ * The PFN range does not have to be pageblock aligned. The PFN range must
+ * belong to a single zone.
  *
  * The first thing this routine does is attempt to MIGRATE_ISOLATE all
  * pageblocks in the range.  Once isolated, the pageblocks should not
-- 
2.34.1


^ permalink raw reply related

* [PATCH v5 6/6] arch: powerpc: adjust fadump alignment to be pageblock aligned.
From: Zi Yan @ 2022-02-11 16:41 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm
  Cc: Mel Gorman, Zi Yan, Oscar Salvador, Robin Murphy, linux-kernel,
	iommu, Mike Rapoport, Eric Ren, virtualization, linuxppc-dev,
	Christoph Hellwig, Vlastimil Babka, Marek Szyprowski
In-Reply-To: <20220211164135.1803616-1-zi.yan@sent.com>

From: Zi Yan <ziy@nvidia.com>

CMA only requires pageblock alignment now. Change CMA alignment in
fadump too.

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 arch/powerpc/include/asm/fadump-internal.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/fadump-internal.h b/arch/powerpc/include/asm/fadump-internal.h
index 52189928ec08..fbfca85b4200 100644
--- a/arch/powerpc/include/asm/fadump-internal.h
+++ b/arch/powerpc/include/asm/fadump-internal.h
@@ -20,9 +20,7 @@
 #define memblock_num_regions(memblock_type)	(memblock.memblock_type.cnt)
 
 /* Alignment per CMA requirement. */
-#define FADUMP_CMA_ALIGNMENT	(PAGE_SIZE <<				\
-				 max_t(unsigned long, MAX_ORDER - 1,	\
-				 pageblock_order))
+#define FADUMP_CMA_ALIGNMENT	(PAGE_SIZE << pageblock_order)
 
 /* FAD commands */
 #define FADUMP_REGISTER			1
-- 
2.34.1


^ permalink raw reply related

* [PATCH v5 5/6] drivers: virtio_mem: use pageblock size as the minimum virtio_mem size.
From: Zi Yan @ 2022-02-11 16:41 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm
  Cc: Mel Gorman, Zi Yan, Oscar Salvador, Robin Murphy, linux-kernel,
	iommu, Mike Rapoport, Eric Ren, virtualization, linuxppc-dev,
	Christoph Hellwig, Vlastimil Babka, Marek Szyprowski
In-Reply-To: <20220211164135.1803616-1-zi.yan@sent.com>

From: Zi Yan <ziy@nvidia.com>

alloc_contig_range() now only needs to be aligned to pageblock_order,
drop virtio_mem size requirement that it needs to be the max of
pageblock_order and MAX_ORDER.

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 drivers/virtio/virtio_mem.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
index 38becd8d578c..2307e65d18c2 100644
--- a/drivers/virtio/virtio_mem.c
+++ b/drivers/virtio/virtio_mem.c
@@ -2476,13 +2476,12 @@ static int virtio_mem_init_hotplug(struct virtio_mem *vm)
 				      VIRTIO_MEM_DEFAULT_OFFLINE_THRESHOLD);
 
 	/*
-	 * We want subblocks to span at least MAX_ORDER_NR_PAGES and
-	 * pageblock_nr_pages pages. This:
+	 * We want subblocks to span at least pageblock_nr_pages pages.
+	 * This:
 	 * - Is required for now for alloc_contig_range() to work reliably -
 	 *   it doesn't properly handle smaller granularity on ZONE_NORMAL.
 	 */
-	sb_size = max_t(uint64_t, MAX_ORDER_NR_PAGES,
-			pageblock_nr_pages) * PAGE_SIZE;
+	sb_size = pageblock_nr_pages * PAGE_SIZE;
 	sb_size = max_t(uint64_t, vm->device_block_size, sb_size);
 
 	if (sb_size < memory_block_size_bytes() && !force_bbm) {
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH kernel 2/3] powerpc/llvm: Sample config for LLVM LTO
From: Nick Desaulniers @ 2022-02-12  0:05 UTC (permalink / raw)
  To: Alexey Kardashevskiy
  Cc: Nathan Lynch, Fabiano Rosas, llvm, Nicholas Piggin,
	Nathan Chancellor, Joel Stanley, Naveen N. Rao, linuxppc-dev,
	Daniel Axtens
In-Reply-To: <20220211023125.1790960-3-aik@ozlabs.ru>

On Thu, Feb 10, 2022 at 6:31 PM Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>
> The config is a copy of ppc64_defconfig with a few tweaks. This could be
> a smaller config to merge into ppc64_defconfig but unfortunately
> merger does not allow disabling already enabled options.

Cool series!

>
> This is a command line to compile the kernel using the upstream llvm:
>
> make -j64 O=/home/aik/pbuild/kernels-llvm/ \
>  "KCFLAGS=-Wmissing-braces -Wno-array-bounds" \
>  ARCH=powerpc LLVM_IAS=1 ppc64le_lto_defconfig CC=clang LLVM=1

That command line invocation is kind of a mess, and many things
shouldn't be necessary.

O= is just noise; if folks are doing in tree builds then that doesn't
add anything meaningful.
KCFLAGS= why? I know -Warray-bounds is being worked on actively, but
do we have instances of -Wmissing-braces at the moment? Let's get
those fixed up.
LLVM_IAS=1 is implied by LLVM=1.
CC=clang is implied by LLVM=1

why add a new config? I think it would be simpler to just show command
line invocations of `./scripts/config -e` and `make`. No new config
required.

>
> Forces CONFIG_BTRFS_FS=y to make CONFIG_ZSTD_COMPRESS=y to fix:
> ld.lld: error: linking module flags 'Code Model': IDs have conflicting values in 'lib/built-in.a(entropy_common.o at 5332)' and 'ld-temp.o'
>
> because modules are linked with -mcmodel=large but the kernel uses -mcmodel=medium

Please file a bug about this.
https://github.com/ClangBuiltLinux/linux/issues

>
> Enables CONFIG_USERFAULTFD=y as otherwise vm_userfaultfd_ctx becomes
> 0 bytes long and clang sanitizer crashes as
> https://bugs.llvm.org/show_bug.cgi?id=500375

The above hyperlink doesn't work for me. Upstream llvm just moved from
bugzilla to github issue tracker.

https://github.com/llvm/llvm-project/issues

>
> Disables CONFIG_FTR_FIXUP_SELFTEST as it uses FTR_SECTION_ELSE with
> conditional branches. There are other places like this and the following
> patches address that.
>
> Disables CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT as CONFIG_HAS_LTO_CLANG
> depends on it being disabled. In order to avoid disabling way too many
> options (like DYNAMIC_FTRACE/FUNCTION_TRACER), this converts
> FTRACE_MCOUNT_USE_RECORDMCOUNT from def_bool to bool.
>
> Note that even with this config there is a good chance that LTO
> is going to fail linking vmlinux because of the "bc" problem.

I think rather than adding a new config with LTO enabled and a few
things turned off, it would be better to not allow LTO to be
selectable if those things are turned on, until the combination of the
two are fixed.

-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply

* [PATCH] powerpc: Reduce csum_add() complexity for PPC64
From: Christophe Leroy @ 2022-02-12  7:36 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

PPC64 does everything in C, gcc is able to skip calculation
when one of the operands in zero.

Move the constant folding in PPC32 part.

This helps GCC and reduces ppc64_defconfig by 170 bytes.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/checksum.h | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/checksum.h b/arch/powerpc/include/asm/checksum.h
index 3288a1bf5e8d..e4e25b46ac49 100644
--- a/arch/powerpc/include/asm/checksum.h
+++ b/arch/powerpc/include/asm/checksum.h
@@ -95,16 +95,15 @@ static __always_inline __wsum csum_add(__wsum csum, __wsum addend)
 {
 #ifdef __powerpc64__
 	u64 res = (__force u64)csum;
-#endif
+
+	res += (__force u64)addend;
+	return (__force __wsum)((u32)res + (res >> 32));
+#else
 	if (__builtin_constant_p(csum) && csum == 0)
 		return addend;
 	if (__builtin_constant_p(addend) && addend == 0)
 		return csum;
 
-#ifdef __powerpc64__
-	res += (__force u64)addend;
-	return (__force __wsum)((u32)res + (res >> 32));
-#else
 	asm("addc %0,%0,%1;"
 	    "addze %0,%0;"
 	    : "+r" (csum) : "r" (addend) : "xer");
-- 
2.34.1


^ permalink raw reply related

* [PATCH] powerpc: Fix STACKTRACE=n build
From: Michael Ellerman @ 2022-02-12 11:13 UTC (permalink / raw)
  To: linuxppc-dev

Our skiroot_defconfig doesn't enable FTRACE, and so doesn't get
STACKTRACE enabled either. That leads to a build failure since commit
1614b2b11fab ("arch: Make ARCH_STACKWALK independent of STACKTRACE")
made stacktrace.c build even when STACKTRACE=n.

  arch/powerpc/kernel/stacktrace.c: In function ‘handle_backtrace_ipi’:
  arch/powerpc/kernel/stacktrace.c:171:2: error: implicit declaration of function ‘nmi_cpu_backtrace’
    171 |  nmi_cpu_backtrace(regs);
        |  ^~~~~~~~~~~~~~~~~
  arch/powerpc/kernel/stacktrace.c: In function ‘arch_trigger_cpumask_backtrace’:
  arch/powerpc/kernel/stacktrace.c:226:2: error: implicit declaration of function ‘nmi_trigger_cpumask_backtrace’
    226 |  nmi_trigger_cpumask_backtrace(mask, exclude_self, raise_backtrace_ipi);
        |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This happens because our headers haven't defined
arch_trigger_cpumask_backtrace, which causes lib/nmi_backtrace.c not to
build nmi_cpu_backtrace().

The code in question doesn't actually depend on STACKTRACE=y, that was
just added because arch_trigger_cpumask_backtrace() lived in
stacktrace.c for convenience. So drop the dependency on
CONFIG_STACKTRACE, that causes lib/nmi_backtrace.c to build
nmi_cpu_backtrace() etc. and fixes the build.

Fixes: 1614b2b11fab ("arch: Make ARCH_STACKWALK independent of STACKTRACE")
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/nmi.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/nmi.h b/arch/powerpc/include/asm/nmi.h
index 160abcb8e9fa..ea0e487f87b1 100644
--- a/arch/powerpc/include/asm/nmi.h
+++ b/arch/powerpc/include/asm/nmi.h
@@ -9,7 +9,7 @@ long soft_nmi_interrupt(struct pt_regs *regs);
 static inline void arch_touch_nmi_watchdog(void) {}
 #endif
 
-#if defined(CONFIG_NMI_IPI) && defined(CONFIG_STACKTRACE)
+#ifdef CONFIG_NMI_IPI
 extern void arch_trigger_cpumask_backtrace(const cpumask_t *mask,
 					   bool exclude_self);
 #define arch_trigger_cpumask_backtrace arch_trigger_cpumask_backtrace
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH v4 16/20] Kbuild: add Rust support
From: Russell King (Oracle) @ 2022-02-12 14:16 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Sven Van Asbroeck, Catalin Marinas, Dave Hansen, Miguel Cano,
	Paul Mackerras, Gary Guo, Douglas Su, Borislav Petkov,
	linux-riscv, Will Deacon, Thomas Gleixner, H. Peter Anvin,
	Masahiro Yamada, x86, Ingo Molnar, Wedson Almeida Filho,
	Alex Gaynor, Antonio Terceiro, Adam Bratschi-Kaye, Albert Ou,
	rust-for-linux, linux-kbuild, Boqun Feng, linuxppc-dev, Daniel Xu,
	Paul Walmsley, Dariusz Sosnowski, linux-arm-kernel, Michal Marek,
	Greg Kroah-Hartman, Nick Desaulniers, linux-kernel,
	Boris-Chengbiao Zhou, Palmer Dabbelt, Finn Behrens,
	Linus Torvalds
In-Reply-To: <20220212130410.6901-17-ojeda@kernel.org>

On Sat, Feb 12, 2022 at 02:03:42PM +0100, Miguel Ojeda wrote:
> +config RUST
> +	bool "Rust support"
> +	depends on RUST_IS_AVAILABLE
> +	depends on ARM64 || CPU_32v6 || CPU_32v6K || (PPC64 && CPU_LITTLE_ENDIAN) || X86_64 || RISCV

Please don't use CPU_32v6* here.

It probably makes more sense to add a symbol "HAVE_RUST" and have the
appropriate architecture Kconfig files select HAVE_RUST.

Does Rust support Thumb on ARMv6 and ARMv7 architectures?

> +	depends on !MODVERSIONS
> +	depends on !GCC_PLUGIN_RANDSTRUCT
> +	select CONSTRUCTORS
> +	default n

Please remove every utterance of "default n" from your patch; n is the
default default which default defaults to, so you don't need to specify
default n to make the option default to n. It will default to n purely
because n is the default when no default is specified.

> +	help
> +	  Enables Rust support in the kernel.
> +
> +	  This allows other Rust-related options, like drivers written in Rust,
> +	  to be selected.

As Rust doesn't support all the architectures that the kernel supports,
Rust must not be used for core infrastructure.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply

* Re: [PATCH v4 16/20] Kbuild: add Rust support
From: Miguel Ojeda @ 2022-02-12 15:47 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Sven Van Asbroeck, Catalin Marinas, Dave Hansen, Miguel Cano,
	Paul Mackerras, Gary Guo, Douglas Su, Borislav Petkov,
	linux-riscv, Will Deacon, Thomas Gleixner, H. Peter Anvin,
	Masahiro Yamada, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Ingo Molnar, Wedson Almeida Filho, Alex Gaynor, Antonio Terceiro,
	Miguel Ojeda, Adam Bratschi-Kaye, Albert Ou, rust-for-linux,
	Linux Kbuild mailing list, Boqun Feng, linuxppc-dev, Daniel Xu,
	Paul Walmsley, Dariusz Sosnowski, Linux ARM, Michal Marek,
	Greg Kroah-Hartman, Nick Desaulniers, linux-kernel,
	Boris-Chengbiao Zhou, Palmer Dabbelt, Finn Behrens,
	Linus Torvalds
In-Reply-To: <YgfBUhYdLXA46kOX@shell.armlinux.org.uk>

Hi Russell,

On Sat, Feb 12, 2022 at 3:17 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> Please don't use CPU_32v6* here.
>
> It probably makes more sense to add a symbol "HAVE_RUST" and have the
> appropriate architecture Kconfig files select HAVE_RUST.

We can do it whatever way arch maintainers prefer, of course. Why
would you prefer one over the other?

> Does Rust support Thumb on ARMv6 and ARMv7 architectures?

Yes, the main backend is LLVM. Some built-in targets and their support
level are listed here, if you want to take a look:

    https://doc.rust-lang.org/nightly/rustc/platform-support.html

There is also a GCC backend in `rustc` [1], which is making good
progress and may eventually give us access to architectures supported
by GCC. Furthermore, a from-scratch Rust frontend for GCC [2] is being
worked on, though this effort will likely take more time to reach a
point where it may be used for the kernel.

[1] https://github.com/rust-lang/rust/tree/master/compiler/rustc_codegen_gcc
[2] https://github.com/Rust-GCC/gccrs

> Please remove every utterance of "default n" from your patch; n is the
> default default which default defaults to, so you don't need to specify
> default n to make the option default to n. It will default to n purely
> because n is the default when no default is specified.

Certainly. I am curious, though: is there a reason for most of the
other 500+ instances in the kernel tree?

> As Rust doesn't support all the architectures that the kernel supports,
> Rust must not be used for core infrastructure.

Yeah, although I am not sure I understand what you are getting at here.

Cheers,
Miguel

^ permalink raw reply

* Re: [PATCH v4 16/20] Kbuild: add Rust support
From: Russell King (Oracle) @ 2022-02-12 16:18 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Sven Van Asbroeck, Catalin Marinas, Dave Hansen, Miguel Cano,
	Paul Mackerras, Gary Guo, Douglas Su, Borislav Petkov,
	linux-riscv, Will Deacon, Thomas Gleixner, H. Peter Anvin,
	Masahiro Yamada, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Ingo Molnar, Wedson Almeida Filho, Alex Gaynor, Antonio Terceiro,
	Miguel Ojeda, Adam Bratschi-Kaye, Albert Ou, rust-for-linux,
	Linux Kbuild mailing list, Boqun Feng, linuxppc-dev, Daniel Xu,
	Paul Walmsley, Dariusz Sosnowski, Linux ARM, Michal Marek,
	Greg Kroah-Hartman, Nick Desaulniers, linux-kernel,
	Boris-Chengbiao Zhou, Palmer Dabbelt, Finn Behrens,
	Linus Torvalds
In-Reply-To: <CANiq72nMhUH1s0HMeLb+hfp5=u3h20ryC4uqAgB1Znuq52e=Pw@mail.gmail.com>

On Sat, Feb 12, 2022 at 04:47:33PM +0100, Miguel Ojeda wrote:
> Hi Russell,
> 
> On Sat, Feb 12, 2022 at 3:17 PM Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> >
> > Please don't use CPU_32v6* here.
> >
> > It probably makes more sense to add a symbol "HAVE_RUST" and have the
> > appropriate architecture Kconfig files select HAVE_RUST.
> 
> We can do it whatever way arch maintainers prefer, of course. Why
> would you prefer one over the other?

It would be cleaner, rather than the "depends" line getting longer
and longer over time - and if different architecture maintainers
change it, it will lead to conflicts.

> > Does Rust support Thumb on ARMv6 and ARMv7 architectures?
> 
> Yes, the main backend is LLVM. Some built-in targets and their support
> level are listed here, if you want to take a look:

Right, so why made it dependent on CPU_32v6 || CPU_32v6K if ARMv7 is
supported? What about CPU_32v7? What about CPU_32v7M?

I think it would be saner to use the CPU_V6, CPU_V6K, CPU_V7 and maybe
CPU_V7M here - even bettern to select "HAVE_RUST" from these symbols,
since I'm sure you'd start to see the issue behind my "HAVE_RUST"
suggestion as it means having four symbols just for 32-bit ARM on your
dependency line.

>     https://doc.rust-lang.org/nightly/rustc/platform-support.html

Interestingly, it does not list arm-unknown-linux-gnueabihf, which
is the "tuple" commonly used to build 32-bit ARM kernels. 

> > Please remove every utterance of "default n" from your patch; n is the
> > default default which default defaults to, so you don't need to specify
> > default n to make the option default to n. It will default to n purely
> > because n is the default when no default is specified.
> 
> Certainly. I am curious, though: is there a reason for most of the
> other 500+ instances in the kernel tree?

Probably because people incorrectly think it's required or some other
minor reason. As I say:

config FOO
	bool/tristate ...

always defaults to 'n' without needing "default n" to be specified.

Let's do some proper research on this. There are 19148 "config"
statements in the kernel tree, 521 "default n" and 4818 that
specify any kind of "default". That means there are about 14330
config statements that do not specify any kind of default.

So, there are about 27 times more config statements that specify no
default than those that specify "default n", so using the argument
that there are "500+ instances" and therefore should be seen as
correct is completely misguided.

> > As Rust doesn't support all the architectures that the kernel supports,
> > Rust must not be used for core infrastructure.
> 
> Yeah, although I am not sure I understand what you are getting at here.

I mean, if we end up with, e.g. a filesystem coded in Rust, that
filesystem will not be available on architectures that the kernel
supports until either (a) Rust gains support for that architecture
or (b) someone re-codes the filesystem in C - at which point, what
is the point of having Rust in the kernel?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply

* Re: [PATCH v4 16/20] Kbuild: add Rust support
From: John Paul Adrian Glaubitz @ 2022-02-12 18:27 UTC (permalink / raw)
  To: ojeda
  Cc: thesven73, catalin.marinas, dave.hansen, macanroj, paulus, gary,
	d0u9.su, linux-riscv, will, tglx, hpa, masahiroy, x86, linux,
	torvalds, mingo, wedsonaf, alex.gaynor, ark.email, aou,
	rust-for-linux, linux-kbuild, boqun.feng, antonio.terceiro, bp,
	paul.walmsley, me, dsosnowski, linux-arm-kernel, michal.lkml,
	gregkh, ndesaulniers, linux-kernel, bobo1239, palmer, dxu,
	linuxppc-dev
In-Reply-To: <20220212130410.6901-17-ojeda@kernel.org>

Hi!

On Sat, Feb 12, 2022 at 02:03:42PM +0100, Miguel Ojeda wrote:
> +config RUST
> +	bool "Rust support"
> +	depends on RUST_IS_AVAILABLE
> +	depends on ARM64 || CPU_32v6 || CPU_32v6K || (PPC64 && CPU_LITTLE_ENDIAN) || X86_64 || RISCV

Is there any particular reason why this list excludes MIPS*, i386, big-endian
PowerPC and SPARC targets which are already supported by the Rust programming
language?

Are the arch/$ARCH/rust/target.json files everything that's needed for supporting
the other targets?

Thanks,
Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913


^ permalink raw reply

* Re: [PATCH v4 16/20] Kbuild: add Rust support
From: Miguel Ojeda @ 2022-02-12 18:32 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Sven Van Asbroeck, Catalin Marinas, Dave Hansen, Miguel Cano,
	Paul Mackerras, Gary Guo, Douglas Su, Borislav Petkov,
	linux-riscv, Will Deacon, Thomas Gleixner, H. Peter Anvin,
	Masahiro Yamada, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Ingo Molnar, Wedson Almeida Filho, Alex Gaynor, Antonio Terceiro,
	Miguel Ojeda, Adam Bratschi-Kaye, Albert Ou, rust-for-linux,
	Linux Kbuild mailing list, Boqun Feng, linuxppc-dev, Daniel Xu,
	Paul Walmsley, Dariusz Sosnowski, Linux ARM, Michal Marek,
	Greg Kroah-Hartman, Nick Desaulniers, linux-kernel,
	Boris-Chengbiao Zhou, Palmer Dabbelt, Finn Behrens,
	Linus Torvalds
In-Reply-To: <Ygfd82QN/wmSmlHa@shell.armlinux.org.uk>

On Sat, Feb 12, 2022 at 5:19 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> Right, so why made it dependent on CPU_32v6 || CPU_32v6K if ARMv7 is
> supported? What about CPU_32v7? What about CPU_32v7M?
>
> I think it would be saner to use the CPU_V6, CPU_V6K, CPU_V7 and maybe
> CPU_V7M here - even bettern to select "HAVE_RUST" from these symbols,
> since I'm sure you'd start to see the issue behind my "HAVE_RUST"
> suggestion as it means having four symbols just for 32-bit ARM on your
> dependency line.

To support arch variations properly we also have to configure the
compiler via filling a target spec on the fly, but so far we only have
a few static variations as an example. This is one of the missing
parts of the arch support. I will let you know when we have something
ready.

> Interestingly, it does not list arm-unknown-linux-gnueabihf, which
> is the "tuple" commonly used to build 32-bit ARM kernels.

I see it there (Tier 2).

> Probably because people incorrectly think it's required or some other
> minor reason. As I say:

In that case, we should remove them and warn about those instances,
assuming the preferred style is to not have it.

> so using the argument
> that there are "500+ instances" and therefore should be seen as
> correct is completely misguided.

I did not use any such argument.

> I mean, if we end up with, e.g. a filesystem coded in Rust, that
> filesystem will not be available on architectures that the kernel

As long as that filesystem is an optional feature (or as long as there
is a C version), it should be fine.

> supports until either (a) Rust gains support for that architecture

For this, it would be ideal if entities behind some of the
architectures could support LLVM & ClangBuiltLinux, or the GCC Rust
frontend, or the GCC backend for `rustc`.

For instance, Arm is supporting both LLVM and the Rust project.

> or (b) someone re-codes the filesystem in C - at which point, what
> is the point of having Rust in the kernel?

Having a C version of some system does not mean a Rust version would
not offer advantages. In fact, we are adding Rust precisely because we
believe it offers some advantages over C, for both end users and
maintainers.

(Please see the RFC [1], previous discussions, etc.)

[1] https://lore.kernel.org/lkml/20210414184604.23473-1-ojeda@kernel.org/

Cheers,
Miguel

^ permalink raw reply

* Re: [PATCH v4 16/20] Kbuild: add Rust support
From: Miguel Ojeda @ 2022-02-12 18:57 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz
  Cc: Sven Van Asbroeck, Catalin Marinas, Dave Hansen, Miguel Caño,
	Paul Mackerras, Gary Guo, Douglas Su, linux-riscv, Will Deacon,
	Thomas Gleixner, H. Peter Anvin, Masahiro Yamada,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Russell King - ARM Linux admin, Linus Torvalds, Ingo Molnar,
	Wedson Almeida Filho, Alex Gaynor, Miguel Ojeda,
	Adam Bratschi-Kaye, Albert Ou, rust-for-linux,
	Linux Kbuild mailing list, Boqun Feng, Antonio Terceiro,
	Borislav Petkov, Paul Walmsley, Finn Behrens, Dariusz Sosnowski,
	Linux ARM, Michal Marek, Greg KH, Nick Desaulniers, linux-kernel,
	Boris-Chengbiao Zhou, Palmer Dabbelt, Daniel Xu, linuxppc-dev
In-Reply-To: <0396e38b-f681-a035-b6ea-21127fdf5615@physik.fu-berlin.de>

Hi John Paul,

On Sat, Feb 12, 2022 at 7:27 PM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
>
> Is there any particular reason why this list excludes MIPS*, i386, big-endian
> PowerPC and SPARC targets which are already supported by the Rust programming
> language?

The variations we have so far were intended to showcase the Rust
support in several major architectures, rather than cover everything.
But as long as LLVM (& the kernel, i.e. ClangBuiltLinux) supports the
target (and as long there are no compiler/toolchain issues), you
should be able to try it.

> Are the arch/$ARCH/rust/target.json files everything that's needed for supporting
> the other targets?

Mostly -- there is also `rust/kernel/c_types.rs` and you may need to
tweak `rust/compiler_builtins.rs`, but not much more.

Note that for the target spec files, the short-term plan is to
generate dynamically the target spec file according to what the
architecture requests, instead of using these static files. Longer
term, we need a Rust-stable way to setup custom targets from upstream
`rustc` (though it is not clear yet how it will look, e.g. it could be
via command-line flags).

Cheers,
Miguel

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox