* Re: [PATCH] recordmcount: Support empty section from recent binutils
From: Christophe Leroy @ 2021-11-29 17:56 UTC (permalink / raw)
To: Steven Rostedt, Segher Boessenkool
Cc: Peter Zijlstra, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20211129124355.52474d90@gandalf.local.home>
Le 29/11/2021 à 18:43, Steven Rostedt a écrit :
> On Fri, 26 Nov 2021 08:43:23 +0000
> LEROY Christophe <christophe.leroy@csgroup.eu> wrote:
>
>> Le 24/11/2021 à 15:43, Christophe Leroy a écrit :
>>> Looks like recent binutils (2.36 and over ?) may empty some section,
>>> leading to failure like:
>>>
>>> Cannot find symbol for section 11: .text.unlikely.
>>> kernel/kexec_file.o: failed
>>> make[1]: *** [scripts/Makefile.build:287: kernel/kexec_file.o] Error 1
>>>
>>> In order to avoid that, ensure that the section has a content before
>>> returning it's name in has_rel_mcount().
>>
>> This patch doesn't work, on PPC32 I get the following message with this
>> patch applied:
>>
>> [ 0.000000] ftrace: No functions to be traced?
>>
>> Without the patch I get:
>>
>> [ 0.000000] ftrace: allocating 22381 entries in 66 pages
>> [ 0.000000] ftrace: allocated 66 pages with 2 groups
>
> Because of this report, I have not applied this patch (even though I was
> about to push it to Linus).
>
> I'm pulling it from my queue until this gets resolved.
>
I have no idea on how to fix that for the moment.
With GCC 10 (binutils 2.36) an objdump -x on kernel/kexec_file.o gives:
0000000000000000 l d .text.unlikely 0000000000000000 .text.unlikely
0000000000000000 w F .text.unlikely 0000000000000038
.arch_kexec_apply_relocations_add
0000000000000038 w F .text.unlikely 0000000000000038
.arch_kexec_apply_relocations
With GCC 11 (binutils 2.37) the same gives:
0000000000000000 w F .text.unlikely 0000000000000038
.arch_kexec_apply_relocations_add
0000000000000038 w F .text.unlikely 0000000000000038
.arch_kexec_apply_relocations
The problem is that recordmcount drops weak symbols, and it doesn't find
any non-weak symbol in .text.unlikely
Explication given at
https://elixir.bootlin.com/linux/v5.16-rc2/source/scripts/recordmcount.h#L506
I have no idea on what to do.
Thanks
Christophe
^ permalink raw reply
* [PATCH 11/11] locking: Allow to include asm/spinlock_types.h from linux/spinlock_types_raw.h
From: Sebastian Andrzej Siewior @ 2021-11-29 17:46 UTC (permalink / raw)
To: linux-kernel
Cc: Juri Lelli, Rich Felker, linux-ia64, linux-sh, Peter Zijlstra,
Catalin Marinas, Sebastian Andrzej Siewior, Ben Segall,
Max Filippov, Guo Ren, linux-hexagon, Alexander Gordeev,
Will Deacon, linux-riscv, linux-s390, Vincent Guittot, Brian Cain,
Russell King, linux-csky, Ingo Molnar, Mel Gorman, Waiman Long,
Matt Turner, Christian Borntraeger, Heiko Carstens, linux-xtensa,
Albert Ou, Vasily Gorbik, Boqun Feng, Steven Rostedt,
Ivan Kokshaysky, Paul Walmsley, Thomas Gleixner, Dietmar Eggemann,
linux-arm-kernel, Richard Henderson, Chris Zankel, Yoshinori Sato,
linuxppc-dev, Palmer Dabbelt, linux-alpha, Paul Mackerras,
Daniel Bristot de Oliveira
In-Reply-To: <20211129174654.668506-1-bigeasy@linutronix.de>
The printk header file includes ratelimit_types.h for its __ratelimit()
based usage. It is required for the static initializer used in
printk_ratelimited(). It uses a raw_spinlock_t and includes the
spinlock_types.h.
PREEMPT_RT substitutes spinlock_t with a rtmutex based implementation and so
its spinlock_t implmentation (provided by spinlock_rt.h) includes rtmutex.h and
atomic.h which leads to recursive includes where defines are missing.
By including only the raw_spinlock_t defines it avoids the atomic.h
related includes at this stage.
An example on powerpc:
| CALL scripts/atomic/check-atomics.sh
|In file included from include/linux/bug.h:5,
| from include/linux/page-flags.h:10,
| from kernel/bounds.c:10:
|arch/powerpc/include/asm/page_32.h: In function ‘clear_page’:
|arch/powerpc/include/asm/bug.h:87:4: error: implicit declaration of function ‘__WARN’ [-Werror=implicit-function-declaration]
| 87 | __WARN(); \
| | ^~~~~~
|arch/powerpc/include/asm/page_32.h:48:2: note: in expansion of macro ‘WARN_ON’
| 48 | WARN_ON((unsigned long)addr & (L1_CACHE_BYTES - 1));
| | ^~~~~~~
|arch/powerpc/include/asm/bug.h:58:17: error: invalid application of ‘sizeof’ to incomplete type ‘struct bug_entry’
| 58 | "i" (sizeof(struct bug_entry)), \
| | ^~~~~~
|arch/powerpc/include/asm/bug.h:89:3: note: in expansion of macro ‘BUG_ENTRY’
| 89 | BUG_ENTRY(PPC_TLNEI " %4, 0", \
| | ^~~~~~~~~
|arch/powerpc/include/asm/page_32.h:48:2: note: in expansion of macro ‘WARN_ON’
| 48 | WARN_ON((unsigned long)addr & (L1_CACHE_BYTES - 1));
| | ^~~~~~~
|In file included from arch/powerpc/include/asm/ptrace.h:298,
| from arch/powerpc/include/asm/hw_irq.h:12,
| from arch/powerpc/include/asm/irqflags.h:12,
| from include/linux/irqflags.h:16,
| from include/asm-generic/cmpxchg-local.h:6,
| from arch/powerpc/include/asm/cmpxchg.h:526,
| from arch/powerpc/include/asm/atomic.h:11,
| from include/linux/atomic.h:7,
| from include/linux/rwbase_rt.h:6,
| from include/linux/rwlock_types.h:55,
| from include/linux/spinlock_types.h:74,
| from include/linux/ratelimit_types.h:7,
| from include/linux/printk.h:10,
| from include/asm-generic/bug.h:22,
| from arch/powerpc/include/asm/bug.h:109,
| from include/linux/bug.h:5,
| from include/linux/page-flags.h:10,
| from kernel/bounds.c:10:
|include/linux/thread_info.h: In function ‘copy_overflow’:
|include/linux/thread_info.h:210:2: error: implicit declaration of function ‘WARN’ [-Werror=implicit-function-declaration]
| 210 | WARN(1, "Buffer overflow detected (%d < %lu)!\n", size, count);
| | ^~~~
The WARN / BUG include pulls in printk.h and then ptrace.h expects WARN
(from bug.h) which is not yet complete. Even hw_irq.h has WARN_ON()
statements.
On POWERPC64 there are missing atomic64 defines while building 32bit
VDSO:
| VDSO32C arch/powerpc/kernel/vdso32/vgettimeofday.o
|In file included from include/linux/atomic.h:80,
| from include/linux/rwbase_rt.h:6,
| from include/linux/rwlock_types.h:55,
| from include/linux/spinlock_types.h:74,
| from include/linux/ratelimit_types.h:7,
| from include/linux/printk.h:10,
| from include/linux/kernel.h:19,
| from arch/powerpc/include/asm/page.h:11,
| from arch/powerpc/include/asm/vdso/gettimeofday.h:5,
| from include/vdso/datapage.h:137,
| from lib/vdso/gettimeofday.c:5,
| from <command-line>:
|include/linux/atomic-arch-fallback.h: In function ‘arch_atomic64_inc’:
|include/linux/atomic-arch-fallback.h:1447:2: error: implicit declaration of function ‘arch_atomic64_add’; did you mean ‘arch_atomic_add’? [-Werror=impl
|icit-function-declaration]
| 1447 | arch_atomic64_add(1, v);
| | ^~~~~~~~~~~~~~~~~
| | arch_atomic_add
The generic fallback is not included, atomics itself are not used. If
kernel.h does not include printk.h then it comes later from the bug.h
include.
Allow asm/spinlock_types.h to be included from
linux/spinlock_types_raw.h.
Cc: Albert Ou <aou@eecs.berkeley.edu>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Brian Cain <bcain@codeaurora.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Chris Zankel <chris@zankel.net>
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
Cc: Guo Ren <guoren@kernel.org>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Rich Felker <dalias@libc.org>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: linux-alpha@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-csky@vger.kernel.org
Cc: linux-hexagon@vger.kernel.org
Cc: linux-ia64@vger.kernel.org
Cc: linux-riscv@lists.infradead.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: linux-xtensa@linux-xtensa.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
arch/alpha/include/asm/spinlock_types.h | 2 +-
arch/arm/include/asm/spinlock_types.h | 2 +-
arch/arm64/include/asm/spinlock_types.h | 2 +-
arch/csky/include/asm/spinlock_types.h | 2 +-
arch/hexagon/include/asm/spinlock_types.h | 2 +-
arch/ia64/include/asm/spinlock_types.h | 2 +-
arch/powerpc/include/asm/simple_spinlock_types.h | 2 +-
arch/powerpc/include/asm/spinlock_types.h | 2 +-
arch/riscv/include/asm/spinlock_types.h | 2 +-
arch/s390/include/asm/spinlock_types.h | 2 +-
arch/sh/include/asm/spinlock_types.h | 2 +-
arch/xtensa/include/asm/spinlock_types.h | 2 +-
include/linux/ratelimit_types.h | 2 +-
include/linux/spinlock_types_up.h | 2 +-
14 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/arch/alpha/include/asm/spinlock_types.h b/arch/alpha/include/asm/spinlock_types.h
index 1d5716bc060be..2526fd3be5fd7 100644
--- a/arch/alpha/include/asm/spinlock_types.h
+++ b/arch/alpha/include/asm/spinlock_types.h
@@ -2,7 +2,7 @@
#ifndef _ALPHA_SPINLOCK_TYPES_H
#define _ALPHA_SPINLOCK_TYPES_H
-#ifndef __LINUX_SPINLOCK_TYPES_H
+#ifndef __LINUX_SPINLOCK_TYPES_RAW_H
# error "please don't include this file directly"
#endif
diff --git a/arch/arm/include/asm/spinlock_types.h b/arch/arm/include/asm/spinlock_types.h
index 5976958647fe1..0c14b36ef1013 100644
--- a/arch/arm/include/asm/spinlock_types.h
+++ b/arch/arm/include/asm/spinlock_types.h
@@ -2,7 +2,7 @@
#ifndef __ASM_SPINLOCK_TYPES_H
#define __ASM_SPINLOCK_TYPES_H
-#ifndef __LINUX_SPINLOCK_TYPES_H
+#ifndef __LINUX_SPINLOCK_TYPES_RAW_H
# error "please don't include this file directly"
#endif
diff --git a/arch/arm64/include/asm/spinlock_types.h b/arch/arm64/include/asm/spinlock_types.h
index 18782f0c47212..11ab1c0776977 100644
--- a/arch/arm64/include/asm/spinlock_types.h
+++ b/arch/arm64/include/asm/spinlock_types.h
@@ -5,7 +5,7 @@
#ifndef __ASM_SPINLOCK_TYPES_H
#define __ASM_SPINLOCK_TYPES_H
-#if !defined(__LINUX_SPINLOCK_TYPES_H) && !defined(__ASM_SPINLOCK_H)
+#if !defined(__LINUX_SPINLOCK_TYPES_RAW_H) && !defined(__ASM_SPINLOCK_H)
# error "please don't include this file directly"
#endif
diff --git a/arch/csky/include/asm/spinlock_types.h b/arch/csky/include/asm/spinlock_types.h
index 8ff0f6ff3a006..db87a12c3827d 100644
--- a/arch/csky/include/asm/spinlock_types.h
+++ b/arch/csky/include/asm/spinlock_types.h
@@ -3,7 +3,7 @@
#ifndef __ASM_CSKY_SPINLOCK_TYPES_H
#define __ASM_CSKY_SPINLOCK_TYPES_H
-#ifndef __LINUX_SPINLOCK_TYPES_H
+#ifndef __LINUX_SPINLOCK_TYPES_RAW_H
# error "please don't include this file directly"
#endif
diff --git a/arch/hexagon/include/asm/spinlock_types.h b/arch/hexagon/include/asm/spinlock_types.h
index 19d233497ba52..d5f66495b670f 100644
--- a/arch/hexagon/include/asm/spinlock_types.h
+++ b/arch/hexagon/include/asm/spinlock_types.h
@@ -8,7 +8,7 @@
#ifndef _ASM_SPINLOCK_TYPES_H
#define _ASM_SPINLOCK_TYPES_H
-#ifndef __LINUX_SPINLOCK_TYPES_H
+#ifndef __LINUX_SPINLOCK_TYPES_RAW_H
# error "please don't include this file directly"
#endif
diff --git a/arch/ia64/include/asm/spinlock_types.h b/arch/ia64/include/asm/spinlock_types.h
index 6e345fefcdcab..14b8a161c1652 100644
--- a/arch/ia64/include/asm/spinlock_types.h
+++ b/arch/ia64/include/asm/spinlock_types.h
@@ -2,7 +2,7 @@
#ifndef _ASM_IA64_SPINLOCK_TYPES_H
#define _ASM_IA64_SPINLOCK_TYPES_H
-#ifndef __LINUX_SPINLOCK_TYPES_H
+#ifndef __LINUX_SPINLOCK_TYPES_RAW_H
# error "please don't include this file directly"
#endif
diff --git a/arch/powerpc/include/asm/simple_spinlock_types.h b/arch/powerpc/include/asm/simple_spinlock_types.h
index 0f3cdd8faa959..08243338069d2 100644
--- a/arch/powerpc/include/asm/simple_spinlock_types.h
+++ b/arch/powerpc/include/asm/simple_spinlock_types.h
@@ -2,7 +2,7 @@
#ifndef _ASM_POWERPC_SIMPLE_SPINLOCK_TYPES_H
#define _ASM_POWERPC_SIMPLE_SPINLOCK_TYPES_H
-#ifndef __LINUX_SPINLOCK_TYPES_H
+#ifndef __LINUX_SPINLOCK_TYPES_RAW_H
# error "please don't include this file directly"
#endif
diff --git a/arch/powerpc/include/asm/spinlock_types.h b/arch/powerpc/include/asm/spinlock_types.h
index c5d742f18021d..d5f8a74ed2e8c 100644
--- a/arch/powerpc/include/asm/spinlock_types.h
+++ b/arch/powerpc/include/asm/spinlock_types.h
@@ -2,7 +2,7 @@
#ifndef _ASM_POWERPC_SPINLOCK_TYPES_H
#define _ASM_POWERPC_SPINLOCK_TYPES_H
-#ifndef __LINUX_SPINLOCK_TYPES_H
+#ifndef __LINUX_SPINLOCK_TYPES_RAW_H
# error "please don't include this file directly"
#endif
diff --git a/arch/riscv/include/asm/spinlock_types.h b/arch/riscv/include/asm/spinlock_types.h
index f398e7638dd63..5a35a49505da2 100644
--- a/arch/riscv/include/asm/spinlock_types.h
+++ b/arch/riscv/include/asm/spinlock_types.h
@@ -6,7 +6,7 @@
#ifndef _ASM_RISCV_SPINLOCK_TYPES_H
#define _ASM_RISCV_SPINLOCK_TYPES_H
-#ifndef __LINUX_SPINLOCK_TYPES_H
+#ifndef __LINUX_SPINLOCK_TYPES_RAW_H
# error "please don't include this file directly"
#endif
diff --git a/arch/s390/include/asm/spinlock_types.h b/arch/s390/include/asm/spinlock_types.h
index a2bbfd7df85fa..b69695e399574 100644
--- a/arch/s390/include/asm/spinlock_types.h
+++ b/arch/s390/include/asm/spinlock_types.h
@@ -2,7 +2,7 @@
#ifndef __ASM_SPINLOCK_TYPES_H
#define __ASM_SPINLOCK_TYPES_H
-#ifndef __LINUX_SPINLOCK_TYPES_H
+#ifndef __LINUX_SPINLOCK_TYPES_RAW_H
# error "please don't include this file directly"
#endif
diff --git a/arch/sh/include/asm/spinlock_types.h b/arch/sh/include/asm/spinlock_types.h
index e82369f286a20..907bda4b1619a 100644
--- a/arch/sh/include/asm/spinlock_types.h
+++ b/arch/sh/include/asm/spinlock_types.h
@@ -2,7 +2,7 @@
#ifndef __ASM_SH_SPINLOCK_TYPES_H
#define __ASM_SH_SPINLOCK_TYPES_H
-#ifndef __LINUX_SPINLOCK_TYPES_H
+#ifndef __LINUX_SPINLOCK_TYPES_RAW_H
# error "please don't include this file directly"
#endif
diff --git a/arch/xtensa/include/asm/spinlock_types.h b/arch/xtensa/include/asm/spinlock_types.h
index 64c9389254f13..797aed7df3dd8 100644
--- a/arch/xtensa/include/asm/spinlock_types.h
+++ b/arch/xtensa/include/asm/spinlock_types.h
@@ -2,7 +2,7 @@
#ifndef __ASM_SPINLOCK_TYPES_H
#define __ASM_SPINLOCK_TYPES_H
-#if !defined(__LINUX_SPINLOCK_TYPES_H) && !defined(__ASM_SPINLOCK_H)
+#if !defined(__LINUX_SPINLOCK_TYPES_RAW_H) && !defined(__ASM_SPINLOCK_H)
# error "please don't include this file directly"
#endif
diff --git a/include/linux/ratelimit_types.h b/include/linux/ratelimit_types.h
index b676aa419eef8..c21c7f8103e2b 100644
--- a/include/linux/ratelimit_types.h
+++ b/include/linux/ratelimit_types.h
@@ -4,7 +4,7 @@
#include <linux/bits.h>
#include <linux/param.h>
-#include <linux/spinlock_types.h>
+#include <linux/spinlock_types_raw.h>
#define DEFAULT_RATELIMIT_INTERVAL (5 * HZ)
#define DEFAULT_RATELIMIT_BURST 10
diff --git a/include/linux/spinlock_types_up.h b/include/linux/spinlock_types_up.h
index c09b6407ae1b3..7f86a2016ac5c 100644
--- a/include/linux/spinlock_types_up.h
+++ b/include/linux/spinlock_types_up.h
@@ -1,7 +1,7 @@
#ifndef __LINUX_SPINLOCK_TYPES_UP_H
#define __LINUX_SPINLOCK_TYPES_UP_H
-#ifndef __LINUX_SPINLOCK_TYPES_H
+#ifndef __LINUX_SPINLOCK_TYPES_RAW_H
# error "please don't include this file directly"
#endif
--
2.34.0
^ permalink raw reply related
* Re: [PATCH] powerpc/pseries/vas: Don't print an error when VAS is unavailable
From: Tyrel Datwyler @ 2021-11-29 18:13 UTC (permalink / raw)
To: Nicholas Piggin, Cédric Le Goater, linuxppc-dev; +Cc: Haren Myneni
In-Reply-To: <1637922573.8ofrolskkj.astroid@bobo.none>
On 11/26/21 2:31 AM, Nicholas Piggin wrote:
> Excerpts from Cédric Le Goater's message of November 26, 2021 5:13 pm:
>> On 11/26/21 06:21, Nicholas Piggin wrote:
>>> KVM does not support VAS so guests always print a useless error on boot
>>>
>>> vas: HCALL(398) error -2, query_type 0, result buffer 0x57f2000
>>>
>>> Change this to only print the message if the error is not H_FUNCTION.
>>
>>
>> Just being curious, why is it even called since "ibm,compression" should
>> not be exposed in the DT ?
>
> It looks like vas does not test for it. I guess in theory there can be
> other functions than compression implemented as an accelerator. Maybe
> that's why?
>
> Thanks,
> Nick
>
Looks like pseries_vas_init() simply calls h_query_vas_capabilities() to test
for VAS coprocessor support. I would assume KVM doesn't expose hcall-vas or
hcall-nx in /rtas/ibm,hypertas-functions? Doesn't look like hcall-vas or
hcall-nx have been added to the hypertas_fw_feature matching, but maybe they
should and we can gate VAS initialization on those, or at the minimum
FW_FEATURE_VAS?
-Tyrel
^ permalink raw reply
* [Bug 205099] KASAN hit at raid6_pq: BUG: Unable to handle kernel data access at 0x00f0fd0d
From: bugzilla-daemon @ 2021-11-29 19:46 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-205099-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=205099
--- Comment #46 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 299775
--> https://bugzilla.kernel.org/attachment.cgi?id=299775&action=edit
kernel_page_tables (5.15.5, INLINE KASAN, LOWMEM_SIZE=0x20000000, PowerMac G4
DP)
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 205099] KASAN hit at raid6_pq: BUG: Unable to handle kernel data access at 0x00f0fd0d
From: bugzilla-daemon @ 2021-11-29 19:46 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-205099-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=205099
--- Comment #45 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 299773
--> https://bugzilla.kernel.org/attachment.cgi?id=299773&action=edit
kernel_page_tables (5.15.5, OUTLINE KASAN, LOWMEM_SIZE=0x30000000, PowerMac G4
DP)
Ah yes, I forgot about including the /sys/kernel/debug/kernel_page_tables..
Sorry! Here you are.
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 215169] New: UBSAN: shift-out-of-bounds in arch/powerpc/mm/kasan/book3s_32.c:22:23
From: bugzilla-daemon @ 2021-11-29 20:06 UTC (permalink / raw)
To: linuxppc-dev
https://bugzilla.kernel.org/show_bug.cgi?id=215169
Bug ID: 215169
Summary: UBSAN: shift-out-of-bounds in
arch/powerpc/mm/kasan/book3s_32.c:22:23
Product: Platform Specific/Hardware
Version: 2.5
Kernel Version: 5.15.5
Hardware: PPC-32
OS: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: PPC-32
Assignee: platform_ppc-32@kernel-bugs.osdl.org
Reporter: erhard_f@mailbox.org
Regression: No
Created attachment 299777
--> https://bugzilla.kernel.org/attachment.cgi?id=299777&action=edit
dmesg (5.15.5, INLINE KASAN, PowerMac G4 DP)
Noticed another small glitch during 5.15.x testing. Happens when inline KASAN
is selected:
Total memory = 2048MB; using 4096kB for hash table
Activating Kernel Userspace Access Protection
Linux version 5.15.5-gentoo-PowerMacG4 (root@T1000) (gcc (Gentoo 11.2.0 p1)
11.2.0, GNU ld (Gentoo 2.37_p1 p0) 2.37) #9 SMP Mon Nov 29 20:46:44 CET 2021
================================================================================
UBSAN: shift-out-of-bounds in arch/powerpc/mm/kasan/book3s_32.c:22:23
shift exponent -1 is negative
CPU: 0 PID: 0 Comm: swapper Not tainted 5.15.5-gentoo-PowerMacG4 #9
Call Trace:
[c214be60] [c0ba0048] dump_stack_lvl+0x80/0xb0 (unreliable)
[c214be80] [c0b99288] ubsan_epilogue+0x10/0x5c
[c214be90] [c0b98fe0] __ubsan_handle_shift_out_of_bounds+0x94/0x138
[c214bf00] [c1c0f010] kasan_init_region+0xd8/0x26c
[c214bf30] [c1c0ed84] kasan_init+0xc0/0x198
[c214bf70] [c1c08024] setup_arch+0x18/0x54c
[c214bfc0] [c1c037f0] start_kernel+0x90/0x33c
[c214bff0] [00003610] 0x3610
================================================================================
setbat: no BAT available for mapping 0x1c000000
KASAN init done
[...]
The other UBSAN complaint in the netconsole.log is bug #214867.
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 215169] UBSAN: shift-out-of-bounds in arch/powerpc/mm/kasan/book3s_32.c:22:23
From: bugzilla-daemon @ 2021-11-29 20:09 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-215169-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=215169
--- Comment #1 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 299779
--> https://bugzilla.kernel.org/attachment.cgi?id=299779&action=edit
kernel .config (5.15.5, PowerMac G4 DP)
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [patch 05/22] genirq/msi: Fixup includes
From: Thomas Gleixner @ 2021-11-29 21:38 UTC (permalink / raw)
To: Cédric Le Goater, LKML
Cc: linux-hyperv, linux-pci, Paul Mackerras, sparclinux, Wei Liu,
Ashok Raj, Marc Zygnier, x86, Christian Borntraeger,
Bjorn Helgaas, Jason Gunthorpe, xen-devel, ath11k, Kevin Tian,
Heiko Carstens, Alex Williamson, Megha Dey, Juergen Gross,
Thomas Bogendoerfer, Greg Kroah-Hartman, linux-mips, linuxppc-dev
In-Reply-To: <b1a6d267-c7b4-c4b9-ab0e-f5cc32bfe9bf@kaod.org>
Cedric,
On Mon, Nov 29 2021 at 08:33, Cédric Le Goater wrote:
> On 11/27/21 02:18, Thomas Gleixner wrote:
>> Remove the kobject.h include from msi.h as it's not required and add a
>> sysfs.h include to the core code instead.
>>
>> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>
>
> This patch breaks compile on powerpc :
>
> CC arch/powerpc/kernel/msi.o
> In file included from ../arch/powerpc/kernel/msi.c:7:
> ../include/linux/msi.h:410:65: error: ‘struct cpumask’ declared inside parameter list will not be visible outside of this definition or declaration [-Werror]
> 410 | int msi_domain_set_affinity(struct irq_data *data, const struct cpumask *mask,
> | ^~~~~~~
> cc1: all warnings being treated as errors
>
> Below is fix you can merge in patch 5.
thanks for having a look. I fixed up this and other fallout and pushed out an
updated series (all 4 parts) to:
git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel msi
Thanks,
tglx
^ permalink raw reply
* Re: [RFC PATCH 0/3] Use pageblock_order for cma and alloc_contig_range alignment.
From: Zi Yan @ 2021-11-29 22:08 UTC (permalink / raw)
To: Vlastimil Babka
Cc: David Hildenbrand, linuxppc-dev, linux-kernel, virtualization,
linux-mm, iommu, Robin Murphy, Christoph Hellwig,
Marek Szyprowski
In-Reply-To: <52dbf824-76be-cc34-3983-d45510b1b618@suse.cz>
[-- Attachment #1: Type: text/plain, Size: 2062 bytes --]
On 23 Nov 2021, at 12:32, Vlastimil Babka wrote:
> On 11/23/21 17:35, Zi Yan wrote:
>> On 19 Nov 2021, at 10:15, Zi Yan wrote:
>>>>> From what my understanding, cma required alignment of
>>>>> max(MAX_ORDER - 1, pageblock_order), because when MIGRATE_CMA was introduced,
>>>>> __free_one_page() does not prevent merging two different pageblocks, when
>>>>> MAX_ORDER - 1 > pageblock_order. But current __free_one_page() implementation
>>>>> does prevent that.
>>>>
>>>> But it does prevent that only for isolated pageblock, not CMA, and yout
>>>> patchset doesn't seem to expand that to CMA? Or am I missing something.
>>>
>>> Yeah, you are right. Originally, I thought preventing merging isolated pageblock
>>> with other types of pageblocks is sufficient, since MIGRATE_CMA is always
>>> converted from MIGRATE_ISOLATE. But that is not true. I will rework the code.
>>> Thanks for pointing this out.
>>>
>>
>> I find that two pageblocks with different migratetypes, like MIGRATE_RECLAIMABLE
>> and MIGRATE_MOVABLE can be merged into a single free page after I checked
>> __free_one_page() in detail and printed pageblock information during buddy page
>> merging.
>
> Yes, that can happen.
>
> I am not sure what consequence it will cause. Do you have any idea?
>
> For MIGRATE_RECLAIMABLE or MIGRATE_MOVABLE or even MIGRATE_UNMOVABLE it's
> absolutely fine. As long as these pageblocks are fully free (and they are if
> it's a single free page spanning 2 pageblocks), they can be of any of these
> type, as they can be reused as needed without causing fragmentation.
>
> But in case of MIGRATE_CMA and MIGRATE_ISOLATE, uncontrolled merging would
> break the specifics of those types. That's why the code is careful for
> MIGRATE_ISOLATE, and MIGRATE_CMA was until now done in MAX_ORDER granularity.
Thanks for the explanation. Basically migratetypes that can fall back to each
other can be merged into a single free page, right?
How about MIGRATE_HIGHATOMIC? It should not be merged with other migratetypes
from my understanding.
--
Best Regards,
Yan, Zi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 854 bytes --]
^ permalink raw reply
* Re: [PATCH RFC 1/4] mm: percpu: Generalize percpu related config
From: Dennis Zhou @ 2021-11-29 22:36 UTC (permalink / raw)
To: Kefeng Wang
Cc: linux-ia64, dave.hansen, linux-mips, linux-mm, paulus, hpa,
sparclinux, cl, will, linux-riscv, x86, mingo, catalin.marinas,
dennis, aou, bp, paul.walmsley, tglx, linux-arm-kernel, tsbogend,
gregkh, linux-kernel, palmer, tj, akpm, linuxppc-dev, davem
In-Reply-To: <20211121093557.139034-2-wangkefeng.wang@huawei.com>
Hello,
On Sun, Nov 21, 2021 at 05:35:54PM +0800, Kefeng Wang wrote:
> The HAVE_SETUP_PER_CPU_AREA/NEED_PER_CPU_EMBED_FIRST_CHUNK/
> NEED_PER_CPU_PAGE_FIRST_CHUNK/USE_PERCPU_NUMA_NODE_ID configs,
> which has duplicate definitions on platforms that subscribe it.
>
> Move them into mm, drop these redundant definitions and instead
> just select it on applicable platforms.
>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
> arch/arm64/Kconfig | 20 ++++----------------
> arch/ia64/Kconfig | 9 ++-------
> arch/mips/Kconfig | 10 ++--------
> arch/powerpc/Kconfig | 17 ++++-------------
> arch/riscv/Kconfig | 10 ++--------
> arch/sparc/Kconfig | 12 +++---------
> arch/x86/Kconfig | 17 ++++-------------
> mm/Kconfig | 12 ++++++++++++
> 8 files changed, 33 insertions(+), 74 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index c4207cf9bb17..4ff73299f8a9 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1135,6 +1135,10 @@ config NUMA
> select GENERIC_ARCH_NUMA
> select ACPI_NUMA if ACPI
> select OF_NUMA
> + select HAVE_SETUP_PER_CPU_AREA
> + select NEED_PER_CPU_EMBED_FIRST_CHUNK
> + select NEED_PER_CPU_PAGE_FIRST_CHUNK
> + select USE_PERCPU_NUMA_NODE_ID
> help
> Enable NUMA (Non-Uniform Memory Access) support.
>
> @@ -1151,22 +1155,6 @@ config NODES_SHIFT
> Specify the maximum number of NUMA Nodes available on the target
> system. Increases memory reserved to accommodate various tables.
>
> -config USE_PERCPU_NUMA_NODE_ID
> - def_bool y
> - depends on NUMA
> -
> -config HAVE_SETUP_PER_CPU_AREA
> - def_bool y
> - depends on NUMA
> -
> -config NEED_PER_CPU_EMBED_FIRST_CHUNK
> - def_bool y
> - depends on NUMA
> -
> -config NEED_PER_CPU_PAGE_FIRST_CHUNK
> - def_bool y
> - depends on NUMA
> -
> source "kernel/Kconfig.hz"
>
> config ARCH_SPARSEMEM_ENABLE
> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
> index 1e33666fa679..703952819e10 100644
> --- a/arch/ia64/Kconfig
> +++ b/arch/ia64/Kconfig
> @@ -32,6 +32,7 @@ config IA64
> select HAVE_FTRACE_MCOUNT_RECORD
> select HAVE_DYNAMIC_FTRACE if (!ITANIUM)
> select HAVE_FUNCTION_TRACER
> + select HAVE_SETUP_PER_CPU_AREA
> select TTY
> select HAVE_ARCH_TRACEHOOK
> select HAVE_VIRT_CPU_ACCOUNTING
> @@ -88,9 +89,6 @@ config GENERIC_CALIBRATE_DELAY
> bool
> default y
>
> -config HAVE_SETUP_PER_CPU_AREA
> - def_bool y
> -
> config DMI
> bool
> default y
> @@ -292,6 +290,7 @@ config NUMA
> bool "NUMA support"
> depends on !FLATMEM
> select SMP
> + select USE_PERCPU_NUMA_NODE_ID
> help
> Say Y to compile the kernel to support NUMA (Non-Uniform Memory
> Access). This option is for configuring high-end multiprocessor
> @@ -311,10 +310,6 @@ config HAVE_ARCH_NODEDATA_EXTENSION
> def_bool y
> depends on NUMA
>
> -config USE_PERCPU_NUMA_NODE_ID
> - def_bool y
> - depends on NUMA
> -
> config HAVE_MEMORYLESS_NODES
> def_bool NUMA
>
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index de60ad190057..c106a2080877 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -2666,6 +2666,8 @@ config NUMA
> bool "NUMA Support"
> depends on SYS_SUPPORTS_NUMA
> select SMP
> + select HAVE_SETUP_PER_CPU_AREA
> + select NEED_PER_CPU_EMBED_FIRST_CHUNK
> help
> Say Y to compile the kernel to support NUMA (Non-Uniform Memory
> Access). This option improves performance on systems with more
> @@ -2676,14 +2678,6 @@ config NUMA
> config SYS_SUPPORTS_NUMA
> bool
>
> -config HAVE_SETUP_PER_CPU_AREA
> - def_bool y
> - depends on NUMA
> -
> -config NEED_PER_CPU_EMBED_FIRST_CHUNK
> - def_bool y
> - depends on NUMA
> -
> config RELOCATABLE
> bool "Relocatable kernel"
> depends on SYS_SUPPORTS_RELOCATABLE
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index dea74d7717c0..8badd39854a0 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -55,15 +55,6 @@ config ARCH_MMAP_RND_COMPAT_BITS_MIN
> default 9 if PPC_16K_PAGES # 9 = 23 (8MB) - 14 (16K)
> default 11 # 11 = 23 (8MB) - 12 (4K)
>
> -config HAVE_SETUP_PER_CPU_AREA
> - def_bool PPC64
> -
> -config NEED_PER_CPU_EMBED_FIRST_CHUNK
> - def_bool y if PPC64
> -
> -config NEED_PER_CPU_PAGE_FIRST_CHUNK
> - def_bool y if PPC64
> -
> config NR_IRQS
> int "Number of virtual interrupt numbers"
> range 32 1048576
> @@ -240,6 +231,7 @@ config PPC
> select HAVE_REGS_AND_STACK_ACCESS_API
> select HAVE_RELIABLE_STACKTRACE
> select HAVE_RSEQ
> + select HAVE_SETUP_PER_CPU_AREA if PPC64
> select HAVE_SOFTIRQ_ON_OWN_STACK
> select HAVE_STACKPROTECTOR if PPC32 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r2)
> select HAVE_STACKPROTECTOR if PPC64 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r13)
> @@ -254,6 +246,8 @@ config PPC
> select MMU_GATHER_RCU_TABLE_FREE
> select MODULES_USE_ELF_RELA
> select NEED_DMA_MAP_STATE if PPC64 || NOT_COHERENT_CACHE
> + select NEED_PER_CPU_EMBED_FIRST_CHUNK if PPC64
> + select NEED_PER_CPU_PAGE_FIRST_CHUNK if PPC64
> select NEED_SG_DMA_LENGTH
> select OF
> select OF_DMA_DEFAULT_COHERENT if !NOT_COHERENT_CACHE
> @@ -659,6 +653,7 @@ config NUMA
> bool "NUMA Memory Allocation and Scheduler Support"
> depends on PPC64 && SMP
> default y if PPC_PSERIES || PPC_POWERNV
> + select USE_PERCPU_NUMA_NODE_ID
> help
> Enable NUMA (Non-Uniform Memory Access) support.
>
> @@ -672,10 +667,6 @@ config NODES_SHIFT
> default "4"
> depends on NUMA
>
> -config USE_PERCPU_NUMA_NODE_ID
> - def_bool y
> - depends on NUMA
> -
> config HAVE_MEMORYLESS_NODES
> def_bool y
> depends on NUMA
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 821252b65f89..bf66bcbc5a39 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -334,6 +334,8 @@ config NUMA
> select GENERIC_ARCH_NUMA
> select OF_NUMA
> select ARCH_SUPPORTS_NUMA_BALANCING
> + select USE_PERCPU_NUMA_NODE_ID
> + select NEED_PER_CPU_EMBED_FIRST_CHUNK
> help
> Enable NUMA (Non-Uniform Memory Access) support.
>
> @@ -349,14 +351,6 @@ config NODES_SHIFT
> Specify the maximum number of NUMA Nodes available on the target
> system. Increases memory reserved to accommodate various tables.
>
> -config USE_PERCPU_NUMA_NODE_ID
> - def_bool y
> - depends on NUMA
> -
> -config NEED_PER_CPU_EMBED_FIRST_CHUNK
> - def_bool y
> - depends on NUMA
> -
> config RISCV_ISA_C
> bool "Emit compressed instructions when building Linux"
> default y
> diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
> index 66fc08646be5..a6765e0fe6a8 100644
> --- a/arch/sparc/Kconfig
> +++ b/arch/sparc/Kconfig
> @@ -97,6 +97,9 @@ config SPARC64
> select PCI_DOMAINS if PCI
> select ARCH_HAS_GIGANTIC_PAGE
> select HAVE_SOFTIRQ_ON_OWN_STACK
> + select HAVE_SETUP_PER_CPU_AREA
> + select NEED_PER_CPU_EMBED_FIRST_CHUNK
> + select NEED_PER_CPU_PAGE_FIRST_CHUUNK
>
> config ARCH_PROC_KCORE_TEXT
> def_bool y
> @@ -123,15 +126,6 @@ config AUDIT_ARCH
> bool
> default y
>
> -config HAVE_SETUP_PER_CPU_AREA
> - def_bool y if SPARC64
> -
> -config NEED_PER_CPU_EMBED_FIRST_CHUNK
> - def_bool y if SPARC64
> -
> -config NEED_PER_CPU_PAGE_FIRST_CHUNK
> - def_bool y if SPARC64
> -
> config MMU
> bool
> default y
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 7399327d1eff..ca120a1f5857 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -239,6 +239,7 @@ config X86
> select HAVE_REGS_AND_STACK_ACCESS_API
> select HAVE_RELIABLE_STACKTRACE if X86_64 && (UNWINDER_FRAME_POINTER || UNWINDER_ORC) && STACK_VALIDATION
> select HAVE_FUNCTION_ARG_ACCESS_API
> + select HAVE_SETUP_PER_CPU_AREA
> select HAVE_SOFTIRQ_ON_OWN_STACK
> select HAVE_STACKPROTECTOR if CC_HAS_SANE_STACKPROTECTOR
> select HAVE_STACK_VALIDATION if X86_64
> @@ -252,6 +253,8 @@ config X86
> select HAVE_GENERIC_VDSO
> select HOTPLUG_SMT if SMP
> select IRQ_FORCED_THREADING
> + select NEED_PER_CPU_EMBED_FIRST_CHUNK
> + select NEED_PER_CPU_PAGE_FIRST_CHUNK
> select NEED_SG_DMA_LENGTH
> select PCI_DOMAINS if PCI
> select PCI_LOCKLESS_CONFIG if PCI
> @@ -331,15 +334,6 @@ config ARCH_HAS_CPU_RELAX
> config ARCH_HAS_FILTER_PGPROT
> def_bool y
>
> -config HAVE_SETUP_PER_CPU_AREA
> - def_bool y
> -
> -config NEED_PER_CPU_EMBED_FIRST_CHUNK
> - def_bool y
> -
> -config NEED_PER_CPU_PAGE_FIRST_CHUNK
> - def_bool y
> -
> config ARCH_HIBERNATION_POSSIBLE
> def_bool y
>
> @@ -1557,6 +1551,7 @@ config NUMA
> depends on SMP
> depends on X86_64 || (X86_32 && HIGHMEM64G && X86_BIGSMP)
> default y if X86_BIGSMP
> + select USE_PERCPU_NUMA_NODE_ID
> help
> Enable NUMA (Non-Uniform Memory Access) support.
>
> @@ -2430,10 +2425,6 @@ config ARCH_HAS_ADD_PAGES
> config ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE
> def_bool y
>
> -config USE_PERCPU_NUMA_NODE_ID
> - def_bool y
> - depends on NUMA
> -
> menu "Power management and ACPI options"
>
> config ARCH_HIBERNATION_HEADER
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 28edafc820ad..6bc5d780c51b 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -432,6 +432,18 @@ config NEED_PER_CPU_KM
> bool
> default y
>
> +config NEED_PER_CPU_EMBED_FIRST_CHUNK
> + bool
> +
> +config NEED_PER_CPU_PAGE_FIRST_CHUNK
> + bool
> +
> +config USE_PERCPU_NUMA_NODE_ID
> + bool
> +
> +config HAVE_SETUP_PER_CPU_AREA
> + bool
> +
> config CLEANCACHE
> bool "Enable cleancache driver to cache clean pages if tmem is present"
> help
> --
> 2.26.2
>
This makes sense and looks good. A series like this is a little tricky.
The latter patches change the contracts so it'd be easiest to run it
through my tree. We'd need to get explicit acks from each arch
maintainer to make sure they're fine with this.
Thanks,
Dennis
^ permalink raw reply
* Re: [PATCH RFC 2/4] mm: percpu: Add pcpu_fc_cpu_to_node_fn_t typedef
From: Dennis Zhou @ 2021-11-29 22:40 UTC (permalink / raw)
To: Kefeng Wang
Cc: linux-ia64, dave.hansen, linux-mips, linux-mm, paulus, hpa,
sparclinux, cl, will, linux-riscv, x86, mingo, catalin.marinas,
aou, bp, paul.walmsley, tglx, linux-arm-kernel, tsbogend, gregkh,
linux-kernel, palmer, tj, akpm, linuxppc-dev, davem
In-Reply-To: <20211121093557.139034-3-wangkefeng.wang@huawei.com>
On Sun, Nov 21, 2021 at 05:35:55PM +0800, Kefeng Wang wrote:
> Add pcpu_fc_cpu_to_node_fn_t and pass it into pcpu_fc_alloc_fn_t,
> pcpu first chunk allocation will call it to alloc memblock on the
> corresponding node by it.
>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
> arch/mips/mm/init.c | 12 +++++++++---
> arch/powerpc/kernel/setup_64.c | 14 +++++++++++---
> arch/sparc/kernel/smp_64.c | 8 +++++---
> arch/x86/kernel/setup_percpu.c | 18 +++++++++++++-----
> drivers/base/arch_numa.c | 8 +++++---
> include/linux/percpu.h | 7 +++++--
> mm/percpu.c | 14 +++++++++-----
> 7 files changed, 57 insertions(+), 24 deletions(-)
>
> diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
> index 325e1552cbea..ebbf6923532c 100644
> --- a/arch/mips/mm/init.c
> +++ b/arch/mips/mm/init.c
> @@ -519,12 +519,17 @@ static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
> return node_distance(cpu_to_node(from), cpu_to_node(to));
> }
>
> -static void * __init pcpu_fc_alloc(unsigned int cpu, size_t size,
> - size_t align)
> +static int __init pcpu_cpu_to_node(int cpu)
> +{
> + return cpu_to_node(cpu);
> +}
> +
> +static void * __init pcpu_fc_alloc(unsigned int cpu, size_t size, size_t align,
> + pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn)
> {
> return memblock_alloc_try_nid(size, align, __pa(MAX_DMA_ADDRESS),
> MEMBLOCK_ALLOC_ACCESSIBLE,
> - cpu_to_node(cpu));
> + cpu_to_nd_fun(cpu));
> }
>
> static void __init pcpu_fc_free(void *ptr, size_t size)
> @@ -545,6 +550,7 @@ void __init setup_per_cpu_areas(void)
> rc = pcpu_embed_first_chunk(PERCPU_MODULE_RESERVE,
> PERCPU_DYNAMIC_RESERVE, PAGE_SIZE,
> pcpu_cpu_distance,
> + pcpu_cpu_to_node,
> pcpu_fc_alloc, pcpu_fc_free);
> if (rc < 0)
> panic("Failed to initialize percpu areas.");
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index 6052f5d5ded3..9a5609c821df 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -771,6 +771,12 @@ void __init emergency_stack_init(void)
> }
>
> #ifdef CONFIG_SMP
> +
> +static __init int pcpu_cpu_to_node(int cpu)
> +{
> + return IS_ENABLED(CONFIG_NUMA) ? early_cpu_to_node(cpu) : NUMA_NO_NODE;
> +}
> +
> /**
> * pcpu_alloc_bootmem - NUMA friendly alloc_bootmem wrapper for percpu
> * @cpu: cpu to allocate for
> @@ -784,12 +790,12 @@ void __init emergency_stack_init(void)
> * RETURNS:
> * Pointer to the allocated area on success, NULL on failure.
> */
> -static void * __init pcpu_alloc_bootmem(unsigned int cpu, size_t size,
> - size_t align)
> +static void * __init pcpu_alloc_bootmem(unsigned int cpu, size_t size, size_t align,
> + pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn)
> {
> const unsigned long goal = __pa(MAX_DMA_ADDRESS);
> #ifdef CONFIG_NUMA
> - int node = early_cpu_to_node(cpu);
> + int node = cpu_to_nd_fun(cpu);
^ typo - cpu_to_nd_fn().
> void *ptr;
>
> if (!node_online(node) || !NODE_DATA(node)) {
> @@ -891,6 +897,7 @@ void __init setup_per_cpu_areas(void)
>
> if (pcpu_chosen_fc != PCPU_FC_PAGE) {
> rc = pcpu_embed_first_chunk(0, dyn_size, atom_size, pcpu_cpu_distance,
> + pcpu_cpu_to_node,
> pcpu_alloc_bootmem, pcpu_free_bootmem);
> if (rc)
> pr_warn("PERCPU: %s allocator failed (%d), "
> @@ -900,6 +907,7 @@ void __init setup_per_cpu_areas(void)
>
> if (rc < 0)
> rc = pcpu_page_first_chunk(0, pcpu_alloc_bootmem, pcpu_free_bootmem,
> + pcpu_cpu_to_node,
> pcpu_populate_pte);
> if (rc < 0)
> panic("cannot initialize percpu area (err=%d)", rc);
> diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c
> index b98a7bbe6728..026aa3ccbc30 100644
> --- a/arch/sparc/kernel/smp_64.c
> +++ b/arch/sparc/kernel/smp_64.c
> @@ -1539,12 +1539,12 @@ void smp_send_stop(void)
> * RETURNS:
> * Pointer to the allocated area on success, NULL on failure.
> */
> -static void * __init pcpu_alloc_bootmem(unsigned int cpu, size_t size,
> - size_t align)
> +static void * __init pcpu_alloc_bootmem(unsigned int cpu, size_t size, size_t align,
> + pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn)
> {
> const unsigned long goal = __pa(MAX_DMA_ADDRESS);
> #ifdef CONFIG_NUMA
> - int node = cpu_to_node(cpu);
> + int node = cpu_to_nd_fn(cpu);
> void *ptr;
>
> if (!node_online(node) || !NODE_DATA(node)) {
> @@ -1641,6 +1641,7 @@ void __init setup_per_cpu_areas(void)
> rc = pcpu_embed_first_chunk(PERCPU_MODULE_RESERVE,
> PERCPU_DYNAMIC_RESERVE, 4 << 20,
> pcpu_cpu_distance,
> + cpu_to_node,
> pcpu_alloc_bootmem,
> pcpu_free_bootmem);
> if (rc)
> @@ -1652,6 +1653,7 @@ void __init setup_per_cpu_areas(void)
> rc = pcpu_page_first_chunk(PERCPU_MODULE_RESERVE,
> pcpu_alloc_bootmem,
> pcpu_free_bootmem,
> + cpu_to_node,
> pcpu_populate_pte);
> if (rc < 0)
> panic("cannot initialize percpu area (err=%d)", rc);
> diff --git a/arch/x86/kernel/setup_percpu.c b/arch/x86/kernel/setup_percpu.c
> index 7b65275544b2..bba4fa174a16 100644
> --- a/arch/x86/kernel/setup_percpu.c
> +++ b/arch/x86/kernel/setup_percpu.c
> @@ -97,12 +97,12 @@ static bool __init pcpu_need_numa(void)
> * RETURNS:
> * Pointer to the allocated area on success, NULL on failure.
> */
> -static void * __init pcpu_alloc_bootmem(unsigned int cpu, unsigned long size,
> - unsigned long align)
> +static void * __init pcpu_alloc_bootmem(unsigned int cpu, unsigned long size, unsigned long align,
> + pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn)
> {
> const unsigned long goal = __pa(MAX_DMA_ADDRESS);
> #ifdef CONFIG_NUMA
> - int node = early_cpu_to_node(cpu);
> + int node = cpu_to_nd_fn(cpu);
> void *ptr;
>
> if (!node_online(node) || !NODE_DATA(node)) {
> @@ -128,9 +128,10 @@ static void * __init pcpu_alloc_bootmem(unsigned int cpu, unsigned long size,
> /*
> * Helpers for first chunk memory allocation
> */
> -static void * __init pcpu_fc_alloc(unsigned int cpu, size_t size, size_t align)
> +static void * __init pcpu_fc_alloc(unsigned int cpu, size_t size, size_t align,
> + pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn)
> {
> - return pcpu_alloc_bootmem(cpu, size, align);
> + return pcpu_alloc_bootmem(cpu, size, align, cpu_to_nd_fn);
> }
>
> static void __init pcpu_fc_free(void *ptr, size_t size)
> @@ -150,6 +151,11 @@ static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
> #endif
> }
>
> +static int __init pcpu_cpu_to_node(int cpu)
> +{
> + return IS_ENABLED(CONFIG_NUMA) ? early_cpu_to_node(cpu) : NUMA_NO_NODE;
> +}
> +
> static void __init pcpup_populate_pte(unsigned long addr)
> {
> populate_extra_pte(addr);
> @@ -205,6 +211,7 @@ void __init setup_per_cpu_areas(void)
> rc = pcpu_embed_first_chunk(PERCPU_FIRST_CHUNK_RESERVE,
> dyn_size, atom_size,
> pcpu_cpu_distance,
> + pcpu_cpu_to_node,
> pcpu_fc_alloc, pcpu_fc_free);
> if (rc < 0)
> pr_warn("%s allocator failed (%d), falling back to page size\n",
> @@ -213,6 +220,7 @@ void __init setup_per_cpu_areas(void)
> if (rc < 0)
> rc = pcpu_page_first_chunk(PERCPU_FIRST_CHUNK_RESERVE,
> pcpu_fc_alloc, pcpu_fc_free,
> + pcpu_cpu_to_node,
> pcpup_populate_pte);
> if (rc < 0)
> panic("cannot initialize percpu area (err=%d)", rc);
> diff --git a/drivers/base/arch_numa.c b/drivers/base/arch_numa.c
> index bc1876915457..273543d9ff85 100644
> --- a/drivers/base/arch_numa.c
> +++ b/drivers/base/arch_numa.c
> @@ -155,10 +155,10 @@ static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
> return node_distance(early_cpu_to_node(from), early_cpu_to_node(to));
> }
>
> -static void * __init pcpu_fc_alloc(unsigned int cpu, size_t size,
> - size_t align)
> +static void * __init pcpu_fc_alloc(unsigned int cpu, size_t size, size_t align,
> + pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn)
> {
> - int nid = early_cpu_to_node(cpu);
> + int nid = cpu_to_nd_fn(cpu);
>
> return memblock_alloc_try_nid(size, align,
> __pa(MAX_DMA_ADDRESS), MEMBLOCK_ALLOC_ACCESSIBLE, nid);
> @@ -229,6 +229,7 @@ void __init setup_per_cpu_areas(void)
> rc = pcpu_embed_first_chunk(PERCPU_MODULE_RESERVE,
> PERCPU_DYNAMIC_RESERVE, PAGE_SIZE,
> pcpu_cpu_distance,
> + early_cpu_to_node,
> pcpu_fc_alloc, pcpu_fc_free);
> #ifdef CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK
> if (rc < 0)
> @@ -242,6 +243,7 @@ void __init setup_per_cpu_areas(void)
> rc = pcpu_page_first_chunk(PERCPU_MODULE_RESERVE,
> pcpu_fc_alloc,
> pcpu_fc_free,
> + early_cpu_to_node,
> pcpu_populate_pte);
> #endif
> if (rc < 0)
> diff --git a/include/linux/percpu.h b/include/linux/percpu.h
> index ae4004e7957e..41bb54715b0c 100644
> --- a/include/linux/percpu.h
> +++ b/include/linux/percpu.h
> @@ -94,8 +94,9 @@ extern const char * const pcpu_fc_names[PCPU_FC_NR];
>
> extern enum pcpu_fc pcpu_chosen_fc;
>
> -typedef void * (*pcpu_fc_alloc_fn_t)(unsigned int cpu, size_t size,
> - size_t align);
> +typedef int (pcpu_fc_cpu_to_node_fn_t)(int cpu);
> +typedef void * (*pcpu_fc_alloc_fn_t)(unsigned int cpu, size_t size, size_t align,
> + pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn);
> typedef void (*pcpu_fc_free_fn_t)(void *ptr, size_t size);
> typedef void (*pcpu_fc_populate_pte_fn_t)(unsigned long addr);
> typedef int (pcpu_fc_cpu_distance_fn_t)(unsigned int from, unsigned int to);
> @@ -111,6 +112,7 @@ extern void __init pcpu_setup_first_chunk(const struct pcpu_alloc_info *ai,
> extern int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size,
> size_t atom_size,
> pcpu_fc_cpu_distance_fn_t cpu_distance_fn,
> + pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn,
> pcpu_fc_alloc_fn_t alloc_fn,
> pcpu_fc_free_fn_t free_fn);
> #endif
> @@ -119,6 +121,7 @@ extern int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size,
> extern int __init pcpu_page_first_chunk(size_t reserved_size,
> pcpu_fc_alloc_fn_t alloc_fn,
> pcpu_fc_free_fn_t free_fn,
> + pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn,
> pcpu_fc_populate_pte_fn_t populate_pte_fn);
> #endif
Be consistent here. In pcpu_setup_first_chunk() you add the
cpu_to_node() before alloc()/free() and then in pcpu_embed_first_chunk()
you add it after. I'd prefer to add it before as to keep the
cpu_distance()/cpu_to_node() grouping.
>
> diff --git a/mm/percpu.c b/mm/percpu.c
> index f5b2c2ea5a54..3f6cf1ff0be2 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -3001,6 +3001,7 @@ static struct pcpu_alloc_info * __init __flatten pcpu_build_alloc_info(
> * @dyn_size: minimum free size for dynamic allocation in bytes
> * @atom_size: allocation atom size
> * @cpu_distance_fn: callback to determine distance between cpus, optional
> + * @cpu_to_nd_fn: callback to convert cpu to it's node, optional
> * @alloc_fn: function to allocate percpu page
> * @free_fn: function to free percpu page
> *
> @@ -3030,6 +3031,7 @@ static struct pcpu_alloc_info * __init __flatten pcpu_build_alloc_info(
> int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size,
> size_t atom_size,
> pcpu_fc_cpu_distance_fn_t cpu_distance_fn,
> + pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn,
> pcpu_fc_alloc_fn_t alloc_fn,
> pcpu_fc_free_fn_t free_fn)
> {
> @@ -3066,7 +3068,7 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size,
> BUG_ON(cpu == NR_CPUS);
>
> /* allocate space for the whole group */
> - ptr = alloc_fn(cpu, gi->nr_units * ai->unit_size, atom_size);
> + ptr = alloc_fn(cpu, gi->nr_units * ai->unit_size, atom_size, cpu_to_nd_fn);
> if (!ptr) {
> rc = -ENOMEM;
> goto out_free_areas;
> @@ -3145,6 +3147,7 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size,
> * @reserved_size: the size of reserved percpu area in bytes
> * @alloc_fn: function to allocate percpu page, always called with PAGE_SIZE
> * @free_fn: function to free percpu page, always called with PAGE_SIZE
> + * @cpu_to_nd_fn: callback to convert cpu to it's node, optional
> * @populate_pte_fn: function to populate pte
> *
> * This is a helper to ease setting up page-remapped first percpu
> @@ -3159,6 +3162,7 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size,
> int __init pcpu_page_first_chunk(size_t reserved_size,
> pcpu_fc_alloc_fn_t alloc_fn,
> pcpu_fc_free_fn_t free_fn,
> + pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn,
> pcpu_fc_populate_pte_fn_t populate_pte_fn)
> {
> static struct vm_struct vm;
> @@ -3201,7 +3205,7 @@ int __init pcpu_page_first_chunk(size_t reserved_size,
> for (i = 0; i < unit_pages; i++) {
> void *ptr;
>
> - ptr = alloc_fn(cpu, PAGE_SIZE, PAGE_SIZE);
> + ptr = alloc_fn(cpu, PAGE_SIZE, PAGE_SIZE, cpu_to_nd_fn);
> if (!ptr) {
> pr_warn("failed to allocate %s page for cpu%u\n",
> psize_str, cpu);
> @@ -3278,8 +3282,8 @@ int __init pcpu_page_first_chunk(size_t reserved_size,
> unsigned long __per_cpu_offset[NR_CPUS] __read_mostly;
> EXPORT_SYMBOL(__per_cpu_offset);
>
> -static void * __init pcpu_dfl_fc_alloc(unsigned int cpu, size_t size,
> - size_t align)
> +static void * __init pcpu_dfl_fc_alloc(unsigned int cpu, size_t size, size_t align,
> + pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn)
> {
> return memblock_alloc_from(size, align, __pa(MAX_DMA_ADDRESS));
> }
> @@ -3300,7 +3304,7 @@ void __init setup_per_cpu_areas(void)
> * what the legacy allocator did.
> */
> rc = pcpu_embed_first_chunk(PERCPU_MODULE_RESERVE,
> - PERCPU_DYNAMIC_RESERVE, PAGE_SIZE, NULL,
> + PERCPU_DYNAMIC_RESERVE, PAGE_SIZE, NULL, NULL,
> pcpu_dfl_fc_alloc, pcpu_dfl_fc_free);
> if (rc < 0)
> panic("Failed to initialize percpu areas.");
> --
> 2.26.2
>
Thanks,
Dennis
^ permalink raw reply
* Re: [PATCH RFC 3/4] mm: percpu: Add generic pcpu_fc_alloc/free funciton
From: Dennis Zhou @ 2021-11-29 22:45 UTC (permalink / raw)
To: Kefeng Wang
Cc: linux-ia64, dave.hansen, linux-mips, linux-mm, paulus, hpa,
sparclinux, cl, will, linux-riscv, x86, mingo, catalin.marinas,
aou, bp, paul.walmsley, tglx, linux-arm-kernel, tsbogend, gregkh,
linux-kernel, palmer, tj, akpm, linuxppc-dev, davem
In-Reply-To: <20211121093557.139034-4-wangkefeng.wang@huawei.com>
On Sun, Nov 21, 2021 at 05:35:56PM +0800, Kefeng Wang wrote:
> With previous patch, we could add a generic pcpu first chunk
> allocation and free function to cleanup the duplicated definations
> on each architecture.
>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
> arch/mips/mm/init.c | 16 +--------
> arch/powerpc/kernel/setup_64.c | 51 ++------------------------
> arch/sparc/kernel/smp_64.c | 50 +-------------------------
> arch/x86/kernel/setup_percpu.c | 59 +-----------------------------
> drivers/base/arch_numa.c | 19 +---------
> include/linux/percpu.h | 9 +----
> mm/percpu.c | 66 ++++++++++++++++++----------------
> 7 files changed, 42 insertions(+), 228 deletions(-)
>
> diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
> index ebbf6923532c..5a8002839550 100644
> --- a/arch/mips/mm/init.c
> +++ b/arch/mips/mm/init.c
> @@ -524,19 +524,6 @@ static int __init pcpu_cpu_to_node(int cpu)
> return cpu_to_node(cpu);
> }
>
> -static void * __init pcpu_fc_alloc(unsigned int cpu, size_t size, size_t align,
> - pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn)
> -{
> - return memblock_alloc_try_nid(size, align, __pa(MAX_DMA_ADDRESS),
> - MEMBLOCK_ALLOC_ACCESSIBLE,
> - cpu_to_nd_fun(cpu));
> -}
> -
> -static void __init pcpu_fc_free(void *ptr, size_t size)
> -{
> - memblock_free(ptr, size);
> -}
> -
> void __init setup_per_cpu_areas(void)
> {
> unsigned long delta;
> @@ -550,8 +537,7 @@ void __init setup_per_cpu_areas(void)
> rc = pcpu_embed_first_chunk(PERCPU_MODULE_RESERVE,
> PERCPU_DYNAMIC_RESERVE, PAGE_SIZE,
> pcpu_cpu_distance,
> - pcpu_cpu_to_node,
> - pcpu_fc_alloc, pcpu_fc_free);
> + pcpu_cpu_to_node);
> if (rc < 0)
> panic("Failed to initialize percpu areas.");
>
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index 9a5609c821df..364b1567f822 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -777,50 +777,6 @@ static __init int pcpu_cpu_to_node(int cpu)
> return IS_ENABLED(CONFIG_NUMA) ? early_cpu_to_node(cpu) : NUMA_NO_NODE;
> }
>
> -/**
> - * pcpu_alloc_bootmem - NUMA friendly alloc_bootmem wrapper for percpu
> - * @cpu: cpu to allocate for
> - * @size: size allocation in bytes
> - * @align: alignment
> - *
> - * Allocate @size bytes aligned at @align for cpu @cpu. This wrapper
> - * does the right thing for NUMA regardless of the current
> - * configuration.
> - *
> - * RETURNS:
> - * Pointer to the allocated area on success, NULL on failure.
> - */
> -static void * __init pcpu_alloc_bootmem(unsigned int cpu, size_t size, size_t align,
> - pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn)
> -{
> - const unsigned long goal = __pa(MAX_DMA_ADDRESS);
> -#ifdef CONFIG_NUMA
> - int node = cpu_to_nd_fun(cpu);
> - void *ptr;
> -
> - if (!node_online(node) || !NODE_DATA(node)) {
> - ptr = memblock_alloc_from(size, align, goal);
> - pr_info("cpu %d has no node %d or node-local memory\n",
> - cpu, node);
> - pr_debug("per cpu data for cpu%d %lu bytes at %016lx\n",
> - cpu, size, __pa(ptr));
> - } else {
> - ptr = memblock_alloc_try_nid(size, align, goal,
> - MEMBLOCK_ALLOC_ACCESSIBLE, node);
> - pr_debug("per cpu data for cpu%d %lu bytes on node%d at "
> - "%016lx\n", cpu, size, node, __pa(ptr));
> - }
> - return ptr;
> -#else
> - return memblock_alloc_from(size, align, goal);
> -#endif
> -}
> -
> -static void __init pcpu_free_bootmem(void *ptr, size_t size)
> -{
> - memblock_free(ptr, size);
> -}
> -
> static int pcpu_cpu_distance(unsigned int from, unsigned int to)
> {
> if (early_cpu_to_node(from) == early_cpu_to_node(to))
> @@ -897,8 +853,7 @@ void __init setup_per_cpu_areas(void)
>
> if (pcpu_chosen_fc != PCPU_FC_PAGE) {
> rc = pcpu_embed_first_chunk(0, dyn_size, atom_size, pcpu_cpu_distance,
> - pcpu_cpu_to_node,
> - pcpu_alloc_bootmem, pcpu_free_bootmem);
> + pcpu_cpu_to_node);
> if (rc)
> pr_warn("PERCPU: %s allocator failed (%d), "
> "falling back to page size\n",
> @@ -906,9 +861,7 @@ void __init setup_per_cpu_areas(void)
> }
>
> if (rc < 0)
> - rc = pcpu_page_first_chunk(0, pcpu_alloc_bootmem, pcpu_free_bootmem,
> - pcpu_cpu_to_node,
> - pcpu_populate_pte);
> + rc = pcpu_page_first_chunk(0, pcpu_cpu_to_node, pcpu_populate_pte);
> if (rc < 0)
> panic("cannot initialize percpu area (err=%d)", rc);
>
> diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c
> index 026aa3ccbc30..198dadddb75d 100644
> --- a/arch/sparc/kernel/smp_64.c
> +++ b/arch/sparc/kernel/smp_64.c
> @@ -1526,50 +1526,6 @@ void smp_send_stop(void)
> smp_call_function(stop_this_cpu, NULL, 0);
> }
>
> -/**
> - * pcpu_alloc_bootmem - NUMA friendly alloc_bootmem wrapper for percpu
> - * @cpu: cpu to allocate for
> - * @size: size allocation in bytes
> - * @align: alignment
> - *
> - * Allocate @size bytes aligned at @align for cpu @cpu. This wrapper
> - * does the right thing for NUMA regardless of the current
> - * configuration.
> - *
> - * RETURNS:
> - * Pointer to the allocated area on success, NULL on failure.
> - */
> -static void * __init pcpu_alloc_bootmem(unsigned int cpu, size_t size, size_t align,
> - pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn)
> -{
> - const unsigned long goal = __pa(MAX_DMA_ADDRESS);
> -#ifdef CONFIG_NUMA
> - int node = cpu_to_nd_fn(cpu);
> - void *ptr;
> -
> - if (!node_online(node) || !NODE_DATA(node)) {
> - ptr = memblock_alloc_from(size, align, goal);
> - pr_info("cpu %d has no node %d or node-local memory\n",
> - cpu, node);
> - pr_debug("per cpu data for cpu%d %lu bytes at %016lx\n",
> - cpu, size, __pa(ptr));
> - } else {
> - ptr = memblock_alloc_try_nid(size, align, goal,
> - MEMBLOCK_ALLOC_ACCESSIBLE, node);
> - pr_debug("per cpu data for cpu%d %lu bytes on node%d at "
> - "%016lx\n", cpu, size, node, __pa(ptr));
> - }
> - return ptr;
> -#else
> - return memblock_alloc_from(size, align, goal);
> -#endif
> -}
> -
> -static void __init pcpu_free_bootmem(void *ptr, size_t size)
> -{
> - memblock_free(ptr, size);
> -}
> -
> static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
> {
> if (cpu_to_node(from) == cpu_to_node(to))
> @@ -1641,9 +1597,7 @@ void __init setup_per_cpu_areas(void)
> rc = pcpu_embed_first_chunk(PERCPU_MODULE_RESERVE,
> PERCPU_DYNAMIC_RESERVE, 4 << 20,
> pcpu_cpu_distance,
> - cpu_to_node,
> - pcpu_alloc_bootmem,
> - pcpu_free_bootmem);
> + cpu_to_node);
> if (rc)
> pr_warn("PERCPU: %s allocator failed (%d), "
> "falling back to page size\n",
> @@ -1651,8 +1605,6 @@ void __init setup_per_cpu_areas(void)
> }
> if (rc < 0)
> rc = pcpu_page_first_chunk(PERCPU_MODULE_RESERVE,
> - pcpu_alloc_bootmem,
> - pcpu_free_bootmem,
> cpu_to_node,
> pcpu_populate_pte);
> if (rc < 0)
> diff --git a/arch/x86/kernel/setup_percpu.c b/arch/x86/kernel/setup_percpu.c
> index bba4fa174a16..cd672bd46241 100644
> --- a/arch/x86/kernel/setup_percpu.c
> +++ b/arch/x86/kernel/setup_percpu.c
> @@ -84,61 +84,6 @@ static bool __init pcpu_need_numa(void)
> }
> #endif
>
> -/**
> - * pcpu_alloc_bootmem - NUMA friendly alloc_bootmem wrapper for percpu
> - * @cpu: cpu to allocate for
> - * @size: size allocation in bytes
> - * @align: alignment
> - *
> - * Allocate @size bytes aligned at @align for cpu @cpu. This wrapper
> - * does the right thing for NUMA regardless of the current
> - * configuration.
> - *
> - * RETURNS:
> - * Pointer to the allocated area on success, NULL on failure.
> - */
> -static void * __init pcpu_alloc_bootmem(unsigned int cpu, unsigned long size, unsigned long align,
> - pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn)
> -{
> - const unsigned long goal = __pa(MAX_DMA_ADDRESS);
> -#ifdef CONFIG_NUMA
> - int node = cpu_to_nd_fn(cpu);
> - void *ptr;
> -
> - if (!node_online(node) || !NODE_DATA(node)) {
> - ptr = memblock_alloc_from(size, align, goal);
> - pr_info("cpu %d has no node %d or node-local memory\n",
> - cpu, node);
> - pr_debug("per cpu data for cpu%d %lu bytes at %016lx\n",
> - cpu, size, __pa(ptr));
> - } else {
> - ptr = memblock_alloc_try_nid(size, align, goal,
> - MEMBLOCK_ALLOC_ACCESSIBLE,
> - node);
> -
> - pr_debug("per cpu data for cpu%d %lu bytes on node%d at %016lx\n",
> - cpu, size, node, __pa(ptr));
> - }
> - return ptr;
> -#else
> - return memblock_alloc_from(size, align, goal);
> -#endif
> -}
> -
> -/*
> - * Helpers for first chunk memory allocation
> - */
> -static void * __init pcpu_fc_alloc(unsigned int cpu, size_t size, size_t align,
> - pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn)
> -{
> - return pcpu_alloc_bootmem(cpu, size, align, cpu_to_nd_fn);
> -}
> -
> -static void __init pcpu_fc_free(void *ptr, size_t size)
> -{
> - memblock_free(ptr, size);
> -}
> -
> static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
> {
> #ifdef CONFIG_NUMA
> @@ -211,15 +156,13 @@ void __init setup_per_cpu_areas(void)
> rc = pcpu_embed_first_chunk(PERCPU_FIRST_CHUNK_RESERVE,
> dyn_size, atom_size,
> pcpu_cpu_distance,
> - pcpu_cpu_to_node,
> - pcpu_fc_alloc, pcpu_fc_free);
> + pcpu_cpu_to_node);
> if (rc < 0)
> pr_warn("%s allocator failed (%d), falling back to page size\n",
> pcpu_fc_names[pcpu_chosen_fc], rc);
> }
> if (rc < 0)
> rc = pcpu_page_first_chunk(PERCPU_FIRST_CHUNK_RESERVE,
> - pcpu_fc_alloc, pcpu_fc_free,
> pcpu_cpu_to_node,
> pcpup_populate_pte);
> if (rc < 0)
> diff --git a/drivers/base/arch_numa.c b/drivers/base/arch_numa.c
> index 273543d9ff85..23a10cc36165 100644
> --- a/drivers/base/arch_numa.c
> +++ b/drivers/base/arch_numa.c
> @@ -155,20 +155,6 @@ static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
> return node_distance(early_cpu_to_node(from), early_cpu_to_node(to));
> }
>
> -static void * __init pcpu_fc_alloc(unsigned int cpu, size_t size, size_t align,
> - pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn)
> -{
> - int nid = cpu_to_nd_fn(cpu);
> -
> - return memblock_alloc_try_nid(size, align,
> - __pa(MAX_DMA_ADDRESS), MEMBLOCK_ALLOC_ACCESSIBLE, nid);
> -}
> -
> -static void __init pcpu_fc_free(void *ptr, size_t size)
> -{
> - memblock_free(ptr, size);
> -}
> -
> #ifdef CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK
> static void __init pcpu_populate_pte(unsigned long addr)
> {
> @@ -229,8 +215,7 @@ void __init setup_per_cpu_areas(void)
> rc = pcpu_embed_first_chunk(PERCPU_MODULE_RESERVE,
> PERCPU_DYNAMIC_RESERVE, PAGE_SIZE,
> pcpu_cpu_distance,
> - early_cpu_to_node,
> - pcpu_fc_alloc, pcpu_fc_free);
> + early_cpu_to_node);
> #ifdef CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK
> if (rc < 0)
> pr_warn("PERCPU: %s allocator failed (%d), falling back to page size\n",
> @@ -241,8 +226,6 @@ void __init setup_per_cpu_areas(void)
> #ifdef CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK
> if (rc < 0)
> rc = pcpu_page_first_chunk(PERCPU_MODULE_RESERVE,
> - pcpu_fc_alloc,
> - pcpu_fc_free,
> early_cpu_to_node,
> pcpu_populate_pte);
> #endif
> diff --git a/include/linux/percpu.h b/include/linux/percpu.h
> index 41bb54715b0c..d73c97ef4ff4 100644
> --- a/include/linux/percpu.h
> +++ b/include/linux/percpu.h
> @@ -95,9 +95,6 @@ extern const char * const pcpu_fc_names[PCPU_FC_NR];
> extern enum pcpu_fc pcpu_chosen_fc;
>
> typedef int (pcpu_fc_cpu_to_node_fn_t)(int cpu);
> -typedef void * (*pcpu_fc_alloc_fn_t)(unsigned int cpu, size_t size, size_t align,
> - pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn);
> -typedef void (*pcpu_fc_free_fn_t)(void *ptr, size_t size);
> typedef void (*pcpu_fc_populate_pte_fn_t)(unsigned long addr);
> typedef int (pcpu_fc_cpu_distance_fn_t)(unsigned int from, unsigned int to);
>
> @@ -112,15 +109,11 @@ extern void __init pcpu_setup_first_chunk(const struct pcpu_alloc_info *ai,
> extern int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size,
> size_t atom_size,
> pcpu_fc_cpu_distance_fn_t cpu_distance_fn,
> - pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn,
> - pcpu_fc_alloc_fn_t alloc_fn,
> - pcpu_fc_free_fn_t free_fn);
> + pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn);
> #endif
>
> #ifdef CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK
> extern int __init pcpu_page_first_chunk(size_t reserved_size,
> - pcpu_fc_alloc_fn_t alloc_fn,
> - pcpu_fc_free_fn_t free_fn,
> pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn,
> pcpu_fc_populate_pte_fn_t populate_pte_fn);
> #endif
> diff --git a/mm/percpu.c b/mm/percpu.c
> index 3f6cf1ff0be2..efaa1cbaf73d 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -2992,6 +2992,30 @@ static struct pcpu_alloc_info * __init __flatten pcpu_build_alloc_info(
>
> return ai;
> }
> +
> +static void * __init pcpu_fc_alloc(unsigned int cpu, size_t size, size_t align,
> + pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn)
> +{
> + const unsigned long goal = __pa(MAX_DMA_ADDRESS);
> + int node = NUMA_NO_NODE;
> + void *ptr;
> +
> + if (cpu_to_nd_fn)
> + node = cpu_to_nd_fn(cpu);
> +
> + if (node == NUMA_NO_NODE || !node_online(node) || !NODE_DATA(node)) {
> + ptr = memblock_alloc_from(size, align, goal);
> + } else {
> + ptr = memblock_alloc_try_nid(size, align, goal,
> + MEMBLOCK_ALLOC_ACCESSIBLE, node);
> + }
> + return ptr;
> +}
My preference here would be to keep this identical to the x86
implementation where we #ifdef CONFIG_NUMA.
> +
> +static void __init pcpu_fc_free(void *ptr, size_t size)
> +{
> + memblock_free(ptr, size);
> +}
> #endif /* BUILD_EMBED_FIRST_CHUNK || BUILD_PAGE_FIRST_CHUNK */
>
> #if defined(BUILD_EMBED_FIRST_CHUNK)
> @@ -3002,14 +3026,12 @@ static struct pcpu_alloc_info * __init __flatten pcpu_build_alloc_info(
> * @atom_size: allocation atom size
> * @cpu_distance_fn: callback to determine distance between cpus, optional
> * @cpu_to_nd_fn: callback to convert cpu to it's node, optional
> - * @alloc_fn: function to allocate percpu page
> - * @free_fn: function to free percpu page
> *
> * This is a helper to ease setting up embedded first percpu chunk and
> * can be called where pcpu_setup_first_chunk() is expected.
> *
> * If this function is used to setup the first chunk, it is allocated
> - * by calling @alloc_fn and used as-is without being mapped into
> + * by calling pcpu_fc_alloc and used as-is without being mapped into
> * vmalloc area. Allocations are always whole multiples of @atom_size
> * aligned to @atom_size.
> *
> @@ -3023,7 +3045,7 @@ static struct pcpu_alloc_info * __init __flatten pcpu_build_alloc_info(
> * @dyn_size specifies the minimum dynamic area size.
> *
> * If the needed size is smaller than the minimum or specified unit
> - * size, the leftover is returned using @free_fn.
> + * size, the leftover is returned using pcpu_fc_free.
> *
> * RETURNS:
> * 0 on success, -errno on failure.
> @@ -3031,9 +3053,7 @@ static struct pcpu_alloc_info * __init __flatten pcpu_build_alloc_info(
> int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size,
> size_t atom_size,
> pcpu_fc_cpu_distance_fn_t cpu_distance_fn,
> - pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn,
> - pcpu_fc_alloc_fn_t alloc_fn,
> - pcpu_fc_free_fn_t free_fn)
> + pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn)
> {
> void *base = (void *)ULONG_MAX;
> void **areas = NULL;
> @@ -3068,7 +3088,7 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size,
> BUG_ON(cpu == NR_CPUS);
>
> /* allocate space for the whole group */
> - ptr = alloc_fn(cpu, gi->nr_units * ai->unit_size, atom_size, cpu_to_nd_fn);
> + ptr = pcpu_fc_alloc(cpu, gi->nr_units * ai->unit_size, atom_size, cpu_to_nd_fn);
> if (!ptr) {
> rc = -ENOMEM;
> goto out_free_areas;
> @@ -3107,12 +3127,12 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size,
> for (i = 0; i < gi->nr_units; i++, ptr += ai->unit_size) {
> if (gi->cpu_map[i] == NR_CPUS) {
> /* unused unit, free whole */
> - free_fn(ptr, ai->unit_size);
> + pcpu_fc_free(ptr, ai->unit_size);
> continue;
> }
> /* copy and return the unused part */
> memcpy(ptr, __per_cpu_load, ai->static_size);
> - free_fn(ptr + size_sum, ai->unit_size - size_sum);
> + pcpu_fc_free(ptr + size_sum, ai->unit_size - size_sum);
> }
> }
>
> @@ -3131,7 +3151,7 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size,
> out_free_areas:
> for (group = 0; group < ai->nr_groups; group++)
> if (areas[group])
> - free_fn(areas[group],
> + pcpu_fc_free(areas[group],
> ai->groups[group].nr_units * ai->unit_size);
> out_free:
> pcpu_free_alloc_info(ai);
> @@ -3145,8 +3165,6 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size,
> /**
> * pcpu_page_first_chunk - map the first chunk using PAGE_SIZE pages
> * @reserved_size: the size of reserved percpu area in bytes
> - * @alloc_fn: function to allocate percpu page, always called with PAGE_SIZE
> - * @free_fn: function to free percpu page, always called with PAGE_SIZE
> * @cpu_to_nd_fn: callback to convert cpu to it's node, optional
> * @populate_pte_fn: function to populate pte
> *
> @@ -3160,8 +3178,6 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size,
> * 0 on success, -errno on failure.
> */
> int __init pcpu_page_first_chunk(size_t reserved_size,
> - pcpu_fc_alloc_fn_t alloc_fn,
> - pcpu_fc_free_fn_t free_fn,
> pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn,
> pcpu_fc_populate_pte_fn_t populate_pte_fn)
> {
> @@ -3205,7 +3221,7 @@ int __init pcpu_page_first_chunk(size_t reserved_size,
> for (i = 0; i < unit_pages; i++) {
> void *ptr;
>
> - ptr = alloc_fn(cpu, PAGE_SIZE, PAGE_SIZE, cpu_to_nd_fn);
> + ptr = pcpu_fc_alloc(cpu, PAGE_SIZE, PAGE_SIZE, cpu_to_nd_fn);
> if (!ptr) {
> pr_warn("failed to allocate %s page for cpu%u\n",
> psize_str, cpu);
> @@ -3257,7 +3273,7 @@ int __init pcpu_page_first_chunk(size_t reserved_size,
>
> enomem:
> while (--j >= 0)
> - free_fn(page_address(pages[j]), PAGE_SIZE);
> + pcpu_fc_free(page_address(pages[j]), PAGE_SIZE);
> rc = -ENOMEM;
> out_free_ar:
> memblock_free(pages, pages_size);
> @@ -3282,17 +3298,6 @@ int __init pcpu_page_first_chunk(size_t reserved_size,
> unsigned long __per_cpu_offset[NR_CPUS] __read_mostly;
> EXPORT_SYMBOL(__per_cpu_offset);
>
> -static void * __init pcpu_dfl_fc_alloc(unsigned int cpu, size_t size, size_t align,
> - pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn)
> -{
> - return memblock_alloc_from(size, align, __pa(MAX_DMA_ADDRESS));
> -}
> -
> -static void __init pcpu_dfl_fc_free(void *ptr, size_t size)
> -{
> - memblock_free(ptr, size);
> -}
> -
> void __init setup_per_cpu_areas(void)
> {
> unsigned long delta;
> @@ -3303,9 +3308,8 @@ void __init setup_per_cpu_areas(void)
> * Always reserve area for module percpu variables. That's
> * what the legacy allocator did.
> */
> - rc = pcpu_embed_first_chunk(PERCPU_MODULE_RESERVE,
> - PERCPU_DYNAMIC_RESERVE, PAGE_SIZE, NULL, NULL,
> - pcpu_dfl_fc_alloc, pcpu_dfl_fc_free);
> + rc = pcpu_embed_first_chunk(PERCPU_MODULE_RESERVE, PERCPU_DYNAMIC_RESERVE,
> + PAGE_SIZE, NULL, NULL);
> if (rc < 0)
> panic("Failed to initialize percpu areas.");
>
> --
> 2.26.2
>
Overall this makes sense.
Thanks,
Dennis
^ permalink raw reply
* Re: [PATCH RFC 4/4] mm: percpu: Add generic pcpu_populate_pte() function
From: Dennis Zhou @ 2021-11-29 22:49 UTC (permalink / raw)
To: Kefeng Wang
Cc: linux-ia64, dave.hansen, linux-mips, linux-mm, paulus, hpa,
sparclinux, cl, will, linux-riscv, x86, mingo, catalin.marinas,
aou, bp, paul.walmsley, tglx, linux-arm-kernel, tsbogend, gregkh,
linux-kernel, palmer, tj, akpm, linuxppc-dev, davem
In-Reply-To: <20211121093557.139034-5-wangkefeng.wang@huawei.com>
On Sun, Nov 21, 2021 at 05:35:57PM +0800, Kefeng Wang wrote:
> When NEED_PER_CPU_PAGE_FIRST_CHUNK enabled, we need a function to
> populate pte, add a generic pcpu populate pte function and switch
> to use it.
>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
> arch/powerpc/kernel/setup_64.c | 47 +--------------------
> arch/sparc/kernel/smp_64.c | 57 +------------------------
> arch/x86/kernel/setup_percpu.c | 5 +--
> drivers/base/arch_numa.c | 51 +---------------------
> include/linux/percpu.h | 5 +--
> mm/percpu.c | 77 +++++++++++++++++++++++++++++++---
> 6 files changed, 79 insertions(+), 163 deletions(-)
>
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index 364b1567f822..1a17828af77f 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -788,51 +788,6 @@ static int pcpu_cpu_distance(unsigned int from, unsigned int to)
> unsigned long __per_cpu_offset[NR_CPUS] __read_mostly;
> EXPORT_SYMBOL(__per_cpu_offset);
>
> -static void __init pcpu_populate_pte(unsigned long addr)
> -{
> - pgd_t *pgd = pgd_offset_k(addr);
> - p4d_t *p4d;
> - pud_t *pud;
> - pmd_t *pmd;
> -
> - p4d = p4d_offset(pgd, addr);
> - if (p4d_none(*p4d)) {
> - pud_t *new;
> -
> - new = memblock_alloc(PUD_TABLE_SIZE, PUD_TABLE_SIZE);
> - if (!new)
> - goto err_alloc;
> - p4d_populate(&init_mm, p4d, new);
> - }
> -
> - pud = pud_offset(p4d, addr);
> - if (pud_none(*pud)) {
> - pmd_t *new;
> -
> - new = memblock_alloc(PMD_TABLE_SIZE, PMD_TABLE_SIZE);
> - if (!new)
> - goto err_alloc;
> - pud_populate(&init_mm, pud, new);
> - }
> -
> - pmd = pmd_offset(pud, addr);
> - if (!pmd_present(*pmd)) {
> - pte_t *new;
> -
> - new = memblock_alloc(PTE_TABLE_SIZE, PTE_TABLE_SIZE);
> - if (!new)
> - goto err_alloc;
> - pmd_populate_kernel(&init_mm, pmd, new);
> - }
> -
> - return;
> -
> -err_alloc:
> - panic("%s: Failed to allocate %lu bytes align=%lx from=%lx\n",
> - __func__, PAGE_SIZE, PAGE_SIZE, PAGE_SIZE);
> -}
> -
> -
> void __init setup_per_cpu_areas(void)
> {
> const size_t dyn_size = PERCPU_MODULE_RESERVE + PERCPU_DYNAMIC_RESERVE;
> @@ -861,7 +816,7 @@ void __init setup_per_cpu_areas(void)
> }
>
> if (rc < 0)
> - rc = pcpu_page_first_chunk(0, pcpu_cpu_to_node, pcpu_populate_pte);
> + rc = pcpu_page_first_chunk(0, pcpu_cpu_to_node);
> if (rc < 0)
> panic("cannot initialize percpu area (err=%d)", rc);
>
> diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c
> index 198dadddb75d..00dffe2d834b 100644
> --- a/arch/sparc/kernel/smp_64.c
> +++ b/arch/sparc/kernel/smp_64.c
> @@ -1534,59 +1534,6 @@ static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
> return REMOTE_DISTANCE;
> }
>
> -static void __init pcpu_populate_pte(unsigned long addr)
> -{
> - pgd_t *pgd = pgd_offset_k(addr);
> - p4d_t *p4d;
> - pud_t *pud;
> - pmd_t *pmd;
> -
> - if (pgd_none(*pgd)) {
> - pud_t *new;
> -
> - new = memblock_alloc_from(PAGE_SIZE, PAGE_SIZE, PAGE_SIZE);
> - if (!new)
> - goto err_alloc;
> - pgd_populate(&init_mm, pgd, new);
> - }
> -
> - p4d = p4d_offset(pgd, addr);
> - if (p4d_none(*p4d)) {
> - pud_t *new;
> -
> - new = memblock_alloc_from(PAGE_SIZE, PAGE_SIZE, PAGE_SIZE);
> - if (!new)
> - goto err_alloc;
> - p4d_populate(&init_mm, p4d, new);
> - }
> -
> - pud = pud_offset(p4d, addr);
> - if (pud_none(*pud)) {
> - pmd_t *new;
> -
> - new = memblock_alloc_from(PAGE_SIZE, PAGE_SIZE, PAGE_SIZE);
> - if (!new)
> - goto err_alloc;
> - pud_populate(&init_mm, pud, new);
> - }
> -
> - pmd = pmd_offset(pud, addr);
> - if (!pmd_present(*pmd)) {
> - pte_t *new;
> -
> - new = memblock_alloc_from(PAGE_SIZE, PAGE_SIZE, PAGE_SIZE);
> - if (!new)
> - goto err_alloc;
> - pmd_populate_kernel(&init_mm, pmd, new);
> - }
> -
> - return;
> -
> -err_alloc:
> - panic("%s: Failed to allocate %lu bytes align=%lx from=%lx\n",
> - __func__, PAGE_SIZE, PAGE_SIZE, PAGE_SIZE);
> -}
> -
> void __init setup_per_cpu_areas(void)
> {
> unsigned long delta;
> @@ -1604,9 +1551,7 @@ void __init setup_per_cpu_areas(void)
> pcpu_fc_names[pcpu_chosen_fc], rc);
> }
> if (rc < 0)
> - rc = pcpu_page_first_chunk(PERCPU_MODULE_RESERVE,
> - cpu_to_node,
> - pcpu_populate_pte);
> + rc = pcpu_page_first_chunk(PERCPU_MODULE_RESERVE, cpu_to_node);
> if (rc < 0)
> panic("cannot initialize percpu area (err=%d)", rc);
>
> diff --git a/arch/x86/kernel/setup_percpu.c b/arch/x86/kernel/setup_percpu.c
> index cd672bd46241..4eadbe45078e 100644
> --- a/arch/x86/kernel/setup_percpu.c
> +++ b/arch/x86/kernel/setup_percpu.c
> @@ -101,7 +101,7 @@ static int __init pcpu_cpu_to_node(int cpu)
> return IS_ENABLED(CONFIG_NUMA) ? early_cpu_to_node(cpu) : NUMA_NO_NODE;
> }
>
> -static void __init pcpup_populate_pte(unsigned long addr)
> +void __init pcpu_populate_pte(unsigned long addr)
> {
> populate_extra_pte(addr);
> }
> @@ -163,8 +163,7 @@ void __init setup_per_cpu_areas(void)
> }
> if (rc < 0)
> rc = pcpu_page_first_chunk(PERCPU_FIRST_CHUNK_RESERVE,
> - pcpu_cpu_to_node,
> - pcpup_populate_pte);
> + pcpu_cpu_to_node);
x86 has it's own implementation that differs for 32 bit. I'm not
confident this is correct to drop in as a replacement for x86, so I'd
prefer to keep populate_pte_fn() around.
> if (rc < 0)
> panic("cannot initialize percpu area (err=%d)", rc);
>
> diff --git a/drivers/base/arch_numa.c b/drivers/base/arch_numa.c
> index 23a10cc36165..eaa31e567d1e 100644
> --- a/drivers/base/arch_numa.c
> +++ b/drivers/base/arch_numa.c
> @@ -14,7 +14,6 @@
> #include <linux/of.h>
>
> #include <asm/sections.h>
> -#include <asm/pgalloc.h>
>
> struct pglist_data *node_data[MAX_NUMNODES] __read_mostly;
> EXPORT_SYMBOL(node_data);
> @@ -155,52 +154,6 @@ static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
> return node_distance(early_cpu_to_node(from), early_cpu_to_node(to));
> }
>
> -#ifdef CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK
> -static void __init pcpu_populate_pte(unsigned long addr)
> -{
> - pgd_t *pgd = pgd_offset_k(addr);
> - p4d_t *p4d;
> - pud_t *pud;
> - pmd_t *pmd;
> -
> - p4d = p4d_offset(pgd, addr);
> - if (p4d_none(*p4d)) {
> - pud_t *new;
> -
> - new = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
> - if (!new)
> - goto err_alloc;
> - p4d_populate(&init_mm, p4d, new);
> - }
> -
> - pud = pud_offset(p4d, addr);
> - if (pud_none(*pud)) {
> - pmd_t *new;
> -
> - new = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
> - if (!new)
> - goto err_alloc;
> - pud_populate(&init_mm, pud, new);
> - }
> -
> - pmd = pmd_offset(pud, addr);
> - if (!pmd_present(*pmd)) {
> - pte_t *new;
> -
> - new = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
> - if (!new)
> - goto err_alloc;
> - pmd_populate_kernel(&init_mm, pmd, new);
> - }
> -
> - return;
> -
> -err_alloc:
> - panic("%s: Failed to allocate %lu bytes align=%lx from=%lx\n",
> - __func__, PAGE_SIZE, PAGE_SIZE, PAGE_SIZE);
> -}
> -#endif
> -
> void __init setup_per_cpu_areas(void)
> {
> unsigned long delta;
> @@ -225,9 +178,7 @@ void __init setup_per_cpu_areas(void)
>
> #ifdef CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK
> if (rc < 0)
> - rc = pcpu_page_first_chunk(PERCPU_MODULE_RESERVE,
> - early_cpu_to_node,
> - pcpu_populate_pte);
> + rc = pcpu_page_first_chunk(PERCPU_MODULE_RESERVE, early_cpu_to_node);
> #endif
> if (rc < 0)
> panic("Failed to initialize percpu areas (err=%d).", rc);
> diff --git a/include/linux/percpu.h b/include/linux/percpu.h
> index d73c97ef4ff4..f1ec5ad1351c 100644
> --- a/include/linux/percpu.h
> +++ b/include/linux/percpu.h
> @@ -95,7 +95,6 @@ extern const char * const pcpu_fc_names[PCPU_FC_NR];
> extern enum pcpu_fc pcpu_chosen_fc;
>
> typedef int (pcpu_fc_cpu_to_node_fn_t)(int cpu);
> -typedef void (*pcpu_fc_populate_pte_fn_t)(unsigned long addr);
> typedef int (pcpu_fc_cpu_distance_fn_t)(unsigned int from, unsigned int to);
>
> extern struct pcpu_alloc_info * __init pcpu_alloc_alloc_info(int nr_groups,
> @@ -113,9 +112,9 @@ extern int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size,
> #endif
>
> #ifdef CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK
> +void __init pcpu_populate_pte(unsigned long addr);
> extern int __init pcpu_page_first_chunk(size_t reserved_size,
> - pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn,
> - pcpu_fc_populate_pte_fn_t populate_pte_fn);
> + pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn);
> #endif
>
> extern void __percpu *__alloc_reserved_percpu(size_t size, size_t align) __alloc_size(1);
> diff --git a/mm/percpu.c b/mm/percpu.c
> index efaa1cbaf73d..d907daed04eb 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -3162,11 +3162,80 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size,
> #endif /* BUILD_EMBED_FIRST_CHUNK */
>
> #ifdef BUILD_PAGE_FIRST_CHUNK
> +#include <asm/pgalloc.h>
> +
> +#ifndef P4D_TABLE_SIZE
> +#define P4D_TABLE_SIZE PAGE_SIZE
> +#endif
> +
> +#ifndef PUD_TABLE_SIZE
> +#define PUD_TABLE_SIZE PAGE_SIZE
> +#endif
> +
> +#ifndef PMD_TABLE_SIZE
> +#define PMD_TABLE_SIZE PAGE_SIZE
> +#endif
> +
> +#ifndef PTE_TABLE_SIZE
> +#define PTE_TABLE_SIZE PAGE_SIZE
> +#endif
> +void __init __weak pcpu_populate_pte(unsigned long addr)
> +{
> + pgd_t *pgd = pgd_offset_k(addr);
> + p4d_t *p4d;
> + pud_t *pud;
> + pmd_t *pmd;
> +
> + if (pgd_none(*pgd)) {
> + p4d_t *new;
> +
> + new = memblock_alloc_from(P4D_TABLE_SIZE, P4D_TABLE_SIZE, PAGE_SIZE);
It's unnecessary to specify a min_addr to memblock_alloc_from() as it
won't allocate 0 anyway. So please use memblock_alloc() instead.
> + if (!new)
> + goto err_alloc;
> + pgd_populate(&init_mm, pgd, new);
> + }
> +
> + p4d = p4d_offset(pgd, addr);
> + if (p4d_none(*p4d)) {
> + pud_t *new;
> +
> + new = memblock_alloc_from(PUD_TABLE_SIZE, PUD_TABLE_SIZE, PAGE_SIZE);
See above.
> + if (!new)
> + goto err_alloc;
> + p4d_populate(&init_mm, p4d, new);
> + }
> +
> + pud = pud_offset(p4d, addr);
> + if (pud_none(*pud)) {
> + pmd_t *new;
> +
> + new = memblock_alloc_from(PMD_TABLE_SIZE, PMD_TABLE_SIZE, PAGE_SIZE);
See above.
> + if (!new)
> + goto err_alloc;
> + pud_populate(&init_mm, pud, new);
> + }
> +
> + pmd = pmd_offset(pud, addr);
> + if (!pmd_present(*pmd)) {
> + pte_t *new;
> +
> + new = memblock_alloc_from(PTE_TABLE_SIZE, PTE_TABLE_SIZE, PAGE_SIZE);
See above.
> + if (!new)
> + goto err_alloc;
> + pmd_populate_kernel(&init_mm, pmd, new);
> + }
> +
> + return;
> +
> +err_alloc:
> + panic("%s: Failed to allocate %lu bytes align=%lx from=%lx\n",
> + __func__, PAGE_SIZE, PAGE_SIZE, PAGE_SIZE);
> +}
> +
> /**
> * pcpu_page_first_chunk - map the first chunk using PAGE_SIZE pages
> * @reserved_size: the size of reserved percpu area in bytes
> * @cpu_to_nd_fn: callback to convert cpu to it's node, optional
> - * @populate_pte_fn: function to populate pte
> *
> * This is a helper to ease setting up page-remapped first percpu
> * chunk and can be called where pcpu_setup_first_chunk() is expected.
> @@ -3177,9 +3246,7 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size,
> * RETURNS:
> * 0 on success, -errno on failure.
> */
> -int __init pcpu_page_first_chunk(size_t reserved_size,
> - pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn,
> - pcpu_fc_populate_pte_fn_t populate_pte_fn)
> +int __init pcpu_page_first_chunk(size_t reserved_size, pcpu_fc_cpu_to_node_fn_t cpu_to_nd_fn)
> {
> static struct vm_struct vm;
> struct pcpu_alloc_info *ai;
> @@ -3243,7 +3310,7 @@ int __init pcpu_page_first_chunk(size_t reserved_size,
> (unsigned long)vm.addr + unit * ai->unit_size;
>
> for (i = 0; i < unit_pages; i++)
> - populate_pte_fn(unit_addr + (i << PAGE_SHIFT));
> + pcpu_populate_pte(unit_addr + (i << PAGE_SHIFT));
>
> /* pte already populated, the following shouldn't fail */
> rc = __pcpu_map_pages(unit_addr, &pages[unit * unit_pages],
> --
> 2.26.2
>
^ permalink raw reply
* Re: [PATCH] powerpc/rtas: Introduce rtas_get_sensor_nonblocking() for pci hotplug driver.
From: Tyrel Datwyler @ 2021-11-29 22:54 UTC (permalink / raw)
To: Mahesh Salgaonkar, linuxppc-dev; +Cc: lxie, Oliver O'Halloran
In-Reply-To: <163817631601.2016996.16085383012429651821.stgit@jupiter>
On 11/29/21 12:58 AM, Mahesh Salgaonkar wrote:
> When certain PHB HW failure causes phyp to recover PHB, it marks the PE
> state as temporarily unavailable until recovery is complete. This also
> triggers an EEH handler in Linux which needs to notify drivers, and perform
> recovery. But before notifying the driver about the pci error it uses
> get_adapter_state()->get-sesnor-state() operation of the hotplug_slot to
> determine if the slot contains a device or not. if the slot is empty, the
> recovery is skipped entirely.
>
> However on certain PHB failures, the rtas call get-sesnor-state() returns
> extended busy error (9902) until PHB is recovered by phyp. Once PHB is
> recovered, the get-sensor-state() returns success with correct presence
> status. The rtas call interface rtas_get_sensor() loops over the rtas call
> on extended delay return code (9902) until the return value is either
> success (0) or error (-1). This causes the EEH handler to get stuck for ~6
> seconds before it could notify that the pci error has been detected and
> stop any active operations. Hence with running I/O traffic, during this 6
> seconds, the network driver continues its operation and hits a timeout
> (netdev watchdog). On timeouts, network driver go into ffdc capture mode
> and reset path assuming the PCI device is in fatal condition. This
> sometimes causes EEH recovery to fail. This impacts the ssh connection and
> leads to the system being inaccessible.
>
> ------------
> [52732.244731] DEBUG: ibm_read_slot_reset_state2()
> [52732.244762] DEBUG: ret = 0, rets[0]=5, rets[1]=1, rets[2]=4000, rets[3]=>
> [52732.244798] DEBUG: in eeh_slot_presence_check
> [52732.244804] DEBUG: error state check
> [52732.244807] DEBUG: Is slot hotpluggable
> [52732.244810] DEBUG: hotpluggable ops ?
> [52732.244953] DEBUG: Calling ops->get_adapter_status
> [52732.244958] DEBUG: calling rpaphp_get_sensor_state
> [52736.564262] ------------[ cut here ]------------
> [52736.564299] NETDEV WATCHDOG: enP64p1s0f3 (tg3): transmit queue 0 timed o>
> [52736.564324] WARNING: CPU: 1442 PID: 0 at net/sched/sch_generic.c:478 dev>
> [...]
> [52736.564505] NIP [c000000000c32368] dev_watchdog+0x438/0x440
> [52736.564513] LR [c000000000c32364] dev_watchdog+0x434/0x440
> ------------
>
> Fix this issue by introducing a new rtas_get_sensor_nonblocking() that does
> not get blocked on BUSY condition and returns immediately with error. Use
> this function in pseries pci hotplug driver which can return an error if
> slot presence state can not be detected immediately. Please note that only
> in certain PHB failures, the slot presence check returns BUSY condition. In
> normal cases it returns immediately with a correct presence state value.
> Hence this change has no impact on normal pci dlpar operations.
>
> We could use rtas_get_sensor_fast() variant, but it thorws WARN_ON on BUSY
> condition. The rtas_get_sensor_nonblocking() suppresses WARN_ON.
>
> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> ---
>
> This is an alternate approach to fix the EEH issue instead of delaying slot
> presence check proposed at
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2021-November/236956.html
>
> Also refer:
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2021-November/237027.html
> ---
> arch/powerpc/include/asm/rtas.h | 1 +
> arch/powerpc/kernel/rtas.c | 19 ++++++++++++++++---
> drivers/pci/hotplug/rpaphp_pci.c | 8 ++++----
> 3 files changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
> index 9dc97d2f9d27e..d8e8befb1c193 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -250,6 +250,7 @@ extern void rtas_os_term(char *str);
> void rtas_activate_firmware(void);
> extern int rtas_get_sensor(int sensor, int index, int *state);
> extern int rtas_get_sensor_fast(int sensor, int index, int *state);
> +int rtas_get_sensor_nonblocking(int sensor, int index, int *state);
> extern int rtas_get_power_level(int powerdomain, int *level);
> extern int rtas_set_power_level(int powerdomain, int level, int *setlevel);
> extern bool rtas_indicator_present(int token, int *maxindex);
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index ac61e226c9af6..fd5aa3bbd46c5 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -609,7 +609,8 @@ int rtas_get_sensor(int sensor, int index, int *state)
> }
> EXPORT_SYMBOL(rtas_get_sensor);
>
> -int rtas_get_sensor_fast(int sensor, int index, int *state)
> +static int
> +__rtas_get_sensor(int sensor, int index, int *state, bool warn_on)
> {
> int token = rtas_token("get-sensor-state");
> int rc;
> @@ -618,14 +619,26 @@ int rtas_get_sensor_fast(int sensor, int index, int *state)
> return -ENOENT;
>
> rc = rtas_call(token, 2, 2, state, sensor, index);
> - WARN_ON(rc == RTAS_BUSY || (rc >= RTAS_EXTENDED_DELAY_MIN &&
> - rc <= RTAS_EXTENDED_DELAY_MAX));
> + WARN_ON(warn_on &&
> + (rc == RTAS_BUSY || (rc >= RTAS_EXTENDED_DELAY_MIN &&
> + rc <= RTAS_EXTENDED_DELAY_MAX)));
The whole point of rtas_get_sensor_fast() is that on busy we will just let it
error out because we don't want to wait. I'm not sure I see the point of the
spurious WARN_ONs anytime we hit a BUSY or DELAY return code. Maybe converting
that to a pr_debug() might be better and save expanding the API with a _fast and
_nonblocking variant that do the same thing minus one surpressing a WARN_ON splat.
-Tyrel
>
> if (rc < 0)
> return rtas_error_rc(rc);
> return rc;
> }
>
> +int rtas_get_sensor_fast(int sensor, int index, int *state)
> +{
> + return __rtas_get_sensor(sensor, index, state, true);
> +}
> +
> +int rtas_get_sensor_nonblocking(int sensor, int index, int *state)
> +{
> + return __rtas_get_sensor(sensor, index, state, false);
> +}
> +EXPORT_SYMBOL(rtas_get_sensor_nonblocking);
> +
> bool rtas_indicator_present(int token, int *maxindex)
> {
> int proplen, count, i;
> diff --git a/drivers/pci/hotplug/rpaphp_pci.c b/drivers/pci/hotplug/rpaphp_pci.c
> index c380bdacd1466..8a7d681254ce9 100644
> --- a/drivers/pci/hotplug/rpaphp_pci.c
> +++ b/drivers/pci/hotplug/rpaphp_pci.c
> @@ -23,7 +23,7 @@ int rpaphp_get_sensor_state(struct slot *slot, int *state)
> int rc;
> int setlevel;
>
> - rc = rtas_get_sensor(DR_ENTITY_SENSE, slot->index, state);
> + rc = rtas_get_sensor_nonblocking(DR_ENTITY_SENSE, slot->index, state);
>
> if (rc < 0) {
> if (rc == -EFAULT || rc == -EEXIST) {
> @@ -38,10 +38,10 @@ int rpaphp_get_sensor_state(struct slot *slot, int *state)
> if (rc < 0) {
> dbg("%s: power on slot[%s] failed rc=%d.\n",
> __func__, slot->name, rc);
> - } else {
> - rc = rtas_get_sensor(DR_ENTITY_SENSE,
> - slot->index, state);
> + return rc;
> }
> + rc = rtas_get_sensor_nonblocking(DR_ENTITY_SENSE,
> + slot->index, state);
> } else if (rc == -ENODEV)
> info("%s: slot is unusable\n", __func__);
> else
>
>
^ permalink raw reply
* Re: [PATCH RFC 0/4] mm: percpu: Cleanup percpu first chunk funciton
From: Dennis Zhou @ 2021-11-29 22:55 UTC (permalink / raw)
To: Kefeng Wang
Cc: linux-ia64, dave.hansen, linux-mips, linux-mm, paulus, hpa,
sparclinux, cl, will, linux-riscv, x86, mingo, catalin.marinas,
aou, bp, paul.walmsley, tglx, linux-arm-kernel, tsbogend, gregkh,
linux-kernel, palmer, tj, akpm, linuxppc-dev, davem
In-Reply-To: <20211121093557.139034-1-wangkefeng.wang@huawei.com>
Hello,
On Sun, Nov 21, 2021 at 05:35:53PM +0800, Kefeng Wang wrote:
> When support page mapping percpu first chunk allocator on arm64, we
> found there are lots of duplicated codes in percpu embed/page first
> chunk allocator. This patchset is aimed to cleanup them and should
> no funciton change, only test on arm64.
>
> Kefeng Wang (4):
> mm: percpu: Generalize percpu related config
> mm: percpu: Add pcpu_fc_cpu_to_node_fn_t typedef
> mm: percpu: Add generic pcpu_fc_alloc/free funciton
> mm: percpu: Add generic pcpu_populate_pte() function
>
> arch/arm64/Kconfig | 20 +----
> arch/ia64/Kconfig | 9 +--
> arch/mips/Kconfig | 10 +--
> arch/mips/mm/init.c | 14 +---
> arch/powerpc/Kconfig | 17 +---
> arch/powerpc/kernel/setup_64.c | 92 +--------------------
> arch/riscv/Kconfig | 10 +--
> arch/sparc/Kconfig | 12 +--
> arch/sparc/kernel/smp_64.c | 105 +-----------------------
> arch/x86/Kconfig | 17 +---
> arch/x86/kernel/setup_percpu.c | 66 ++-------------
> drivers/base/arch_numa.c | 68 +---------------
> include/linux/percpu.h | 13 +--
> mm/Kconfig | 12 +++
> mm/percpu.c | 143 +++++++++++++++++++++++++--------
> 15 files changed, 165 insertions(+), 443 deletions(-)
>
> --
> 2.26.2
>
I've made a few comments. I think this will be a little bit of a
challenge to get through due to it touching so many architectures. For
ease, it probably makes sense to run it through mny tree, but we'll need
explicit acks as I mentioned.
I like getting rid of the pcpu_alloc_bootmem()/pcpu_free_bootmem()
functions. However, let's keep the implementation identical to x86.
I don't think we should get rid of the populate_pte_fn(). I'm not
comfortable changing x86's implementation. Simply offer a NULL, and if
NULL use the default.
Do you have a tree that intel pulls? I suggest cleaning up the patches
and pushing to a remote branch that they pick up. That would have caught
the mips typo. Send a PR creating a file in [1] for your branch, github
is fine. Basic validation needs to be done before I can pick this up
too on more than arm64.
[1] https://github.com/intel/lkp-tests/tree/master/repo/linux
Thanks,
Dennis
^ permalink raw reply
* Re: [PATCH v5 5/5] powerpc/inst: Optimise copy_inst_from_kernel_nofault()
From: kernel test robot @ 2021-11-29 22:55 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linuxppc-dev, llvm, kbuild-all, linux-kernel
In-Reply-To: <0d5b12183d5176dd702d29ad94c39c384e51c78f.1638208156.git.christophe.leroy@csgroup.eu>
Hi Christophe,
I love your patch! Perhaps something to improve:
[auto build test WARNING on powerpc/next]
[also build test WARNING on v5.16-rc3 next-20211129]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/powerpc-inst-Refactor-___get_user_instr/20211130-015346
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-randconfig-r023-20211129 (https://download.01.org/0day-ci/archive/20211130/202111300652.0yDBNvyJ-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project df08b2fe8b35cb63dfb3b49738a3494b9b4e6f8e)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install powerpc cross compiling tool for clang build
# apt-get install binutils-powerpc-linux-gnu
# https://github.com/0day-ci/linux/commit/fb7bff30cc0efc7e4df1b48bb69de1f325eee826
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Christophe-Leroy/powerpc-inst-Refactor-___get_user_instr/20211130-015346
git checkout fb7bff30cc0efc7e4df1b48bb69de1f325eee826
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc prepare
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from arch/powerpc/kernel/asm-offsets.c:71:
In file included from arch/powerpc/kernel/../xmon/xmon_bpts.h:7:
>> arch/powerpc/include/asm/inst.h:165:20: warning: variable 'val' is uninitialized when used here [-Wuninitialized]
*inst = ppc_inst(val);
^~~
arch/powerpc/include/asm/inst.h:53:22: note: expanded from macro 'ppc_inst'
#define ppc_inst(x) (x)
^
arch/powerpc/include/asm/inst.h:155:18: note: initialize the variable 'val' to silence this warning
unsigned int val, suffix;
^
= 0
1 warning generated.
<stdin>:1559:2: warning: syscall futex_waitv not implemented [-W#warnings]
#warning syscall futex_waitv not implemented
^
1 warning generated.
arch/powerpc/kernel/vdso32/gettimeofday.S:72:8: error: unsupported directive '.stabs'
.stabs "_restgpr_31_x:F-1",36,0,0,_restgpr_31_x; .globl _restgpr_31_x; _restgpr_31_x:
^
arch/powerpc/kernel/vdso32/gettimeofday.S:73:8: error: unsupported directive '.stabs'
.stabs "_rest32gpr_31_x:F-1",36,0,0,_rest32gpr_31_x; .globl _rest32gpr_31_x; _rest32gpr_31_x:
^
make[2]: *** [arch/powerpc/kernel/vdso32/Makefile:55: arch/powerpc/kernel/vdso32/gettimeofday.o] Error 1
make[2]: Target 'include/generated/vdso32-offsets.h' not remade because of errors.
make[1]: *** [arch/powerpc/Makefile:421: vdso_prepare] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [Makefile:219: __sub-make] Error 2
make: Target 'prepare' not remade because of errors.
vim +/val +165 arch/powerpc/include/asm/inst.h
152
153 static inline int copy_inst_from_kernel_nofault(ppc_inst_t *inst, u32 *src)
154 {
155 unsigned int val, suffix;
156
157 if (unlikely(!is_kernel_addr((unsigned long)src)))
158 return -ERANGE;
159
160 __get_kernel_nofault(&val, src, u32, Efault);
161 if (IS_ENABLED(CONFIG_PPC64) && get_op(val) == OP_PREFIX) {
162 __get_kernel_nofault(&suffix, src + 1, u32, Efault);
163 *inst = ppc_inst_prefix(val, suffix);
164 } else {
> 165 *inst = ppc_inst(val);
166 }
167 return 0;
168 Efault:
169 return -EFAULT;
170 }
171
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH] powerpc/pseries/vas: Don't print an error when VAS is unavailable
From: Michael Ellerman @ 2021-11-29 23:25 UTC (permalink / raw)
To: Nicholas Piggin, Cédric Le Goater, linuxppc-dev; +Cc: Haren Myneni
In-Reply-To: <1637922573.8ofrolskkj.astroid@bobo.none>
Nicholas Piggin <npiggin@gmail.com> writes:
> Excerpts from Cédric Le Goater's message of November 26, 2021 5:13 pm:
>> On 11/26/21 06:21, Nicholas Piggin wrote:
>>> KVM does not support VAS so guests always print a useless error on boot
>>>
>>> vas: HCALL(398) error -2, query_type 0, result buffer 0x57f2000
>>>
>>> Change this to only print the message if the error is not H_FUNCTION.
>>
>>
>> Just being curious, why is it even called since "ibm,compression" should
>> not be exposed in the DT ?
>
> It looks like vas does not test for it. I guess in theory there can be
> other functions than compression implemented as an accelerator. Maybe
> that's why?
Yeah I guess, or it's just not structured that well. The vas platform
code is a bit awkward, it's there to support drivers, but it's not
actually driver code.
I think we can probably rework it so the vas code does nothing until a
driver calls in to it.
eg. something like below.
cheers
diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
index b043e3936d21..dc3491fc919d 100644
--- a/arch/powerpc/platforms/pseries/vas.c
+++ b/arch/powerpc/platforms/pseries/vas.c
@@ -454,6 +454,8 @@ static const struct vas_user_win_ops vops_pseries = {
.close_win = vas_deallocate_window, /* Close window */
};
+static int pseries_vas_init(void);
+
/*
* Supporting only nx-gzip coprocessor type now, but this API code
* extended to other coprocessor types later.
@@ -463,7 +465,8 @@ int vas_register_api_pseries(struct module *mod, enum vas_cop_type cop_type,
{
int rc;
- if (!copypaste_feat)
+ rc = pseries_vas_init();
+ if (rc || !copypaste_feat)
return -ENOTSUPP;
rc = vas_register_coproc_api(mod, cop_type, name, &vops_pseries);
@@ -531,7 +534,7 @@ static int get_vas_capabilities(u8 feat, enum vas_cop_feat_type type,
return 0;
}
-static int __init pseries_vas_init(void)
+static int pseries_vas_init(void)
{
struct hv_vas_cop_feat_caps *hv_cop_caps;
struct hv_vas_all_caps *hv_caps;
@@ -592,4 +595,3 @@ static int __init pseries_vas_init(void)
kfree(hv_caps);
return rc;
}
-machine_device_initcall(pseries, pseries_vas_init);
^ permalink raw reply related
* RE: bug: usb: gadget: FSL_UDC_CORE Corrupted request list leads to unrecoverable loop.
From: Leo Li @ 2021-11-29 23:37 UTC (permalink / raw)
To: Eugene Bordenkircher, Thorsten Leemhuis, jocke@infinera.com,
linuxppc-dev@lists.ozlabs.org, linux-usb@vger.kernel.org
Cc: gregkh@linuxfoundation.org, balbi@kernel.org
In-Reply-To: <MWHPR2201MB1520A85FE05B281DAA30F44A91669@MWHPR2201MB1520.namprd22.prod.outlook.com>
> -----Original Message-----
> From: Eugene Bordenkircher <Eugene_Bordenkircher@selinc.com>
> Sent: Monday, November 29, 2021 11:25 AM
> To: Thorsten Leemhuis <regressions@leemhuis.info>; jocke@infinera.com
> <joakim.tjernlund@infinera.com>; linuxppc-dev@lists.ozlabs.org; linux-
> usb@vger.kernel.org
> Cc: Leo Li <leoyang.li@nxp.com>; gregkh@linuxfoundation.org;
> balbi@kernel.org
> Subject: RE: bug: usb: gadget: FSL_UDC_CORE Corrupted request list leads to
> unrecoverable loop.
>
> The final result of our testing is that the patch set posted seems to address all
> known defects in the Linux kernel. The mentioned additional problems are
> entirely caused by the antivirus solution on the windows box. The antivirus
> solution blocks the disconnect messages from reaching the RNDIS driver so it
> has no idea the USB device went away. There is nothing we can do to
> address this in the Linux kernel.
Thanks for the confirmation.
>
> I propose we move forward with the patchset.
I think that we should proceed to merge the patchset but it seems to need some cleanup for coding style issues and better description before submitted formally.
>
> Eugene T. Bordenkircher
>
> -----Original Message-----
> From: Thorsten Leemhuis <regressions@leemhuis.info>
> Sent: Thursday, November 25, 2021 5:59 AM
> To: Eugene Bordenkircher <Eugene_Bordenkircher@selinc.com>; Thorsten
> Leemhuis <regressions@leemhuis.info>; Joakim Tjernlund
> <Joakim.Tjernlund@infinera.com>; linuxppc-dev@lists.ozlabs.org; linux-
> usb@vger.kernel.org
> Cc: leoyang.li@nxp.com; gregkh@linuxfoundation.org; balbi@kernel.org
> Subject: Re: bug: usb: gadget: FSL_UDC_CORE Corrupted request list leads to
> unrecoverable loop.
>
> Hi, this is your Linux kernel regression tracker speaking.
>
> Top-posting for once, to make this easy to process for everyone:
>
> Li Yang and Felipe Balbi: how to move on with this? It's quite an old
> regression, but nevertheless it is one and thus should be fixed. Part of my
> position is to make that happen and thus remind developers and maintainers
> about this until the regression is resolved.
>
> Ciao, Thorsten
>
> On 16.11.21 20:11, Eugene Bordenkircher wrote:
> > On 02.11.21 22:15, Joakim Tjernlund wrote:
> >> On Sat, 2021-10-30 at 14:20 +0000, Joakim Tjernlund wrote:
> >>> On Fri, 2021-10-29 at 17:14 +0000, Eugene Bordenkircher wrote:
> >>
> >>>> We've discovered a situation where the FSL udc driver
> (drivers/usb/gadget/udc/fsl_udc_core.c) will enter a loop iterating over the
> request queue, but the queue has been corrupted at some point so it loops
> infinitely. I believe we have narrowed into the offending code, but we are in
> need of assistance trying to find an appropriate fix for the problem. The
> identified code appears to be in all versions of the Linux kernel the driver
> exists in.
> >>>>
> >>>> The problem appears to be when handling a USB_REQ_GET_STATUS
> request. The driver gets this request and then calls the ch9getstatus()
> function. In this function, it starts a request by "borrowing" the per device
> status_req, filling it in, and then queuing it with a call to list_add_tail() to add
> the request to the endpoint queue. Right before it exits the function
> however, it's calling ep0_prime_status(), which is filling out that same
> status_req structure and then queuing it with another call to list_add_tail() to
> add the request to the endpoint queue. This adds two instances of the exact
> same LIST_HEAD to the endpoint queue, which breaks the list since the prev
> and next pointers end up pointing to the wrong things. This ends up causing
> a hard loop the next time nuke() gets called, which happens on the next
> setup IRQ.
> >>>>
> >>>> I'm not sure what the appropriate fix to this problem is, mostly due to
> my lack of expertise in USB and this driver stack. The code has been this way
> in the kernel for a very long time, which suggests that it has been working,
> unless USB_REQ_GET_STATUS requests are never made. This further
> suggests that there is something else going on that I don't understand.
> Deleting the call to ep0_prime_status() and the following ep0stall() call
> appears, on the surface, to get the device working again, but may have side
> effects that I'm not seeing.
> >>>>
> >>>> I'm hopeful someone in the community can help provide some
> information on what I may be missing or help come up with a solution to the
> problem. A big thank you to anyone who would like to help out.
> >>>
> >>> Run into this to a while ago. Found the bug and a few more fixes.
> >>> This is against 4.19 so you may have to tweak them a bit.
> >>> Feel free to upstream them.
> >>
> >> Curious, did my patches help? Good to known once we upgrade as well.
> >
> > There's good news and bad news.
> >
> > The good news is that this appears to stop the driver from entering an
> > infinite loop, which prevents the Linux system from locking up and
> > never recovering. So I'm willing to say we've made the behavior
> > better.
> >
> > The bad news is that once we get past this point, there is new bad
> > behavior. What is on top of this driver in our system is the RNDIS
> > gadget driver communicating to a Laptop running Win10 -1809.
> > Everything appears to work fine with the Linux system until there is a
> > USB disconnect. After the disconnect, the Linux side appears to
> > continue on just fine, but the Windows side doesn't seem to recognize
> > the disconnect, which causes the USB driver on that side to hang
> > forever and eventually blue screen the box. This doesn't happen on
> > all machines, just a select few. I think we can isolate the
> > behavior to a specific antivirus/security software driver that is
> > inserting itself into the USB stack and filtering the disconnect
> > message, but we're still proving that.
> >
> > I'm about 90% certain this is a different problem and we can call this
> > patchset good, at least for our test setup. My only hesitation is if
> > the Linux side is sending a set of responses that are confusing the
> > Windows side (specifically this antivirus) or not. I'd be content
> > calling that a separate defect though and letting this one close up
> > with that patchset.
>
> P.S.: As a Linux kernel regression tracker I'm getting a lot of reports on my
> table. I can only look briefly into most of them. Unfortunately therefore I
> sometimes will get things wrong or miss something important.
> I hope that's not the case here; if you think it is, don't hesitate to tell me
> about it in a public reply. That's in everyone's interest, as what I wrote above
> might be misleading to everyone reading this; any suggestion I gave they
> thus might sent someone reading this down the wrong rabbit hole, which
> none of us wants.
>
> BTW, I have no personal interest in this issue, which is tracked using regzbot,
> my Linux kernel regression tracking bot
> (https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Furld
> efense.com%2Fv3%2F__https%3A%2F%2Flinux-
> regtracking.leemhuis.info%2Fregzbot%2F__%3B!!O7uE89YCNVw!aHa5_mLM
> nBeDjINlAtV19tBHm-
> He9jbusXucMA5h7oonHvNFwYpOHAaaqqewPOuGK9HAzJUz%24&data
> =04%7C01%7Cleoyang.li%40nxp.com%7C859ce1560a7344729cea08d9b35d2e
> 67%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6377380350721308
> 84%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luM
> zIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=ONQZyAKXNgok
> 6LgYvnaAL7LVY%2B5Wl7pXglZDqWUJZMc%3D&reserved=0 ). I'm only
> posting this mail to get things rolling again and hence don't need to be CC on
> all further activities wrt to this regression.
>
> #regzbot title: usb: fsl_udc_core: corrupted request list leads to
> unrecoverable loop
^ permalink raw reply
* RE: bug: usb: gadget: FSL_UDC_CORE Corrupted request list leads to unrecoverable loop.
From: Eugene Bordenkircher @ 2021-11-29 23:48 UTC (permalink / raw)
To: Leo Li, Thorsten Leemhuis, jocke@infinera.com,
linuxppc-dev@lists.ozlabs.org, linux-usb@vger.kernel.org
Cc: gregkh@linuxfoundation.org, balbi@kernel.org
In-Reply-To: <AS8PR04MB89461BF7A3272E5A18ECD0948F669@AS8PR04MB8946.eurprd04.prod.outlook.com>
Agreed,
We are happy pick up the torch on this, but I'd like to try and hear from Joakim first before we do. The patch set is his, so I'd like to give him the opportunity. I think he's the only one that can add a truly proper description as well because he mentioned that this includes a "few more fixes" than just the one we ran into. I'd rather hear from him than try to reverse engineer what was being addressed.
Joakim, if you are still watching the thread, would you like to take a stab at it? If I don't hear from you in a couple days, we'll pick up the torch and do what we can.
Eugene T. Bordenkircher
-----Original Message-----
From: Leo Li <leoyang.li@nxp.com>
Sent: Monday, November 29, 2021 3:37 PM
To: Eugene Bordenkircher <Eugene_Bordenkircher@selinc.com>; Thorsten Leemhuis <regressions@leemhuis.info>; jocke@infinera.com <joakim.tjernlund@infinera.com>; linuxppc-dev@lists.ozlabs.org; linux-usb@vger.kernel.org
Cc: gregkh@linuxfoundation.org; balbi@kernel.org
Subject: RE: bug: usb: gadget: FSL_UDC_CORE Corrupted request list leads to unrecoverable loop.
[Caution - External]
> -----Original Message-----
> From: Eugene Bordenkircher <Eugene_Bordenkircher@selinc.com>
> Sent: Monday, November 29, 2021 11:25 AM
> To: Thorsten Leemhuis <regressions@leemhuis.info>; jocke@infinera.com
> <joakim.tjernlund@infinera.com>; linuxppc-dev@lists.ozlabs.org; linux-
> usb@vger.kernel.org
> Cc: Leo Li <leoyang.li@nxp.com>; gregkh@linuxfoundation.org;
> balbi@kernel.org
> Subject: RE: bug: usb: gadget: FSL_UDC_CORE Corrupted request list
> leads to unrecoverable loop.
>
> The final result of our testing is that the patch set posted seems to
> address all known defects in the Linux kernel. The mentioned
> additional problems are entirely caused by the antivirus solution on
> the windows box. The antivirus solution blocks the disconnect
> messages from reaching the RNDIS driver so it has no idea the USB
> device went away. There is nothing we can do to address this in the Linux kernel.
Thanks for the confirmation.
>
> I propose we move forward with the patchset.
I think that we should proceed to merge the patchset but it seems to need some cleanup for coding style issues and better description before submitted formally.
>
> Eugene T. Bordenkircher
>
> -----Original Message-----
> From: Thorsten Leemhuis <regressions@leemhuis.info>
> Sent: Thursday, November 25, 2021 5:59 AM
> To: Eugene Bordenkircher <Eugene_Bordenkircher@selinc.com>; Thorsten
> Leemhuis <regressions@leemhuis.info>; Joakim Tjernlund
> <Joakim.Tjernlund@infinera.com>; linuxppc-dev@lists.ozlabs.org; linux-
> usb@vger.kernel.org
> Cc: leoyang.li@nxp.com; gregkh@linuxfoundation.org; balbi@kernel.org
> Subject: Re: bug: usb: gadget: FSL_UDC_CORE Corrupted request list
> leads to unrecoverable loop.
>
> Hi, this is your Linux kernel regression tracker speaking.
>
> Top-posting for once, to make this easy to process for everyone:
>
> Li Yang and Felipe Balbi: how to move on with this? It's quite an old
> regression, but nevertheless it is one and thus should be fixed. Part
> of my position is to make that happen and thus remind developers and
> maintainers about this until the regression is resolved.
>
> Ciao, Thorsten
>
> On 16.11.21 20:11, Eugene Bordenkircher wrote:
> > On 02.11.21 22:15, Joakim Tjernlund wrote:
> >> On Sat, 2021-10-30 at 14:20 +0000, Joakim Tjernlund wrote:
> >>> On Fri, 2021-10-29 at 17:14 +0000, Eugene Bordenkircher wrote:
> >>
> >>>> We've discovered a situation where the FSL udc driver
> (drivers/usb/gadget/udc/fsl_udc_core.c) will enter a loop iterating
> over the request queue, but the queue has been corrupted at some point
> so it loops infinitely. I believe we have narrowed into the offending
> code, but we are in need of assistance trying to find an appropriate
> fix for the problem. The identified code appears to be in all
> versions of the Linux kernel the driver exists in.
> >>>>
> >>>> The problem appears to be when handling a USB_REQ_GET_STATUS
> request. The driver gets this request and then calls the
> ch9getstatus() function. In this function, it starts a request by
> "borrowing" the per device status_req, filling it in, and then queuing
> it with a call to list_add_tail() to add the request to the endpoint
> queue. Right before it exits the function however, it's calling
> ep0_prime_status(), which is filling out that same status_req
> structure and then queuing it with another call to list_add_tail() to
> add the request to the endpoint queue. This adds two instances of the
> exact same LIST_HEAD to the endpoint queue, which breaks the list
> since the prev and next pointers end up pointing to the wrong things.
> This ends up causing a hard loop the next time nuke() gets called, which happens on the next setup IRQ.
> >>>>
> >>>> I'm not sure what the appropriate fix to this problem is, mostly
> >>>> due to
> my lack of expertise in USB and this driver stack. The code has been
> this way in the kernel for a very long time, which suggests that it
> has been working, unless USB_REQ_GET_STATUS requests are never made.
> This further suggests that there is something else going on that I don't understand.
> Deleting the call to ep0_prime_status() and the following ep0stall()
> call appears, on the surface, to get the device working again, but may
> have side effects that I'm not seeing.
> >>>>
> >>>> I'm hopeful someone in the community can help provide some
> information on what I may be missing or help come up with a solution
> to the problem. A big thank you to anyone who would like to help out.
> >>>
> >>> Run into this to a while ago. Found the bug and a few more fixes.
> >>> This is against 4.19 so you may have to tweak them a bit.
> >>> Feel free to upstream them.
> >>
> >> Curious, did my patches help? Good to known once we upgrade as well.
> >
> > There's good news and bad news.
> >
> > The good news is that this appears to stop the driver from entering
> > an infinite loop, which prevents the Linux system from locking up
> > and never recovering. So I'm willing to say we've made the behavior
> > better.
> >
> > The bad news is that once we get past this point, there is new bad
> > behavior. What is on top of this driver in our system is the RNDIS
> > gadget driver communicating to a Laptop running Win10 -1809.
> > Everything appears to work fine with the Linux system until there is
> > a USB disconnect. After the disconnect, the Linux side appears to
> > continue on just fine, but the Windows side doesn't seem to
> > recognize the disconnect, which causes the USB driver on that side
> > to hang forever and eventually blue screen the box. This doesn't happen on
> > all machines, just a select few. I think we can isolate the
> > behavior to a specific antivirus/security software driver that is
> > inserting itself into the USB stack and filtering the disconnect
> > message, but we're still proving that.
> >
> > I'm about 90% certain this is a different problem and we can call
> > this patchset good, at least for our test setup. My only hesitation
> > is if the Linux side is sending a set of responses that are
> > confusing the Windows side (specifically this antivirus) or not.
> > I'd be content calling that a separate defect though and letting
> > this one close up with that patchset.
>
> P.S.: As a Linux kernel regression tracker I'm getting a lot of
> reports on my table. I can only look briefly into most of them.
> Unfortunately therefore I sometimes will get things wrong or miss something important.
> I hope that's not the case here; if you think it is, don't hesitate to
> tell me about it in a public reply. That's in everyone's interest, as
> what I wrote above might be misleading to everyone reading this; any
> suggestion I gave they thus might sent someone reading this down the
> wrong rabbit hole, which none of us wants.
>
> BTW, I have no personal interest in this issue, which is tracked using
> regzbot, my Linux kernel regression tracking bot
> (https://urldefense.com/v3/__https://eur01.safelinks.protection.outloo
> k.com/?url=https*3A*2F*2Furld__;JSUl!!O7uE89YCNVw!a6nsIMfn544OIzmshw3H
> bMBVcbwor4cV2Q5OsST7-86jy_YZKvDsN-558Ris4wh8Zawz4puN$
> efense.com%2Fv3%2F__https%3A%2F%2Flinux-
> regtracking.leemhuis.info%2Fregzbot%2F__%3B!!O7uE89YCNVw!aHa5_mLM
> nBeDjINlAtV19tBHm-
> He9jbusXucMA5h7oonHvNFwYpOHAaaqqewPOuGK9HAzJUz%24&data
> =04%7C01%7Cleoyang.li%40nxp.com%7C859ce1560a7344729cea08d9b35d2e
> 67%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6377380350721308
> 84%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luM
> zIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=ONQZyAKXNgok
> 6LgYvnaAL7LVY%2B5Wl7pXglZDqWUJZMc%3D&reserved=0 ). I'm only
> posting this mail to get things rolling again and hence don't need to
> be CC on all further activities wrt to this regression.
>
> #regzbot title: usb: fsl_udc_core: corrupted request list leads to
> unrecoverable loop
^ permalink raw reply
* [PATCH] scsi: ibmvfc: replace snprintf with sysfs_emit
From: davidcomponentone @ 2021-11-30 0:06 UTC (permalink / raw)
To: tyreld
Cc: linux-scsi, martin.petersen, jejb, davidcomponentone,
linux-kernel, Yang Guang, paulus, linuxppc-dev, Zeal Robot
From: Yang Guang <yang.guang5@zte.com.cn>
coccinelle report:
./drivers/scsi/ibmvscsi/ibmvfc.c:3453:8-16:
WARNING: use scnprintf or sprintf
./drivers/scsi/ibmvscsi/ibmvfc.c:3416:8-16:
WARNING: use scnprintf or sprintf
./drivers/scsi/ibmvscsi/ibmvfc.c:3436:8-16:
WARNING: use scnprintf or sprintf
./drivers/scsi/ibmvscsi/ibmvfc.c:3426:8-16:
WARNING: use scnprintf or sprintf
./drivers/scsi/ibmvscsi/ibmvfc.c:3445:8-16:
WARNING: use scnprintf or sprintf
./drivers/scsi/ibmvscsi/ibmvfc.c:3406:8-16:
WARNING: use scnprintf or sprintf
Use sysfs_emit instead of scnprintf or sprintf makes more sense.
Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: Yang Guang <yang.guang5@zte.com.cn>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index d0eab5700dc5..69bf55c037a5 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -3403,7 +3403,7 @@ static ssize_t ibmvfc_show_host_partition_name(struct device *dev,
struct Scsi_Host *shost = class_to_shost(dev);
struct ibmvfc_host *vhost = shost_priv(shost);
- return snprintf(buf, PAGE_SIZE, "%s\n",
+ return sysfs_emit(buf, "%s\n",
vhost->login_buf->resp.partition_name);
}
@@ -3413,7 +3413,7 @@ static ssize_t ibmvfc_show_host_device_name(struct device *dev,
struct Scsi_Host *shost = class_to_shost(dev);
struct ibmvfc_host *vhost = shost_priv(shost);
- return snprintf(buf, PAGE_SIZE, "%s\n",
+ return sysfs_emit(buf, "%s\n",
vhost->login_buf->resp.device_name);
}
@@ -3423,7 +3423,7 @@ static ssize_t ibmvfc_show_host_loc_code(struct device *dev,
struct Scsi_Host *shost = class_to_shost(dev);
struct ibmvfc_host *vhost = shost_priv(shost);
- return snprintf(buf, PAGE_SIZE, "%s\n",
+ return sysfs_emit(buf, "%s\n",
vhost->login_buf->resp.port_loc_code);
}
@@ -3433,7 +3433,7 @@ static ssize_t ibmvfc_show_host_drc_name(struct device *dev,
struct Scsi_Host *shost = class_to_shost(dev);
struct ibmvfc_host *vhost = shost_priv(shost);
- return snprintf(buf, PAGE_SIZE, "%s\n",
+ return sysfs_emit(buf, "%s\n",
vhost->login_buf->resp.drc_name);
}
@@ -3442,7 +3442,7 @@ static ssize_t ibmvfc_show_host_npiv_version(struct device *dev,
{
struct Scsi_Host *shost = class_to_shost(dev);
struct ibmvfc_host *vhost = shost_priv(shost);
- return snprintf(buf, PAGE_SIZE, "%d\n", be32_to_cpu(vhost->login_buf->resp.version));
+ return sysfs_emit(buf, "%d\n", be32_to_cpu(vhost->login_buf->resp.version));
}
static ssize_t ibmvfc_show_host_capabilities(struct device *dev,
@@ -3450,7 +3450,7 @@ static ssize_t ibmvfc_show_host_capabilities(struct device *dev,
{
struct Scsi_Host *shost = class_to_shost(dev);
struct ibmvfc_host *vhost = shost_priv(shost);
- return snprintf(buf, PAGE_SIZE, "%llx\n", be64_to_cpu(vhost->login_buf->resp.capabilities));
+ return sysfs_emit(buf, "%llx\n", be64_to_cpu(vhost->login_buf->resp.capabilities));
}
/**
--
2.30.2
^ permalink raw reply related
* Re: [PATCH 0/9] lib/bitmap: optimize bitmap_weight() usage
From: Michał Mirosław @ 2021-11-29 16:34 UTC (permalink / raw)
To: Yury Norov
Cc: Juri Lelli, Andrew Lunn, Rafael J. Wysocki, Catalin Marinas,
Guo Ren, Christoph Lameter, Christoph Hellwig, Andi Kleen,
Vincent Guittot, Ingo Molnar, Geert Uytterhoeven, Mel Gorman,
Viresh Kumar, Petr Mladek, Arnaldo Carvalho de Melo, Jens Axboe,
Andy Lutomirski, Thomas Gleixner, Greg Kroah-Hartman,
Randy Dunlap, linux-kernel, linux-perf-users, Sergey Senozhatsky,
linux-crypto, Tejun Heo, Andrew Morton, Mark Rutland, Anup Patel,
linux-ia64, David Airlie, Roy Pledge, Dave Hansen, Solomon Peachy,
Stephen Rothwell, Krzysztof Kozlowski, Dennis Zhou,
Matti Vaittinen, linux-alpha, Kalle Valo, Stephen Boyd,
Tariq Toukan, Dinh Nguyen, Jonathan Cameron, Ulf Hansson,
Alexander Shishkin, Mike Marciniszyn, Rasmus Villemoes,
Subbaraya Sundeep, Will Deacon, Sagi Grimberg, linux-csky,
bcm-kernel-feedback-list, linux-arm-kernel, linux-snps-arc,
Kees Cook, Arnd Bergmann, James E.J. Bottomley, Vineet Gupta,
Steven Rostedt, Mark Gross, Borislav Petkov,
Mauro Carvalho Chehab, Thomas Bogendoerfer, Martin K. Petersen,
David Laight, Sudeep Holla, Geetha sowjanya, Ian Rogers, kvm,
Peter Zijlstra, Amitkumar Karwar, linux-mm, linux-riscv,
Lee Jones, Ard Biesheuvel, Marc Zyngier, Jiri Olsa, Russell King,
Andy Gross, Jakub Kicinski, Vivien Didelot, Sunil Goutham,
Paul E. McKenney, linux-s390, Alexey Klimov, Heiko Carstens,
Hans de Goede, Nicholas Piggin, Marcin Wojtas, Vlastimil Babka,
linuxppc-dev, linux-mips, Palmer Dabbelt, Daniel Vetter,
Jason Wessel, Saeed Mahameed, Andy Shevchenko
In-Reply-To: <20211129063839.GA338729@lapt>
Dnia 29 listopada 2021 06:38:39 UTC, Yury Norov <yury.norov@gmail.com> napisał/a:
>On Sun, Nov 28, 2021 at 07:03:41PM +0100, mirq-test@rere.qmqm.pl wrote:
>> On Sat, Nov 27, 2021 at 07:56:55PM -0800, Yury Norov wrote:
>> > In many cases people use bitmap_weight()-based functions like this:
>> >
>> > if (num_present_cpus() > 1)
>> > do_something();
>> >
>> > This may take considerable amount of time on many-cpus machines because
>> > num_present_cpus() will traverse every word of underlying cpumask
>> > unconditionally.
>> >
>> > We can significantly improve on it for many real cases if stop traversing
>> > the mask as soon as we count present cpus to any number greater than 1:
>> >
>> > if (num_present_cpus_gt(1))
>> > do_something();
>> >
>> > To implement this idea, the series adds bitmap_weight_{eq,gt,le}
>> > functions together with corresponding wrappers in cpumask and nodemask.
>>
>> Having slept on it I have more structured thoughts:
>>
>> First, I like substituting bitmap_empty/full where possible - I think
>> the change stands on its own, so could be split and sent as is.
>
>Ok, I can do it.
>
>> I don't like the proposed API very much. One problem is that it hides
>> the comparison operator and makes call sites less readable:
>>
>> bitmap_weight(...) > N
>>
>> becomes:
>>
>> bitmap_weight_gt(..., N)
>>
>> and:
>> bitmap_weight(...) <= N
>>
>> becomes:
>>
>> bitmap_weight_lt(..., N+1)
>> or:
>> !bitmap_weight_gt(..., N)
>>
>> I'd rather see something resembling memcmp() API that's known enough
>> to be easier to grasp. For above examples:
>>
>> bitmap_weight_cmp(..., N) > 0
>> bitmap_weight_cmp(..., N) <= 0
>> ...
>
>bitmap_weight_cmp() cannot be efficient. Consider this example:
>
>bitmap_weight_lt(1000 0000 0000 0000, 1) == false
> ^
> stop here
>
>bitmap_weight_cmp(1000 0000 0000 0000, 1) == 0
> ^
> stop here
>
>I agree that '_gt' is less verbose than '>', but the advantage of
>'_gt' over '>' is proportional to length of bitmap, and it means
>that this API should exist.
Thank you for the example. Indeed, for less-than to be efficient here you would need to replace
bitmap_weight_cmp(..., N) < 0
with
bitmap_weight_cmp(..., N-1) <= 0
It would still be more readable, I think.
Best Regards
Michał Mirosław
^ permalink raw reply
* Re: [PATCH 1/2] of: Sort reserved_mem related code
From: Rob Herring @ 2021-11-30 0:01 UTC (permalink / raw)
To: Calvin Zhang
Cc: Mark Rutland, Kefeng Wang, Rich Felker, Jinyang He,
David Hildenbrand, Palmer Dabbelt, linux-kernel, Max Filippov,
Anup Patel, Guo Ren, Guo Ren, linux-csky, Nick Kossifidis,
Vladimir Isaev, Tiezhu Yang, Vincent Chen, Will Deacon,
Markus Elfring, Vitaly Wool, Jonas Bonn, devicetree,
linux-snps-arc, Yoshinori Sato, linux-sh, Rafael J. Wysocki,
Christophe JAILLET, Russell King, Ley Foon Tan,
Geert Uytterhoeven, Aneesh Kumar K.V, Catalin Marinas,
Ganesh Goudar, David Brazdil, linux-riscv, Guenter Roeck,
uclinux-h8-devel, linux-xtensa, Albert Ou, Arnd Bergmann,
Anshuman Khandual, Vineet Gupta, Andreas Oetken,
Stefan Kristiansson, Russell King (Oracle), openrisc,
Alexander Sverdlin, Greentime Hu, Paul Walmsley, Stafford Horne,
Andy Shevchenko, Atish Patra, linux-arm-kernel, Andrey Konovalov,
Christophe Leroy, Chris Zankel, Thomas Bogendoerfer,
Alexandre Ghiti, Nick Hu, Wolfram Sang, linux-mips, Randy Dunlap,
Frank Rowand, Serge Semin, Dinh Nguyen, Zhang Yunkai,
Palmer Dabbelt, Souptick Joarder, Marc Zyngier, Mauri Sandberg,
Paul Mackerras, Andrew Morton, linuxppc-dev, Mike Rapoport
In-Reply-To: <20211119075844.2902592-2-calvinzhang.cool@gmail.com>
On Fri, Nov 19, 2021 at 03:58:18PM +0800, Calvin Zhang wrote:
> Move code about parsing /reserved-memory and initializing of
> reserved_mems array to of_reserved_mem.c for better modularity.
>
> Rename array name from reserved_mem to reserved_mems to distinguish
> from type definition.
>
> Signed-off-by: Calvin Zhang <calvinzhang.cool@gmail.com>
> ---
> drivers/of/fdt.c | 108 +--------------------
> drivers/of/of_private.h | 12 ++-
> drivers/of/of_reserved_mem.c | 163 ++++++++++++++++++++++++++------
> include/linux/of_reserved_mem.h | 4 +
> 4 files changed, 149 insertions(+), 138 deletions(-)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index bdca35284ceb..445af4e69300 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -80,7 +80,7 @@ void __init of_fdt_limit_memory(int limit)
> }
> }
>
> -static bool of_fdt_device_is_available(const void *blob, unsigned long node)
> +bool of_fdt_device_is_available(const void *blob, unsigned long node)
> {
> const char *status = fdt_getprop(blob, node, "status", NULL);
>
> @@ -476,7 +476,7 @@ void *initial_boot_params __ro_after_init;
>
> static u32 of_fdt_crc32;
>
> -static int __init early_init_dt_reserve_memory_arch(phys_addr_t base,
> +int __init early_init_dt_reserve_memory_arch(phys_addr_t base,
> phys_addr_t size, bool nomap)
I think you can move this function too if you change the nomap==false
callers to just call memblock_reserve directly.
> {
> if (nomap) {
> @@ -492,108 +492,6 @@ static int __init early_init_dt_reserve_memory_arch(phys_addr_t base,
> return memblock_reserve(base, size);
> }
>
> -/*
> - * __reserved_mem_reserve_reg() - reserve all memory described in 'reg' property
> - */
> -static int __init __reserved_mem_reserve_reg(unsigned long node,
> - const char *uname)
> -{
> - int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> - phys_addr_t base, size;
> - int len;
> - const __be32 *prop;
> - int first = 1;
> - bool nomap;
> -
> - prop = of_get_flat_dt_prop(node, "reg", &len);
> - if (!prop)
> - return -ENOENT;
> -
> - if (len && len % t_len != 0) {
> - pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
> - uname);
> - return -EINVAL;
> - }
> -
> - nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> -
> - while (len >= t_len) {
> - base = dt_mem_next_cell(dt_root_addr_cells, &prop);
> - size = dt_mem_next_cell(dt_root_size_cells, &prop);
> -
> - if (size &&
> - early_init_dt_reserve_memory_arch(base, size, nomap) == 0)
> - pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %lu MiB\n",
> - uname, &base, (unsigned long)(size / SZ_1M));
> - else
> - pr_info("Reserved memory: failed to reserve memory for node '%s': base %pa, size %lu MiB\n",
> - uname, &base, (unsigned long)(size / SZ_1M));
> -
> - len -= t_len;
> - if (first) {
> - fdt_reserved_mem_save_node(node, uname, base, size);
> - first = 0;
> - }
> - }
> - return 0;
> -}
> -
> -/*
> - * __reserved_mem_check_root() - check if #size-cells, #address-cells provided
> - * in /reserved-memory matches the values supported by the current implementation,
> - * also check if ranges property has been provided
> - */
> -static int __init __reserved_mem_check_root(unsigned long node)
> -{
> - const __be32 *prop;
> -
> - prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> - if (!prop || be32_to_cpup(prop) != dt_root_size_cells)
> - return -EINVAL;
> -
> - prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> - if (!prop || be32_to_cpup(prop) != dt_root_addr_cells)
> - return -EINVAL;
> -
> - prop = of_get_flat_dt_prop(node, "ranges", NULL);
> - if (!prop)
> - return -EINVAL;
> - return 0;
> -}
> -
> -/*
> - * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory
> - */
> -static int __init fdt_scan_reserved_mem(void)
> -{
> - int node, child;
> - const void *fdt = initial_boot_params;
> -
> - node = fdt_path_offset(fdt, "/reserved-memory");
> - if (node < 0)
> - return -ENODEV;
> -
> - if (__reserved_mem_check_root(node) != 0) {
> - pr_err("Reserved memory: unsupported node format, ignoring\n");
> - return -EINVAL;
> - }
> -
> - fdt_for_each_subnode(child, fdt, node) {
> - const char *uname;
> - int err;
> -
> - if (!of_fdt_device_is_available(fdt, child))
> - continue;
> -
> - uname = fdt_get_name(fdt, child, NULL);
> -
> - err = __reserved_mem_reserve_reg(child, uname);
> - if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL))
> - fdt_reserved_mem_save_node(child, uname, 0, 0);
> - }
> - return 0;
> -}
> -
> /*
> * fdt_reserve_elfcorehdr() - reserves memory for elf core header
> *
> @@ -642,7 +540,7 @@ void __init early_init_fdt_scan_reserved_mem(void)
> }
>
> fdt_scan_reserved_mem();
> - fdt_init_reserved_mem();
> + of_reserved_mem_init();
> fdt_reserve_elfcorehdr();
> }
>
> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
> index 9324483397f6..88b67f8ed698 100644
> --- a/drivers/of/of_private.h
> +++ b/drivers/of/of_private.h
> @@ -163,8 +163,14 @@ static inline int of_dma_get_range(struct device_node *np,
> }
> #endif
>
> -void fdt_init_reserved_mem(void);
> -void fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> - phys_addr_t base, phys_addr_t size);
> +bool of_fdt_device_is_available(const void *blob, unsigned long node);
> +int early_init_dt_reserve_memory_arch(phys_addr_t base,
> + phys_addr_t size, bool nomap);
> +#ifdef CONFIG_OF_RESERVED_MEM
> +int fdt_scan_reserved_mem(void);
> +#else
> +static inline int fdt_scan_reserved_mem(void) { }
> +#endif
> +
>
> #endif /* _LINUX_OF_PRIVATE_H */
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 9c0fb962c22b..784cfc5cd251 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -20,13 +20,14 @@
> #include <linux/of_reserved_mem.h>
> #include <linux/sort.h>
> #include <linux/slab.h>
> +#include <linux/libfdt.h>
> #include <linux/memblock.h>
> #include <linux/kmemleak.h>
>
> #include "of_private.h"
>
> #define MAX_RESERVED_REGIONS 64
> -static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
> +static struct reserved_mem reserved_mems[MAX_RESERVED_REGIONS];
Would be a bit easier to review without the rename.
> static int reserved_mem_count;
>
> static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
> @@ -56,12 +57,12 @@ static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
> /*
> * fdt_reserved_mem_save_node() - save fdt node for second pass initialization
> */
> -void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> +static void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> phys_addr_t base, phys_addr_t size)
> {
> - struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];
> + struct reserved_mem *rmem = &reserved_mems[reserved_mem_count];
>
> - if (reserved_mem_count == ARRAY_SIZE(reserved_mem)) {
> + if (reserved_mem_count == ARRAY_SIZE(reserved_mems)) {
> pr_err("not enough space for all defined regions.\n");
> return;
> }
> @@ -173,29 +174,105 @@ static const struct of_device_id __rmem_of_table_sentinel
> __used __section("__reservedmem_of_table_end");
>
> /*
> - * __reserved_mem_init_node() - call region specific reserved memory init code
> + * __reserved_mem_check_root() - check if #size-cells, #address-cells provided
> + * in /reserved-memory matches the values supported by the current implementation,
> + * also check if ranges property has been provided
> */
> -static int __init __reserved_mem_init_node(struct reserved_mem *rmem)
> +static int __init __reserved_mem_check_root(unsigned long node)
> {
> - extern const struct of_device_id __reservedmem_of_table[];
> - const struct of_device_id *i;
> - int ret = -ENOENT;
> + const __be32 *prop;
>
> - for (i = __reservedmem_of_table; i < &__rmem_of_table_sentinel; i++) {
> - reservedmem_of_init_fn initfn = i->data;
> - const char *compat = i->compatible;
> + prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> + if (!prop || be32_to_cpup(prop) != dt_root_size_cells)
> + return -EINVAL;
>
> - if (!of_flat_dt_is_compatible(rmem->fdt_node, compat))
> - continue;
> + prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> + if (!prop || be32_to_cpup(prop) != dt_root_addr_cells)
> + return -EINVAL;
>
> - ret = initfn(rmem);
> - if (ret == 0) {
> - pr_info("initialized node %s, compatible id %s\n",
> - rmem->name, compat);
> - break;
> + prop = of_get_flat_dt_prop(node, "ranges", NULL);
> + if (!prop)
> + return -EINVAL;
> + return 0;
> +}
> +
> +/*
> + * __reserved_mem_reserve_reg() - reserve all memory described in 'reg' property
> + */
> +static int __init __reserved_mem_reserve_reg(unsigned long node,
> + const char *uname)
> +{
> + int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> + phys_addr_t base, size;
> + int len;
> + const __be32 *prop;
> + int first = 1;
> + bool nomap;
> +
> + prop = of_get_flat_dt_prop(node, "reg", &len);
> + if (!prop)
> + return -ENOENT;
> +
> + if (len && len % t_len != 0) {
> + pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
> + uname);
> + return -EINVAL;
> + }
> +
> + nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> +
> + while (len >= t_len) {
> + base = dt_mem_next_cell(dt_root_addr_cells, &prop);
> + size = dt_mem_next_cell(dt_root_size_cells, &prop);
> +
> + if (size &&
> + early_init_dt_reserve_memory_arch(base, size, nomap) == 0)
> + pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %lu MiB\n",
> + uname, &base, (unsigned long)(size / SZ_1M));
> + else
> + pr_info("Reserved memory: failed to reserve memory for node '%s': base %pa, size %lu MiB\n",
> + uname, &base, (unsigned long)(size / SZ_1M));
> +
> + len -= t_len;
> + if (first) {
> + fdt_reserved_mem_save_node(node, uname, base, size);
> + first = 0;
> }
> }
> - return ret;
> + return 0;
> +}
> +
> +/*
> + * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory
> + */
> +int __init fdt_scan_reserved_mem(void)
> +{
> + int node, child;
> + const void *fdt = initial_boot_params;
> +
> + node = fdt_path_offset(fdt, "/reserved-memory");
> + if (node < 0)
> + return -ENODEV;
> +
> + if (__reserved_mem_check_root(node) != 0) {
> + pr_err("Reserved memory: unsupported node format, ignoring\n");
> + return -EINVAL;
> + }
> +
> + fdt_for_each_subnode(child, fdt, node) {
> + const char *uname;
> + int err;
> +
> + if (!of_fdt_device_is_available(fdt, child))
> + continue;
> +
> + uname = fdt_get_name(fdt, child, NULL);
> +
> + err = __reserved_mem_reserve_reg(child, uname);
> + if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL))
> + fdt_reserved_mem_save_node(child, uname, 0, 0);
> + }
> + return 0;
> }
>
> static int __init __rmem_cmp(const void *a, const void *b)
> @@ -228,13 +305,13 @@ static void __init __rmem_check_for_overlap(void)
> if (reserved_mem_count < 2)
> return;
>
> - sort(reserved_mem, reserved_mem_count, sizeof(reserved_mem[0]),
> + sort(reserved_mems, reserved_mem_count, sizeof(reserved_mems[0]),
> __rmem_cmp, NULL);
> for (i = 0; i < reserved_mem_count - 1; i++) {
> struct reserved_mem *this, *next;
>
> - this = &reserved_mem[i];
> - next = &reserved_mem[i + 1];
> + this = &reserved_mems[i];
> + next = &reserved_mems[i + 1];
>
> if (this->base + this->size > next->base) {
> phys_addr_t this_end, next_end;
> @@ -248,10 +325,36 @@ static void __init __rmem_check_for_overlap(void)
> }
> }
>
> +/*
> + * __reserved_mem_init_node() - call region specific reserved memory init code
> + */
> +static int __init __reserved_mem_init_node(struct reserved_mem *rmem)
> +{
> + extern const struct of_device_id __reservedmem_of_table[];
> + const struct of_device_id *i;
> + int ret = -ENOENT;
> +
> + for (i = __reservedmem_of_table; i < &__rmem_of_table_sentinel; i++) {
> + reservedmem_of_init_fn initfn = i->data;
> + const char *compat = i->compatible;
> +
> + if (!of_flat_dt_is_compatible(rmem->fdt_node, compat))
> + continue;
> +
> + ret = initfn(rmem);
> + if (ret == 0) {
> + pr_info("initialized node %s, compatible id %s\n",
> + rmem->name, compat);
> + break;
> + }
> + }
> + return ret;
> +}
> +
> /**
> - * fdt_init_reserved_mem() - allocate and init all saved reserved memory regions
> + * of_reserved_mem_init() - allocate and init all saved reserved memory regions
> */
> -void __init fdt_init_reserved_mem(void)
> +void __init of_reserved_mem_init(void)
> {
> int i;
>
> @@ -259,7 +362,7 @@ void __init fdt_init_reserved_mem(void)
> __rmem_check_for_overlap();
>
> for (i = 0; i < reserved_mem_count; i++) {
> - struct reserved_mem *rmem = &reserved_mem[i];
> + struct reserved_mem *rmem = &reserved_mems[i];
> unsigned long node = rmem->fdt_node;
> int len;
> const __be32 *prop;
> @@ -299,8 +402,8 @@ static inline struct reserved_mem *__find_rmem(struct device_node *node)
> return NULL;
>
> for (i = 0; i < reserved_mem_count; i++)
> - if (reserved_mem[i].phandle == node->phandle)
> - return &reserved_mem[i];
> + if (reserved_mems[i].phandle == node->phandle)
> + return &reserved_mems[i];
> return NULL;
> }
>
> @@ -442,8 +545,8 @@ struct reserved_mem *of_reserved_mem_lookup(struct device_node *np)
>
> name = kbasename(np->full_name);
> for (i = 0; i < reserved_mem_count; i++)
> - if (!strcmp(reserved_mem[i].name, name))
> - return &reserved_mem[i];
> + if (!strcmp(reserved_mems[i].name, name))
> + return &reserved_mems[i];
>
> return NULL;
> }
> diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
> index 4de2a24cadc9..34e134bec606 100644
> --- a/include/linux/of_reserved_mem.h
> +++ b/include/linux/of_reserved_mem.h
> @@ -32,6 +32,8 @@ typedef int (*reservedmem_of_init_fn)(struct reserved_mem *rmem);
> #define RESERVEDMEM_OF_DECLARE(name, compat, init) \
> _OF_DECLARE(reservedmem, name, compat, init, reservedmem_of_init_fn)
>
> +void of_reserved_mem_init(void);
> +
> int of_reserved_mem_device_init_by_idx(struct device *dev,
> struct device_node *np, int idx);
> int of_reserved_mem_device_init_by_name(struct device *dev,
> @@ -45,6 +47,8 @@ struct reserved_mem *of_reserved_mem_lookup(struct device_node *np);
> #define RESERVEDMEM_OF_DECLARE(name, compat, init) \
> _OF_DECLARE_STUB(reservedmem, name, compat, init, reservedmem_of_init_fn)
>
> +static inline void of_reserved_mem_init(void) { }
> +
> static inline int of_reserved_mem_device_init_by_idx(struct device *dev,
> struct device_node *np, int idx)
> {
> --
> 2.30.2
>
>
^ permalink raw reply
* Re: [PATCH 0/2] of: remove reserved regions count restriction
From: Rob Herring @ 2021-11-30 0:08 UTC (permalink / raw)
To: Mike Rapoport
Cc: Kirill A. Shutemov, Mark Rutland, Kefeng Wang, Rich Felker,
Jinyang He, David Hildenbrand, Lee Jones, linux-kernel,
Max Filippov, Anup Patel, Guo Ren, Guo Ren, Calvin Zhang,
Nick Kossifidis, Vladimir Isaev, Tiezhu Yang, Vincent Chen,
Will Deacon, Markus Elfring, Vitaly Wool, Jonas Bonn, devicetree,
linux-snps-arc, uclinux-h8-devel, Yoshinori Sato, Palmer Dabbelt,
linux-sh, Rafael J. Wysocki, Russell King, Ley Foon Tan,
Geert Uytterhoeven, Aneesh Kumar K.V, Catalin Marinas,
Ganesh Goudar, David Brazdil, linux-riscv, Guenter Roeck,
Alexander Sverdlin, Thierry Reding, Albert Ou, Arnd Bergmann,
Anshuman Khandual, linux-xtensa, Vineet Gupta, Andreas Oetken,
Stefan Kristiansson, Russell King (Oracle), linux-csky,
Christophe JAILLET, Greentime Hu, Paul Walmsley, Stafford Horne,
Andy Shevchenko, linux-arm-kernel, Andrey Konovalov,
Christophe Leroy, Chris Zankel, Thomas Bogendoerfer, linux-mips,
Alexandre Ghiti, Nick Hu, Atish Patra, Greg Kroah-Hartman,
Randy Dunlap, Frank Rowand, Serge Semin, Dinh Nguyen,
Zhang Yunkai, Palmer Dabbelt, Souptick Joarder, Marc Zyngier,
Mauri Sandberg, Paul Mackerras, Andrew Morton, linuxppc-dev,
openrisc
In-Reply-To: <YZnqo3oA7srQik4N@kernel.org>
On Sun, Nov 21, 2021 at 08:43:47AM +0200, Mike Rapoport wrote:
> On Fri, Nov 19, 2021 at 03:58:17PM +0800, Calvin Zhang wrote:
> > The count of reserved regions in /reserved-memory was limited because
> > the struct reserved_mem array was defined statically. This series sorts
> > out reserved memory code and allocates that array from early allocator.
> >
> > Note: reserved region with fixed location must be reserved before any
> > memory allocation. While struct reserved_mem array should be allocated
> > after allocator is activated. We make early_init_fdt_scan_reserved_mem()
> > do reservation only and add another call to initialize reserved memory.
> > So arch code have to change for it.
>
> I think much simpler would be to use the same constant for sizing
> memblock.reserved and reserved_mem arrays.
Do those arrays get shrunk? Or do we waste the memory forever?
Maybe we can copy and shrink the initial array? Though I suspect struct
reserved_mem pointers have already been given out.
>
> If there is too much reserved regions in the device tree, reserving them in
> memblock will fail anyway because memblock also starts with static array
> for memblock.reserved, so doing one pass with memblock_reserve() and
> another to set up reserved_mem wouldn't help anyway.
>
> > I'm only familiar with arm and arm64 architectures. Approvals from arch
> > maintainers are required. Thank you all.
> >
> > Calvin Zhang (2):
> > of: Sort reserved_mem related code
> > of: reserved_mem: Remove reserved regions count restriction
> >
> > arch/arc/mm/init.c | 3 +
> > arch/arm/kernel/setup.c | 2 +
> > arch/arm64/kernel/setup.c | 3 +
> > arch/csky/kernel/setup.c | 3 +
> > arch/h8300/kernel/setup.c | 2 +
> > arch/mips/kernel/setup.c | 3 +
> > arch/nds32/kernel/setup.c | 3 +
> > arch/nios2/kernel/setup.c | 2 +
> > arch/openrisc/kernel/setup.c | 3 +
> > arch/powerpc/kernel/setup-common.c | 3 +
> > arch/riscv/kernel/setup.c | 2 +
> > arch/sh/kernel/setup.c | 3 +
> > arch/xtensa/kernel/setup.c | 2 +
> > drivers/of/fdt.c | 107 +---------------
> > drivers/of/of_private.h | 12 +-
> > drivers/of/of_reserved_mem.c | 189 ++++++++++++++++++++++++-----
> > include/linux/of_reserved_mem.h | 4 +
> > 17 files changed, 207 insertions(+), 139 deletions(-)
> >
> > --
> > 2.30.2
> >
>
> --
> Sincerely yours,
> Mike.
>
^ permalink raw reply
* Re: [PATCH] powerpc/rtas: Introduce rtas_get_sensor_nonblocking() for pci hotplug driver.
From: Nathan Lynch @ 2021-11-30 1:06 UTC (permalink / raw)
To: Tyrel Datwyler, Mahesh Salgaonkar, linuxppc-dev
Cc: lxie, Oliver O'Halloran
In-Reply-To: <2083eff5-a00e-5a2b-4e83-20deb302a756@linux.ibm.com>
Tyrel Datwyler <tyreld@linux.ibm.com> writes:
> On 11/29/21 12:58 AM, Mahesh Salgaonkar wrote:
>> -int rtas_get_sensor_fast(int sensor, int index, int *state)
>> +static int
>> +__rtas_get_sensor(int sensor, int index, int *state, bool warn_on)
>> {
>> int token = rtas_token("get-sensor-state");
>> int rc;
>> @@ -618,14 +619,26 @@ int rtas_get_sensor_fast(int sensor, int index, int *state)
>> return -ENOENT;
>>
>> rc = rtas_call(token, 2, 2, state, sensor, index);
>> - WARN_ON(rc == RTAS_BUSY || (rc >= RTAS_EXTENDED_DELAY_MIN &&
>> - rc <= RTAS_EXTENDED_DELAY_MAX));
>> + WARN_ON(warn_on &&
>> + (rc == RTAS_BUSY || (rc >= RTAS_EXTENDED_DELAY_MIN &&
>> + rc <= RTAS_EXTENDED_DELAY_MAX)));
>
> The whole point of rtas_get_sensor_fast() is that on busy we will just let it
> error out because we don't want to wait. I'm not sure I see the point of the
> spurious WARN_ONs anytime we hit a BUSY or DELAY return code. Maybe converting
> that to a pr_debug() might be better and save expanding the API with a _fast and
> _nonblocking variant that do the same thing minus one surpressing a
> WARN_ON splat.
There is a subset of sensors that are specified to not ever return busy
or delay statuses. rtas_get_sensor_fast() is meant to be used with
those, and it would be an error to use it on a sensor not in that set.
So the WARN_ON() is appropriate IMO; if it triggers it indicates either
a misuse of the API or a firmware bug. See commit 1c2cb594441d
"powerpc/rtas: Introduce rtas_get_sensor_fast() for IRQ handlers"
^ permalink raw reply
* Re: [PATCH] powerpc/rtas: Introduce rtas_get_sensor_nonblocking() for pci hotplug driver.
From: Tyrel Datwyler @ 2021-11-30 1:21 UTC (permalink / raw)
To: Nathan Lynch, Mahesh Salgaonkar, linuxppc-dev; +Cc: lxie, Oliver O'Halloran
In-Reply-To: <87sfveo3je.fsf@linux.ibm.com>
On 11/29/21 5:06 PM, Nathan Lynch wrote:
> Tyrel Datwyler <tyreld@linux.ibm.com> writes:
>> On 11/29/21 12:58 AM, Mahesh Salgaonkar wrote:
>>> -int rtas_get_sensor_fast(int sensor, int index, int *state)
>>> +static int
>>> +__rtas_get_sensor(int sensor, int index, int *state, bool warn_on)
>>> {
>>> int token = rtas_token("get-sensor-state");
>>> int rc;
>>> @@ -618,14 +619,26 @@ int rtas_get_sensor_fast(int sensor, int index, int *state)
>>> return -ENOENT;
>>>
>>> rc = rtas_call(token, 2, 2, state, sensor, index);
>>> - WARN_ON(rc == RTAS_BUSY || (rc >= RTAS_EXTENDED_DELAY_MIN &&
>>> - rc <= RTAS_EXTENDED_DELAY_MAX));
>>> + WARN_ON(warn_on &&
>>> + (rc == RTAS_BUSY || (rc >= RTAS_EXTENDED_DELAY_MIN &&
>>> + rc <= RTAS_EXTENDED_DELAY_MAX)));
>>
>> The whole point of rtas_get_sensor_fast() is that on busy we will just let it
>> error out because we don't want to wait. I'm not sure I see the point of the
>> spurious WARN_ONs anytime we hit a BUSY or DELAY return code. Maybe converting
>> that to a pr_debug() might be better and save expanding the API with a _fast and
>> _nonblocking variant that do the same thing minus one surpressing a
>> WARN_ON splat.
>
> There is a subset of sensors that are specified to not ever return busy
> or delay statuses. rtas_get_sensor_fast() is meant to be used with
> those, and it would be an error to use it on a sensor not in that set.
> So the WARN_ON() is appropriate IMO; if it triggers it indicates either
> a misuse of the API or a firmware bug. See commit 1c2cb594441d
> "powerpc/rtas: Introduce rtas_get_sensor_fast() for IRQ handlers"
>
Fair enough. Seems I misremembered the nature of the original problem and should
have looked back at the commit to completely jog my memory.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox