* [PATCH v13 5/8] powerpc/vdso: Prepare for switching VDSO to generic C implementation.
From: Christophe Leroy @ 2020-11-03 18:07 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, nathanl,
anton
Cc: linux-arch, arnd, linux-kernel, luto, tglx, vincenzo.frascino,
linuxppc-dev
In-Reply-To: <cover.1604426550.git.christophe.leroy@csgroup.eu>
Prepare for switching VDSO to generic C implementation in following
patch. Here, we:
- Prepare the helpers to call the C VDSO functions
- Prepare the required callbacks for the C VDSO functions
- Prepare the clocksource.h files to define VDSO_ARCH_CLOCKMODES
- Add the C trampolines to the generic C VDSO functions
powerpc is a bit special for VDSO as well as system calls in the
way that it requires setting CR SO bit which cannot be done in C.
Therefore, entry/exit needs to be performed in ASM.
Implementing __arch_get_vdso_data() would clobber the link register,
requiring the caller to save it. As the ASM calling function already
has to set a stack frame and saves the link register before calling
the C vdso function, retriving the vdso data pointer there is lighter.
Implement __arch_vdso_capable() and always return true.
Provide vdso_shift_ns(), as the generic x >> s gives the following
bad result:
18: 35 25 ff e0 addic. r9,r5,-32
1c: 41 80 00 10 blt 2c <shift+0x14>
20: 7c 64 4c 30 srw r4,r3,r9
24: 38 60 00 00 li r3,0
...
2c: 54 69 08 3c rlwinm r9,r3,1,0,30
30: 21 45 00 1f subfic r10,r5,31
34: 7c 84 2c 30 srw r4,r4,r5
38: 7d 29 50 30 slw r9,r9,r10
3c: 7c 63 2c 30 srw r3,r3,r5
40: 7d 24 23 78 or r4,r9,r4
In our case the shift is always <= 32. In addition, the upper 32 bits
of the result are likely nul. Lets GCC know it, it also optimises the
following calculations.
With the patch, we get:
0: 21 25 00 20 subfic r9,r5,32
4: 7c 69 48 30 slw r9,r3,r9
8: 7c 84 2c 30 srw r4,r4,r5
c: 7d 24 23 78 or r4,r9,r4
10: 7c 63 2c 30 srw r3,r3,r5
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v13:
- Using PPC_MIN_STKFRM instead of STACK_FRAME_OVERHEAD to avoid including ptrace.h
- Reworked header inclusion to avoid build failure when building VDSO32 on PPC64
- __arch_vdso_capable() now returns always true after the removal of ppc601
- Using DOTSYM() to call the function directly.
v11:
- Changed of __arch_get_hw_counter() to adapt to 4c5a116ada95
("vdso/treewide: Add vdso_data pointer argument to __arch_get_hw_counter()")
v10:
- Added a comment to explain the reason for the two stack frames.
- Moved back the .cfi_register next to mflr
v9:
- No more modification of __get_datapage(). Offset is added after.
- Adding a second stack frame because the PPC VDSO ABI doesn't force
the caller to set one.
v8:
- New, splitted out of last patch of the series
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/clocksource.h | 7 +
arch/powerpc/include/asm/ppc_asm.h | 2 +
arch/powerpc/include/asm/vdso/clocksource.h | 7 +
arch/powerpc/include/asm/vdso/gettimeofday.h | 187 +++++++++++++++++++
arch/powerpc/kernel/vdso32/vgettimeofday.c | 28 +++
arch/powerpc/kernel/vdso64/vgettimeofday.c | 29 +++
6 files changed, 260 insertions(+)
create mode 100644 arch/powerpc/include/asm/clocksource.h
create mode 100644 arch/powerpc/include/asm/vdso/clocksource.h
create mode 100644 arch/powerpc/include/asm/vdso/gettimeofday.h
create mode 100644 arch/powerpc/kernel/vdso32/vgettimeofday.c
create mode 100644 arch/powerpc/kernel/vdso64/vgettimeofday.c
diff --git a/arch/powerpc/include/asm/clocksource.h b/arch/powerpc/include/asm/clocksource.h
new file mode 100644
index 000000000000..482185566b0c
--- /dev/null
+++ b/arch/powerpc/include/asm/clocksource.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_CLOCKSOURCE_H
+#define _ASM_CLOCKSOURCE_H
+
+#include <asm/vdso/clocksource.h>
+
+#endif
diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
index 511786f0e40d..db158accb63e 100644
--- a/arch/powerpc/include/asm/ppc_asm.h
+++ b/arch/powerpc/include/asm/ppc_asm.h
@@ -251,6 +251,8 @@ GLUE(.,name):
#define _GLOBAL_TOC(name) _GLOBAL(name)
+#define DOTSYM(a) a
+
#endif
/*
diff --git a/arch/powerpc/include/asm/vdso/clocksource.h b/arch/powerpc/include/asm/vdso/clocksource.h
new file mode 100644
index 000000000000..ec5d672d2569
--- /dev/null
+++ b/arch/powerpc/include/asm/vdso/clocksource.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_VDSOCLOCKSOURCE_H
+#define __ASM_VDSOCLOCKSOURCE_H
+
+#define VDSO_ARCH_CLOCKMODES VDSO_CLOCKMODE_ARCHTIMER
+
+#endif
diff --git a/arch/powerpc/include/asm/vdso/gettimeofday.h b/arch/powerpc/include/asm/vdso/gettimeofday.h
new file mode 100644
index 000000000000..7a8eb9dfd06b
--- /dev/null
+++ b/arch/powerpc/include/asm/vdso/gettimeofday.h
@@ -0,0 +1,187 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_VDSO_GETTIMEOFDAY_H
+#define __ASM_VDSO_GETTIMEOFDAY_H
+
+#ifdef __ASSEMBLY__
+
+#include <asm/ppc_asm.h>
+
+/*
+ * The macros sets two stack frames, one for the caller and one for the callee
+ * because there are no requirement for the caller to set a stack frame when
+ * calling VDSO so it may have omitted to set one, especially on PPC64
+ */
+
+.macro cvdso_call funct
+ .cfi_startproc
+ PPC_STLU r1, -PPC_MIN_STKFRM(r1)
+ mflr r0
+ .cfi_register lr, r0
+ PPC_STLU r1, -PPC_MIN_STKFRM(r1)
+ PPC_STL r0, PPC_MIN_STKFRM + PPC_LR_STKOFF(r1)
+ get_datapage r5, r0
+ addi r5, r5, VDSO_DATA_OFFSET
+ bl DOTSYM(\funct)
+ PPC_LL r0, PPC_MIN_STKFRM + PPC_LR_STKOFF(r1)
+ cmpwi r3, 0
+ mtlr r0
+ .cfi_restore lr
+ addi r1, r1, 2 * PPC_MIN_STKFRM
+ crclr so
+ beqlr+
+ crset so
+ neg r3, r3
+ blr
+ .cfi_endproc
+.endm
+
+.macro cvdso_call_time funct
+ .cfi_startproc
+ PPC_STLU r1, -PPC_MIN_STKFRM(r1)
+ mflr r0
+ .cfi_register lr, r0
+ PPC_STLU r1, -PPC_MIN_STKFRM(r1)
+ PPC_STL r0, PPC_MIN_STKFRM + PPC_LR_STKOFF(r1)
+ get_datapage r4, r0
+ addi r4, r4, VDSO_DATA_OFFSET
+ bl DOTSYM(\funct)
+ PPC_LL r0, PPC_MIN_STKFRM + PPC_LR_STKOFF(r1)
+ crclr so
+ mtlr r0
+ .cfi_restore lr
+ addi r1, r1, 2 * PPC_MIN_STKFRM
+ blr
+ .cfi_endproc
+.endm
+
+#else
+
+#include <asm/timebase.h>
+#include <asm/barrier.h>
+#include <asm/unistd.h>
+#include <uapi/linux/time.h>
+
+#define VDSO_HAS_CLOCK_GETRES 1
+
+#define VDSO_HAS_TIME 1
+
+static __always_inline int do_syscall_2(const unsigned long _r0, const unsigned long _r3,
+ const unsigned long _r4)
+{
+ register long r0 asm("r0") = _r0;
+ register unsigned long r3 asm("r3") = _r3;
+ register unsigned long r4 asm("r4") = _r4;
+ register int ret asm ("r3");
+
+ asm volatile(
+ " sc\n"
+ " bns+ 1f\n"
+ " neg %0, %0\n"
+ "1:\n"
+ : "=r" (ret), "+r" (r4), "+r" (r0)
+ : "r" (r3)
+ : "memory", "r5", "r6", "r7", "r8", "r9", "r10", "r11", "r12", "cr0", "ctr");
+
+ return ret;
+}
+
+static __always_inline
+int gettimeofday_fallback(struct __kernel_old_timeval *_tv, struct timezone *_tz)
+{
+ return do_syscall_2(__NR_gettimeofday, (unsigned long)_tv, (unsigned long)_tz);
+}
+
+static __always_inline
+int clock_gettime_fallback(clockid_t _clkid, struct __kernel_timespec *_ts)
+{
+ return do_syscall_2(__NR_clock_gettime, _clkid, (unsigned long)_ts);
+}
+
+static __always_inline
+int clock_getres_fallback(clockid_t _clkid, struct __kernel_timespec *_ts)
+{
+ return do_syscall_2(__NR_clock_getres, _clkid, (unsigned long)_ts);
+}
+
+#ifdef CONFIG_VDSO32
+
+#define BUILD_VDSO32 1
+
+static __always_inline
+int clock_gettime32_fallback(clockid_t _clkid, struct old_timespec32 *_ts)
+{
+ return do_syscall_2(__NR_clock_gettime, _clkid, (unsigned long)_ts);
+}
+
+static __always_inline
+int clock_getres32_fallback(clockid_t _clkid, struct old_timespec32 *_ts)
+{
+ return do_syscall_2(__NR_clock_getres, _clkid, (unsigned long)_ts);
+}
+#endif
+
+static __always_inline u64 __arch_get_hw_counter(s32 clock_mode,
+ const struct vdso_data *vd)
+{
+ return get_tb();
+}
+
+const struct vdso_data *__arch_get_vdso_data(void);
+
+static inline bool vdso_clocksource_ok(const struct vdso_data *vd)
+{
+ return true;
+}
+#define vdso_clocksource_ok vdso_clocksource_ok
+
+/*
+ * powerpc specific delta calculation.
+ *
+ * This variant removes the masking of the subtraction because the
+ * clocksource mask of all VDSO capable clocksources on powerpc is U64_MAX
+ * which would result in a pointless operation. The compiler cannot
+ * optimize it away as the mask comes from the vdso data and is not compile
+ * time constant.
+ */
+static __always_inline u64 vdso_calc_delta(u64 cycles, u64 last, u64 mask, u32 mult)
+{
+ return (cycles - last) * mult;
+}
+#define vdso_calc_delta vdso_calc_delta
+
+#ifndef __powerpc64__
+static __always_inline u64 vdso_shift_ns(u64 ns, unsigned long shift)
+{
+ u32 hi = ns >> 32;
+ u32 lo = ns;
+
+ lo >>= shift;
+ lo |= hi << (32 - shift);
+ hi >>= shift;
+
+ if (likely(hi == 0))
+ return lo;
+
+ return ((u64)hi << 32) | lo;
+}
+#define vdso_shift_ns vdso_shift_ns
+#endif
+
+#ifdef __powerpc64__
+int __c_kernel_clock_gettime(clockid_t clock, struct __kernel_timespec *ts,
+ const struct vdso_data *vd);
+int __c_kernel_clock_getres(clockid_t clock_id, struct __kernel_timespec *res,
+ const struct vdso_data *vd);
+#else
+int __c_kernel_clock_gettime(clockid_t clock, struct old_timespec32 *ts,
+ const struct vdso_data *vd);
+int __c_kernel_clock_getres(clockid_t clock_id, struct old_timespec32 *res,
+ const struct vdso_data *vd);
+#endif
+int __c_kernel_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz,
+ const struct vdso_data *vd);
+__kernel_old_time_t __c_kernel_time(__kernel_old_time_t *time,
+ const struct vdso_data *vd);
+#endif /* __ASSEMBLY__ */
+
+#endif /* __ASM_VDSO_GETTIMEOFDAY_H */
diff --git a/arch/powerpc/kernel/vdso32/vgettimeofday.c b/arch/powerpc/kernel/vdso32/vgettimeofday.c
new file mode 100644
index 000000000000..0d4bc217529e
--- /dev/null
+++ b/arch/powerpc/kernel/vdso32/vgettimeofday.c
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Powerpc userspace implementations of gettimeofday() and similar.
+ */
+#include <linux/types.h>
+
+int __c_kernel_clock_gettime(clockid_t clock, struct old_timespec32 *ts,
+ const struct vdso_data *vd)
+{
+ return __cvdso_clock_gettime32_data(vd, clock, ts);
+}
+
+int __c_kernel_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz,
+ const struct vdso_data *vd)
+{
+ return __cvdso_gettimeofday_data(vd, tv, tz);
+}
+
+int __c_kernel_clock_getres(clockid_t clock_id, struct old_timespec32 *res,
+ const struct vdso_data *vd)
+{
+ return __cvdso_clock_getres_time32_data(vd, clock_id, res);
+}
+
+__kernel_old_time_t __c_kernel_time(__kernel_old_time_t *time, const struct vdso_data *vd)
+{
+ return __cvdso_time_data(vd, time);
+}
diff --git a/arch/powerpc/kernel/vdso64/vgettimeofday.c b/arch/powerpc/kernel/vdso64/vgettimeofday.c
new file mode 100644
index 000000000000..5b5500058344
--- /dev/null
+++ b/arch/powerpc/kernel/vdso64/vgettimeofday.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Powerpc userspace implementations of gettimeofday() and similar.
+ */
+#include <linux/time.h>
+#include <linux/types.h>
+
+int __c_kernel_clock_gettime(clockid_t clock, struct __kernel_timespec *ts,
+ const struct vdso_data *vd)
+{
+ return __cvdso_clock_gettime_data(vd, clock, ts);
+}
+
+int __c_kernel_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz,
+ const struct vdso_data *vd)
+{
+ return __cvdso_gettimeofday_data(vd, tv, tz);
+}
+
+int __c_kernel_clock_getres(clockid_t clock_id, struct __kernel_timespec *res,
+ const struct vdso_data *vd)
+{
+ return __cvdso_clock_getres_data(vd, clock_id, res);
+}
+
+__kernel_old_time_t __c_kernel_time(__kernel_old_time_t *time, const struct vdso_data *vd)
+{
+ return __cvdso_time_data(vd, time);
+}
--
2.25.0
^ permalink raw reply related
* [PATCH v13 6/8] powerpc/vdso: Save and restore TOC pointer on PPC64
From: Christophe Leroy @ 2020-11-03 18:07 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, nathanl,
anton
Cc: linux-arch, arnd, linux-kernel, luto, tglx, vincenzo.frascino,
linuxppc-dev
In-Reply-To: <cover.1604426550.git.christophe.leroy@csgroup.eu>
On PPC64, the TOC pointer needs to be saved and restored.
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v13: Using __powerpc64__ instead of CONFIG_PPC64 to exclude it from VDSO32 build on PPC64.
v9: New.
I'm not sure this is really needed, I can't see the VDSO C code doing
anything with r2, at least on ppc64_defconfig.
So I let you decide whether you take it or not.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/vdso/gettimeofday.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/powerpc/include/asm/vdso/gettimeofday.h b/arch/powerpc/include/asm/vdso/gettimeofday.h
index 7a8eb9dfd06b..cdbd297e61b8 100644
--- a/arch/powerpc/include/asm/vdso/gettimeofday.h
+++ b/arch/powerpc/include/asm/vdso/gettimeofday.h
@@ -19,10 +19,16 @@
.cfi_register lr, r0
PPC_STLU r1, -PPC_MIN_STKFRM(r1)
PPC_STL r0, PPC_MIN_STKFRM + PPC_LR_STKOFF(r1)
+#ifdef __powerpc64__
+ PPC_STL r2, PPC_MIN_STKFRM + STK_GOT(r1)
+#endif
get_datapage r5, r0
addi r5, r5, VDSO_DATA_OFFSET
bl DOTSYM(\funct)
PPC_LL r0, PPC_MIN_STKFRM + PPC_LR_STKOFF(r1)
+#ifdef __powerpc64__
+ PPC_LL r2, PPC_MIN_STKFRM + STK_GOT(r1)
+#endif
cmpwi r3, 0
mtlr r0
.cfi_restore lr
@@ -42,10 +48,16 @@
.cfi_register lr, r0
PPC_STLU r1, -PPC_MIN_STKFRM(r1)
PPC_STL r0, PPC_MIN_STKFRM + PPC_LR_STKOFF(r1)
+#ifdef __powerpc64__
+ PPC_STL r2, PPC_MIN_STKFRM + STK_GOT(r1)
+#endif
get_datapage r4, r0
addi r4, r4, VDSO_DATA_OFFSET
bl DOTSYM(\funct)
PPC_LL r0, PPC_MIN_STKFRM + PPC_LR_STKOFF(r1)
+#ifdef __powerpc64__
+ PPC_LL r2, PPC_MIN_STKFRM + STK_GOT(r1)
+#endif
crclr so
mtlr r0
.cfi_restore lr
--
2.25.0
^ permalink raw reply related
* [PATCH v13 4/8] powerpc/time: Move timebase functions into new asm/timebase.h
From: Christophe Leroy @ 2020-11-03 18:07 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, nathanl,
anton
Cc: linux-arch, arnd, linux-kernel, luto, tglx, vincenzo.frascino,
linuxppc-dev
In-Reply-To: <cover.1604426550.git.christophe.leroy@csgroup.eu>
In order to easily use get_tb() from C VDSO, move timebase
functions into a new header named asm/timebase.h
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v13: new
---
arch/powerpc/include/asm/time.h | 30 +--------------------
arch/powerpc/include/asm/timebase.h | 42 +++++++++++++++++++++++++++++
2 files changed, 43 insertions(+), 29 deletions(-)
create mode 100644 arch/powerpc/include/asm/timebase.h
diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index 2f566c1a754c..46d0d65cda0e 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -15,6 +15,7 @@
#include <asm/processor.h>
#include <asm/cpu_has_feature.h>
+#include <asm/timebase.h>
/* time.c */
extern unsigned long tb_ticks_per_jiffy;
@@ -38,12 +39,6 @@ struct div_result {
u64 result_low;
};
-/* For compatibility, get_tbl() is defined as get_tb() on ppc64 */
-static inline unsigned long get_tbl(void)
-{
- return mftb();
-}
-
static inline u64 get_vtb(void)
{
#ifdef CONFIG_PPC_BOOK3S_64
@@ -53,29 +48,6 @@ static inline u64 get_vtb(void)
return 0;
}
-static inline u64 get_tb(void)
-{
- unsigned int tbhi, tblo, tbhi2;
-
- if (IS_ENABLED(CONFIG_PPC64))
- return mftb();
-
- do {
- tbhi = mftbu();
- tblo = mftb();
- tbhi2 = mftbu();
- } while (tbhi != tbhi2);
-
- return ((u64)tbhi << 32) | tblo;
-}
-
-static inline void set_tb(unsigned int upper, unsigned int lower)
-{
- mtspr(SPRN_TBWL, 0);
- mtspr(SPRN_TBWU, upper);
- mtspr(SPRN_TBWL, lower);
-}
-
/* Accessor functions for the decrementer register.
* The 4xx doesn't even have a decrementer. I tried to use the
* generic timer interrupt code, which seems OK, with the 4xx PIT
diff --git a/arch/powerpc/include/asm/timebase.h b/arch/powerpc/include/asm/timebase.h
new file mode 100644
index 000000000000..a8eae3adaa91
--- /dev/null
+++ b/arch/powerpc/include/asm/timebase.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Common timebase prototypes and such for all ppc machines.
+ *
+ * Written by Cort Dougan (cort@cs.nmt.edu) to merge
+ * Paul Mackerras' version and mine for PReP and Pmac.
+ */
+
+#ifndef __POWERPC_TIMEBASE_H
+#define __POWERPC_TIMEBASE_H
+
+#include <asm/reg.h>
+
+/* For compatibility, get_tbl() is defined as get_tb() on ppc64 */
+static inline unsigned long get_tbl(void)
+{
+ return mftb();
+}
+
+static inline u64 get_tb(void)
+{
+ unsigned int tbhi, tblo, tbhi2;
+
+ if (IS_ENABLED(CONFIG_PPC64))
+ return mftb();
+
+ do {
+ tbhi = mftbu();
+ tblo = mftb();
+ tbhi2 = mftbu();
+ } while (tbhi != tbhi2);
+
+ return ((u64)tbhi << 32) | tblo;
+}
+
+static inline void set_tb(unsigned int upper, unsigned int lower)
+{
+ mtspr(SPRN_TBWL, 0);
+ mtspr(SPRN_TBWU, upper);
+ mtspr(SPRN_TBWL, lower);
+}
+#endif /* __POWERPC_TIMEBASE_H */
--
2.25.0
^ permalink raw reply related
* [PATCH v13 0/8] powerpc: switch VDSO to C implementation
From: Christophe Leroy @ 2020-11-03 18:07 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, nathanl,
anton
Cc: linux-arch, arnd, linux-kernel, luto, tglx, vincenzo.frascino,
linuxppc-dev
This is a series to switch powerpc VDSO to generic C implementation.
Changes in v13:
- Reorganised headers to avoid the need for a fake 32 bits config for building VDSO32 on PPC64
- Rebased after the removal of powerpc 601
- Using DOTSYM() macro to call functions directly without using OPD
- Explicitely dropped .opd and .got1 sections which are now unused
Changes in v12:
- Rebased to today's powerpc/merge branch (Conflicts on VDSO Makefiles)
- Added missing prototype for __kernel_clock_gettime64()
Changes in v11:
- Rebased to today's powerpc/merge branch
- Prototype of __arch_get_hw_counter() was modified in mainline (patch 2)
Changes in v10 are:
- Added a comment explaining the reason for the double stack frame
- Moved back .cfi_register lr next to mflr
Main changes in v9 are:
- Dropped the patches which put the VDSO datapage in front of VDSO text in the mapping
- Adds a second stack frame because the caller doesn't set one, at least on PPC64
- Saving the TOC pointer on PPC64 (is that really needed ?)
This series applies on today's powerpc/merge branch.
See the last patches for details on changes and performance.
Christophe Leroy (8):
powerpc/feature: Fix CPU_FTRS_ALWAYS by removing CPU_FTRS_GENERIC_32
powerpc/feature: Use CONFIG_PPC64 instead of __powerpc64__ to define
possible features
powerpc/processor: Move cpu_relax() into asm/vdso/processor.h
powerpc/time: Move timebase functions into new asm/timebase.h
powerpc/vdso: Prepare for switching VDSO to generic C implementation.
powerpc/vdso: Save and restore TOC pointer on PPC64
powerpc/vdso: Switch VDSO to generic C implementation.
powerpc/vdso: Provide __kernel_clock_gettime64() on vdso32
arch/powerpc/Kconfig | 2 +
arch/powerpc/include/asm/clocksource.h | 7 +
arch/powerpc/include/asm/cputable.h | 9 +-
arch/powerpc/include/asm/ppc_asm.h | 2 +
arch/powerpc/include/asm/processor.h | 13 +-
arch/powerpc/include/asm/time.h | 30 +-
arch/powerpc/include/asm/timebase.h | 42 +++
arch/powerpc/include/asm/vdso/clocksource.h | 7 +
arch/powerpc/include/asm/vdso/gettimeofday.h | 201 +++++++++++++
arch/powerpc/include/asm/vdso/processor.h | 23 ++
arch/powerpc/include/asm/vdso/vsyscall.h | 25 ++
arch/powerpc/include/asm/vdso_datapage.h | 40 +--
arch/powerpc/kernel/asm-offsets.c | 49 +--
arch/powerpc/kernel/time.c | 91 +-----
arch/powerpc/kernel/vdso.c | 5 +-
arch/powerpc/kernel/vdso32/Makefile | 26 +-
arch/powerpc/kernel/vdso32/gettimeofday.S | 300 +------------------
arch/powerpc/kernel/vdso32/vdso32.lds.S | 2 +
arch/powerpc/kernel/vdso32/vgettimeofday.c | 34 +++
arch/powerpc/kernel/vdso64/Makefile | 23 +-
arch/powerpc/kernel/vdso64/gettimeofday.S | 242 +--------------
arch/powerpc/kernel/vdso64/vdso64.lds.S | 2 +-
arch/powerpc/kernel/vdso64/vgettimeofday.c | 29 ++
23 files changed, 466 insertions(+), 738 deletions(-)
create mode 100644 arch/powerpc/include/asm/clocksource.h
create mode 100644 arch/powerpc/include/asm/timebase.h
create mode 100644 arch/powerpc/include/asm/vdso/clocksource.h
create mode 100644 arch/powerpc/include/asm/vdso/gettimeofday.h
create mode 100644 arch/powerpc/include/asm/vdso/processor.h
create mode 100644 arch/powerpc/include/asm/vdso/vsyscall.h
create mode 100644 arch/powerpc/kernel/vdso32/vgettimeofday.c
create mode 100644 arch/powerpc/kernel/vdso64/vgettimeofday.c
--
2.25.0
^ permalink raw reply
* [PATCH v13 1/8] powerpc/feature: Fix CPU_FTRS_ALWAYS by removing CPU_FTRS_GENERIC_32
From: Christophe Leroy @ 2020-11-03 18:07 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, nathanl,
anton
Cc: linux-arch, arnd, linux-kernel, luto, tglx, vincenzo.frascino,
linuxppc-dev
In-Reply-To: <cover.1604426550.git.christophe.leroy@csgroup.eu>
On 8xx, we get the following features:
[ 0.000000] cpu_features = 0x0000000000000100
[ 0.000000] possible = 0x0000000000000120
[ 0.000000] always = 0x0000000000000000
This is not correct. As CONFIG_PPC_8xx is mutually exclusive with all
other configurations, the three lines should be equal.
The problem is due to CPU_FTRS_GENERIC_32 which is taken when
CONFIG_BOOK3S_32 is NOT selected. This CPU_FTRS_GENERIC_32 is
pointless because there is no generic configuration supporting
all 32 bits but book3s/32.
Remove this pointless generic features definition to unbreak the
calculation of 'possible' features and 'always' features.
Fixes: 76bc080ef5a3 ("[POWERPC] Make default cputable entries reflect selected CPU family")
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/cputable.h | 5 -----
1 file changed, 5 deletions(-)
diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index 3d2f94afc13a..5e31960a56a9 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -409,7 +409,6 @@ static inline void cpu_feature_keys_init(void) { }
CPU_FTR_DBELL | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
CPU_FTR_DEBUG_LVL_EXC | CPU_FTR_EMB_HV | CPU_FTR_ALTIVEC_COMP | \
CPU_FTR_CELL_TB_BUG | CPU_FTR_SMT)
-#define CPU_FTRS_GENERIC_32 (CPU_FTR_COMMON | CPU_FTR_NODSISRALIGN)
/* 64-bit CPUs */
#define CPU_FTRS_PPC970 (CPU_FTR_LWSYNC | \
@@ -520,8 +519,6 @@ enum {
CPU_FTRS_7447 | CPU_FTRS_7447A | CPU_FTRS_82XX |
CPU_FTRS_G2_LE | CPU_FTRS_E300 | CPU_FTRS_E300C2 |
CPU_FTRS_CLASSIC32 |
-#else
- CPU_FTRS_GENERIC_32 |
#endif
#ifdef CONFIG_PPC_8xx
CPU_FTRS_8XX |
@@ -596,8 +593,6 @@ enum {
CPU_FTRS_7447 & CPU_FTRS_7447A & CPU_FTRS_82XX &
CPU_FTRS_G2_LE & CPU_FTRS_E300 & CPU_FTRS_E300C2 &
CPU_FTRS_CLASSIC32 &
-#else
- CPU_FTRS_GENERIC_32 &
#endif
#ifdef CONFIG_PPC_8xx
CPU_FTRS_8XX &
--
2.25.0
^ permalink raw reply related
* [PATCH v13 3/8] powerpc/processor: Move cpu_relax() into asm/vdso/processor.h
From: Christophe Leroy @ 2020-11-03 18:07 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, nathanl,
anton
Cc: linux-arch, arnd, linux-kernel, luto, tglx, vincenzo.frascino,
linuxppc-dev
In-Reply-To: <cover.1604426550.git.christophe.leroy@csgroup.eu>
cpu_relax() need to be in asm/vdso/processor.h to be used by
the C VDSO generic library.
Move it there.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v9: Forgot to remove cpu_relax() from processor.h in v8
---
arch/powerpc/include/asm/processor.h | 13 ++-----------
arch/powerpc/include/asm/vdso/processor.h | 23 +++++++++++++++++++++++
2 files changed, 25 insertions(+), 11 deletions(-)
create mode 100644 arch/powerpc/include/asm/vdso/processor.h
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index c61c859b51a8..333e3b6c76fb 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -6,6 +6,8 @@
* Copyright (C) 2001 PPC 64 Team, IBM Corp
*/
+#include <vdso/processor.h>
+
#include <asm/reg.h>
#ifdef CONFIG_VSX
@@ -63,14 +65,6 @@ extern int _chrp_type;
#endif /* defined(__KERNEL__) && defined(CONFIG_PPC32) */
-/* Macros for adjusting thread priority (hardware multi-threading) */
-#define HMT_very_low() asm volatile("or 31,31,31 # very low priority")
-#define HMT_low() asm volatile("or 1,1,1 # low priority")
-#define HMT_medium_low() asm volatile("or 6,6,6 # medium low priority")
-#define HMT_medium() asm volatile("or 2,2,2 # medium priority")
-#define HMT_medium_high() asm volatile("or 5,5,5 # medium high priority")
-#define HMT_high() asm volatile("or 3,3,3 # high priority")
-
#ifdef __KERNEL__
#ifdef CONFIG_PPC64
@@ -344,7 +338,6 @@ static inline unsigned long __pack_fe01(unsigned int fpmode)
}
#ifdef CONFIG_PPC64
-#define cpu_relax() do { HMT_low(); HMT_medium(); barrier(); } while (0)
#define spin_begin() HMT_low()
@@ -363,8 +356,6 @@ do { \
} \
} while (0)
-#else
-#define cpu_relax() barrier()
#endif
/* Check that a certain kernel stack pointer is valid in task_struct p */
diff --git a/arch/powerpc/include/asm/vdso/processor.h b/arch/powerpc/include/asm/vdso/processor.h
new file mode 100644
index 000000000000..39b9beace9ca
--- /dev/null
+++ b/arch/powerpc/include/asm/vdso/processor.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_VDSO_PROCESSOR_H
+#define __ASM_VDSO_PROCESSOR_H
+
+#ifndef __ASSEMBLY__
+
+/* Macros for adjusting thread priority (hardware multi-threading) */
+#define HMT_very_low() asm volatile("or 31, 31, 31 # very low priority")
+#define HMT_low() asm volatile("or 1, 1, 1 # low priority")
+#define HMT_medium_low() asm volatile("or 6, 6, 6 # medium low priority")
+#define HMT_medium() asm volatile("or 2, 2, 2 # medium priority")
+#define HMT_medium_high() asm volatile("or 5, 5, 5 # medium high priority")
+#define HMT_high() asm volatile("or 3, 3, 3 # high priority")
+
+#ifdef CONFIG_PPC64
+#define cpu_relax() do { HMT_low(); HMT_medium(); barrier(); } while (0)
+#else
+#define cpu_relax() barrier()
+#endif
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* __ASM_VDSO_PROCESSOR_H */
--
2.25.0
^ permalink raw reply related
* [PATCH v13 2/8] powerpc/feature: Use CONFIG_PPC64 instead of __powerpc64__ to define possible features
From: Christophe Leroy @ 2020-11-03 18:07 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, nathanl,
anton
Cc: linux-arch, arnd, linux-kernel, luto, tglx, vincenzo.frascino,
linuxppc-dev
In-Reply-To: <cover.1604426550.git.christophe.leroy@csgroup.eu>
In order to build VDSO32 for PPC64, we need to have CPU_FTRS_POSSIBLE
and CPU_FTRS_ALWAYS independant of whether we are building the
32 bits VDSO or the 64 bits VDSO.
Use #ifdef CONFIG_PPC64 instead of #ifdef __powerpc64__
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v13: new
---
arch/powerpc/include/asm/cputable.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index 5e31960a56a9..e069a2d9f7c1 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -488,7 +488,7 @@ static inline void cpu_feature_keys_init(void) { }
CPU_FTR_PURR | CPU_FTR_REAL_LE | CPU_FTR_DABRX)
#define CPU_FTRS_COMPATIBLE (CPU_FTR_PPCAS_ARCH_V2)
-#ifdef __powerpc64__
+#ifdef CONFIG_PPC64
#ifdef CONFIG_PPC_BOOK3E
#define CPU_FTRS_POSSIBLE (CPU_FTRS_E6500 | CPU_FTRS_E5500)
#else
@@ -545,7 +545,7 @@ enum {
};
#endif /* __powerpc64__ */
-#ifdef __powerpc64__
+#ifdef CONFIG_PPC64
#ifdef CONFIG_PPC_BOOK3E
#define CPU_FTRS_ALWAYS (CPU_FTRS_E6500 & CPU_FTRS_E5500)
#else
--
2.25.0
^ permalink raw reply related
* [PATCH] powerpc/vdso: Fix VDSO unmap check
From: Laurent Dufour @ 2020-11-03 17:13 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-kernel, Paul Mackerras, Thomas Gleixner
The check introduced by the commit 83d3f0e90c6c ("powerpc/mm: tracking vDSO
remap") is wrong and is missing some partial unmaps of the VDSO.
To be complete the check needs the base and end address of the
VDSO. Currently only the base is available in the mm_context of a task, but
the end address can easily be computed because the size of VDSO is
constant. However, there are 2 sizes for 32 or 64 bits task and they are
stored in static variables in arch/powerpc/kernel/vdso.c.
Exporting a new function called vdso_pages() to get the number of pages of
the VDSO based on the static variables from arch/powerpc/kernel/vdso.c.
Fixes: 83d3f0e90c6c ("powerpc/mm: tracking vDSO remap")
Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
Reported-by: Thomas Gleixner <tglx@linutronix.de>
Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
---
arch/powerpc/include/asm/mmu_context.h | 18 ++++++++++++++++--
arch/powerpc/kernel/vdso.c | 14 ++++++++++++++
2 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
index e02aa793420b..ced80897b7a1 100644
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -259,11 +259,25 @@ static inline void enter_lazy_tlb(struct mm_struct *mm,
extern void arch_exit_mmap(struct mm_struct *mm);
+extern int vdso_pages(void);
static inline void arch_unmap(struct mm_struct *mm,
unsigned long start, unsigned long end)
{
- if (start <= mm->context.vdso_base && mm->context.vdso_base < end)
- mm->context.vdso_base = 0;
+ unsigned long vdso_end;
+
+ if (mm->context.vdso_base) {
+ /*
+ * case 1 > | VDSO | <
+ * case 2 > | < |
+ * case 3 | > < |
+ * case 4 | > | <
+ */
+ vdso_end = mm->context.vdso_base;
+ vdso_end += vdso_pages() << PAGE_SHIFT;
+
+ if (start < vdso_end && mm->context.vdso_base < end)
+ mm->context.vdso_base = 0;
+ }
}
#ifdef CONFIG_PPC_MEM_KEYS
diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
index 8dad44262e75..9defa35a1eba 100644
--- a/arch/powerpc/kernel/vdso.c
+++ b/arch/powerpc/kernel/vdso.c
@@ -117,6 +117,20 @@ struct lib64_elfinfo
unsigned long text;
};
+/*
+ * Return the number of pages of the VDSO for the current task.
+ */
+int vdso_pages(void)
+{
+ int vdso_pages = vdso32_pages;
+
+#ifdef CONFIG_PPC64
+ if (!is_32bit_task())
+ vdso_pages = vdso64_pages;
+#endif
+
+ return vdso_pages + 1; /* Add the data page */
+}
/*
* This is called from binfmt_elf, we create the special vma for the
--
2.29.2
^ permalink raw reply related
* Re: [PATCH] x86/mpx: fix recursive munmap() corruption
From: Laurent Dufour @ 2020-11-03 17:11 UTC (permalink / raw)
To: Christophe Leroy, Michael Ellerman
Cc: mhocko, rguenther, linux-mm, Dave Hansen, x86, stable, LKML,
Dave Hansen, Thomas Gleixner, luto, linuxppc-dev, Andrew Morton,
vbabka
In-Reply-To: <12313ba8-75b5-d44d-dbc0-0bf2c87dfb59@csgroup.eu>
Le 23/10/2020 à 14:28, Christophe Leroy a écrit :
> Hi Laurent
>
> Le 07/05/2019 à 18:35, Laurent Dufour a écrit :
>> Le 01/05/2019 à 12:32, Michael Ellerman a écrit :
>>> Laurent Dufour <ldufour@linux.vnet.ibm.com> writes:
>>>> Le 23/04/2019 à 18:04, Dave Hansen a écrit :
>>>>> On 4/23/19 4:16 AM, Laurent Dufour wrote:
>>> ...
>>>>>> There are 2 assumptions here:
>>>>>> 1. 'start' and 'end' are page aligned (this is guaranteed by
>>>>>> __do_munmap().
>>>>>> 2. the VDSO is 1 page (this is guaranteed by the union vdso_data_store
>>>>>> on powerpc)
>>>>>
>>>>> Are you sure about #2? The 'vdso64_pages' variable seems rather
>>>>> unnecessary if the VDSO is only 1 page. ;)
>>>>
>>>> Hum, not so sure now ;)
>>>> I got confused, only the header is one page.
>>>> The test is working as a best effort, and don't cover the case where
>>>> only few pages inside the VDSO are unmmapped (start >
>>>> mm->context.vdso_base). This is not what CRIU is doing and so this was
>>>> enough for CRIU support.
>>>>
>>>> Michael, do you think there is a need to manage all the possibility
>>>> here, since the only user is CRIU and unmapping the VDSO is not a so
>>>> good idea for other processes ?
>>>
>>> Couldn't we implement the semantic that if any part of the VDSO is
>>> unmapped then vdso_base is set to zero? That should be fairly easy, eg:
>>>
>>> if (start < vdso_end && end >= mm->context.vdso_base)
>>> mm->context.vdso_base = 0;
>>>
>>>
>>> We might need to add vdso_end to the mm->context, but that should be OK.
>>>
>>> That seems like it would work for CRIU and make sense in general?
>>
>> Sorry for the late answer, yes this would make more sense.
>>
>> Here is a patch doing that.
>>
>
> In your patch, the test seems overkill:
>
> + if ((start <= vdso_base && vdso_end <= end) || /* 1 */
> + (vdso_base <= start && start < vdso_end) || /* 3,4 */
> + (vdso_base < end && end <= vdso_end)) /* 2,3 */
> + mm->context.vdso_base = mm->context.vdso_end = 0;
>
> What about
>
> if (start < vdso_end && vdso_start < end)
> mm->context.vdso_base = mm->context.vdso_end = 0;
>
> This should cover all cases, or am I missing something ?
>
>
> And do we really need to store vdso_end in the context ?
> I think it should be possible to re-calculate it: the size of the VDSO should be
> (&vdso32_end - &vdso32_start) + PAGE_SIZE for 32 bits VDSO, and (&vdso64_end -
> &vdso64_start) + PAGE_SIZE for the 64 bits VDSO.
Thanks Christophe for the advise.
That is covering all the cases, and indeed is similar to the Michael's proposal
I missed last year.
I'll send a patch fixing this issue following your proposal.
Cheers,
Laurent.
^ permalink raw reply
* Re: [PATCH] powerpc: Don't use asm goto for put_user() with GCC 4.9
From: Christophe Leroy @ 2020-11-03 17:04 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: schwab
In-Reply-To: <4fe837f8-ecae-f009-c193-8da386a70705@csgroup.eu>
Quoting Christophe Leroy <christophe.leroy@csgroup.eu>:
> Le 03/11/2020 à 14:29, Michael Ellerman a écrit :
>> Andreas reported that commit ee0a49a6870e ("powerpc/uaccess: Switch
>> __put_user_size_allowed() to __put_user_asm_goto()") broke
>> CLONE_CHILD_SETTID.
>>
>> Further inspection showed that the put_user() in schedule_tail() was
>> missing entirely, the store not emitted by the compiler.
>>
>
>>
>> Notice there are no stores other than to the stack. There should be a
>> stw in there for the store to current->set_child_tid.
>>
>> This is only seen with GCC 4.9 era compilers (tested with 4.9.3 and
>> 4.9.4), and only when CONFIG_PPC_KUAP is disabled.
>>
>> When CONFIG_PPC_KUAP=y, the memory clobber that's part of the isync()
>> and mtspr() inlined via allow_user_access() seems to be enough to
>> avoid the bug.
>>
>> For now though let's just not use asm goto with GCC 4.9, to avoid this
>> bug and any other issues we haven't noticed yet. Possibly in future we
>> can find a smaller workaround.
>
> Is that https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58670 ?
>
> Should we use asm_volatile_goto() defined in include/linux/compiler-gcc.h ?
It seems to be OK with asm_volatile_goto() with GCC 4.9, and it is
already what is used in our asm/jump_label.h
diff --git a/arch/powerpc/include/asm/uaccess.h
b/arch/powerpc/include/asm/uaccess.h
index ef5bbb705c08..501c9a79038c 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -178,7 +178,7 @@ do { \
* are no aliasing issues.
*/
#define __put_user_asm_goto(x, addr, label, op) \
- asm volatile goto( \
+ asm_volatile_goto( \
"1: " op "%U1%X1 %0,%1 # put_user\n" \
EX_TABLE(1b, %l2) \
: \
@@ -191,7 +191,7 @@ do { \
__put_user_asm_goto(x, ptr, label, "std")
#else /* __powerpc64__ */
#define __put_user_asm2_goto(x, addr, label) \
- asm volatile goto( \
+ asm_volatile_goto( \
"1: stw%X1 %0, %1\n" \
"2: stw%X1 %L0, %L1\n" \
EX_TABLE(1b, %l2) \
---
Christophe
^ permalink raw reply related
* [PATCH v4 4/4] arch, mm: make kernel_page_present() always available
From: Mike Rapoport @ 2020-11-03 16:20 UTC (permalink / raw)
To: Andrew Morton
Cc: David Hildenbrand, Peter Zijlstra, Dave Hansen, linux-mm,
Paul Mackerras, Pavel Machek, H. Peter Anvin, sparclinux,
Christoph Lameter, Will Deacon, linux-riscv, linux-s390, x86,
Mike Rapoport, Christian Borntraeger, Ingo Molnar,
Catalin Marinas, Len Brown, Albert Ou, Vasily Gorbik, linux-pm,
Heiko Carstens, David Rientjes, Borislav Petkov, Andy Lutomirski,
Paul Walmsley, Kirill A. Shutemov, Thomas Gleixner,
linux-arm-kernel, Rafael J. Wysocki, linux-kernel, Pekka Enberg,
Palmer Dabbelt, Kirill A . Shutemov, Joonsoo Kim,
Edgecombe, Rick P, linuxppc-dev, David S. Miller, Mike Rapoport
In-Reply-To: <20201103162057.22916-1-rppt@kernel.org>
From: Mike Rapoport <rppt@linux.ibm.com>
For architectures that enable ARCH_HAS_SET_MEMORY having the ability to
verify that a page is mapped in the kernel direct map can be useful
regardless of hibernation.
Add RISC-V implementation of kernel_page_present(), update its forward
declarations and stubs to be a part of set_memory API and remove ugly
ifdefery in inlcude/linux/mm.h around current declarations of
kernel_page_present().
Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
arch/arm64/include/asm/cacheflush.h | 1 +
arch/arm64/mm/pageattr.c | 4 +---
arch/riscv/include/asm/set_memory.h | 1 +
arch/riscv/mm/pageattr.c | 29 +++++++++++++++++++++++++++++
arch/x86/include/asm/set_memory.h | 1 +
arch/x86/mm/pat/set_memory.c | 4 +---
include/linux/mm.h | 7 -------
include/linux/set_memory.h | 5 +++++
8 files changed, 39 insertions(+), 13 deletions(-)
diff --git a/arch/arm64/include/asm/cacheflush.h b/arch/arm64/include/asm/cacheflush.h
index 9384fd8fc13c..45217f21f1fe 100644
--- a/arch/arm64/include/asm/cacheflush.h
+++ b/arch/arm64/include/asm/cacheflush.h
@@ -140,6 +140,7 @@ int set_memory_valid(unsigned long addr, int numpages, int enable);
int set_direct_map_invalid_noflush(struct page *page);
int set_direct_map_default_noflush(struct page *page);
+bool kernel_page_present(struct page *page);
#include <asm-generic/cacheflush.h>
diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
index 439325532be1..92eccaf595c8 100644
--- a/arch/arm64/mm/pageattr.c
+++ b/arch/arm64/mm/pageattr.c
@@ -186,8 +186,8 @@ void __kernel_map_pages(struct page *page, int numpages, int enable)
set_memory_valid((unsigned long)page_address(page), numpages, enable);
}
+#endif /* CONFIG_DEBUG_PAGEALLOC */
-#ifdef CONFIG_HIBERNATION
/*
* This function is used to determine if a linear map page has been marked as
* not-valid. Walk the page table and check the PTE_VALID bit. This is based
@@ -234,5 +234,3 @@ bool kernel_page_present(struct page *page)
ptep = pte_offset_kernel(pmdp, addr);
return pte_valid(READ_ONCE(*ptep));
}
-#endif /* CONFIG_HIBERNATION */
-#endif /* CONFIG_DEBUG_PAGEALLOC */
diff --git a/arch/riscv/include/asm/set_memory.h b/arch/riscv/include/asm/set_memory.h
index 4c5bae7ca01c..d690b08dff2a 100644
--- a/arch/riscv/include/asm/set_memory.h
+++ b/arch/riscv/include/asm/set_memory.h
@@ -24,6 +24,7 @@ static inline int set_memory_nx(unsigned long addr, int numpages) { return 0; }
int set_direct_map_invalid_noflush(struct page *page);
int set_direct_map_default_noflush(struct page *page);
+bool kernel_page_present(struct page *page);
#endif /* __ASSEMBLY__ */
diff --git a/arch/riscv/mm/pageattr.c b/arch/riscv/mm/pageattr.c
index 321b09d2e2ea..87ba5a68bbb8 100644
--- a/arch/riscv/mm/pageattr.c
+++ b/arch/riscv/mm/pageattr.c
@@ -198,3 +198,32 @@ void __kernel_map_pages(struct page *page, int numpages, int enable)
__pgprot(0), __pgprot(_PAGE_PRESENT));
}
#endif
+
+bool kernel_page_present(struct page *page)
+{
+ unsigned long addr = (unsigned long)page_address(page);
+ pgd_t *pgd;
+ pud_t *pud;
+ p4d_t *p4d;
+ pmd_t *pmd;
+ pte_t *pte;
+
+ pgd = pgd_offset_k(addr);
+ if (!pgd_present(*pgd))
+ return false;
+
+ p4d = p4d_offset(pgd, addr);
+ if (!p4d_present(*p4d))
+ return false;
+
+ pud = pud_offset(p4d, addr);
+ if (!pud_present(*pud))
+ return false;
+
+ pmd = pmd_offset(pud, addr);
+ if (!pmd_present(*pmd))
+ return false;
+
+ pte = pte_offset_kernel(pmd, addr);
+ return pte_present(*pte);
+}
diff --git a/arch/x86/include/asm/set_memory.h b/arch/x86/include/asm/set_memory.h
index 5948218f35c5..4352f08bfbb5 100644
--- a/arch/x86/include/asm/set_memory.h
+++ b/arch/x86/include/asm/set_memory.h
@@ -82,6 +82,7 @@ int set_pages_rw(struct page *page, int numpages);
int set_direct_map_invalid_noflush(struct page *page);
int set_direct_map_default_noflush(struct page *page);
+bool kernel_page_present(struct page *page);
extern int kernel_set_to_readonly;
diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index bc9be96b777f..16f878c26667 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -2226,8 +2226,8 @@ void __kernel_map_pages(struct page *page, int numpages, int enable)
arch_flush_lazy_mmu_mode();
}
+#endif /* CONFIG_DEBUG_PAGEALLOC */
-#ifdef CONFIG_HIBERNATION
bool kernel_page_present(struct page *page)
{
unsigned int level;
@@ -2239,8 +2239,6 @@ bool kernel_page_present(struct page *page)
pte = lookup_address((unsigned long)page_address(page), &level);
return (pte_val(*pte) & _PAGE_PRESENT);
}
-#endif /* CONFIG_HIBERNATION */
-#endif /* CONFIG_DEBUG_PAGEALLOC */
int __init kernel_map_pages_in_pgd(pgd_t *pgd, u64 pfn, unsigned long address,
unsigned numpages, unsigned long page_flags)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index ab0ef6bd351d..44b82f22e76a 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2937,16 +2937,9 @@ static inline void debug_pagealloc_map_pages(struct page *page,
if (debug_pagealloc_enabled_static())
__kernel_map_pages(page, numpages, enable);
}
-
-#ifdef CONFIG_HIBERNATION
-extern bool kernel_page_present(struct page *page);
-#endif /* CONFIG_HIBERNATION */
#else /* CONFIG_DEBUG_PAGEALLOC */
static inline void debug_pagealloc_map_pages(struct page *page,
int numpages, int enable) {}
-#ifdef CONFIG_HIBERNATION
-static inline bool kernel_page_present(struct page *page) { return true; }
-#endif /* CONFIG_HIBERNATION */
#endif /* CONFIG_DEBUG_PAGEALLOC */
#ifdef __HAVE_ARCH_GATE_AREA
diff --git a/include/linux/set_memory.h b/include/linux/set_memory.h
index 860e0f843c12..fe1aa4e54680 100644
--- a/include/linux/set_memory.h
+++ b/include/linux/set_memory.h
@@ -23,6 +23,11 @@ static inline int set_direct_map_default_noflush(struct page *page)
{
return 0;
}
+
+static inline bool kernel_page_present(struct page *page)
+{
+ return true;
+}
#endif
#ifndef set_mce_nospec
--
2.28.0
^ permalink raw reply related
* [PATCH v4 3/4] arch, mm: restore dependency of __kernel_map_pages() of DEBUG_PAGEALLOC
From: Mike Rapoport @ 2020-11-03 16:20 UTC (permalink / raw)
To: Andrew Morton
Cc: David Hildenbrand, Peter Zijlstra, Dave Hansen, linux-mm,
Paul Mackerras, Pavel Machek, H. Peter Anvin, sparclinux,
Christoph Lameter, Will Deacon, linux-riscv, linux-s390, x86,
Mike Rapoport, Christian Borntraeger, Ingo Molnar,
Catalin Marinas, Len Brown, Albert Ou, Vasily Gorbik, linux-pm,
Heiko Carstens, David Rientjes, Borislav Petkov, Andy Lutomirski,
Paul Walmsley, Kirill A. Shutemov, Thomas Gleixner,
linux-arm-kernel, Rafael J. Wysocki, linux-kernel, Pekka Enberg,
Palmer Dabbelt, Kirill A . Shutemov, Joonsoo Kim,
Edgecombe, Rick P, linuxppc-dev, David S. Miller, Mike Rapoport
In-Reply-To: <20201103162057.22916-1-rppt@kernel.org>
From: Mike Rapoport <rppt@linux.ibm.com>
The design of DEBUG_PAGEALLOC presumes that __kernel_map_pages() must never
fail. With this assumption is wouldn't be safe to allow general usage of
this function.
Moreover, some architectures that implement __kernel_map_pages() have this
function guarded by #ifdef DEBUG_PAGEALLOC and some refuse to map/unmap
pages when page allocation debugging is disabled at runtime.
As all the users of __kernel_map_pages() were converted to use
debug_pagealloc_map_pages() it is safe to make it available only when
DEBUG_PAGEALLOC is set.
Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
Acked-by: David Hildenbrand <david@redhat.com>
Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
arch/Kconfig | 3 +++
arch/arm64/Kconfig | 4 +---
arch/arm64/mm/pageattr.c | 8 ++++++--
arch/powerpc/Kconfig | 5 +----
arch/riscv/Kconfig | 4 +---
arch/riscv/include/asm/pgtable.h | 2 --
arch/riscv/mm/pageattr.c | 2 ++
arch/s390/Kconfig | 4 +---
arch/sparc/Kconfig | 4 +---
arch/x86/Kconfig | 4 +---
arch/x86/mm/pat/set_memory.c | 2 ++
include/linux/mm.h | 10 +++++++---
12 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index 56b6ccc0e32d..56d4752b6db6 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1028,6 +1028,9 @@ config HAVE_STATIC_CALL_INLINE
bool
depends on HAVE_STATIC_CALL
+config ARCH_SUPPORTS_DEBUG_PAGEALLOC
+ bool
+
source "kernel/gcov/Kconfig"
source "scripts/gcc-plugins/Kconfig"
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 1d466addb078..a932810cfd90 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -71,6 +71,7 @@ config ARM64
select ARCH_USE_QUEUED_RWLOCKS
select ARCH_USE_QUEUED_SPINLOCKS
select ARCH_USE_SYM_ANNOTATIONS
+ select ARCH_SUPPORTS_DEBUG_PAGEALLOC
select ARCH_SUPPORTS_MEMORY_FAILURE
select ARCH_SUPPORTS_SHADOW_CALL_STACK if CC_HAVE_SHADOW_CALL_STACK
select ARCH_SUPPORTS_ATOMIC_RMW
@@ -1025,9 +1026,6 @@ config HOLES_IN_ZONE
source "kernel/Kconfig.hz"
-config ARCH_SUPPORTS_DEBUG_PAGEALLOC
- def_bool y
-
config ARCH_SPARSEMEM_ENABLE
def_bool y
select SPARSEMEM_VMEMMAP_ENABLE
diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
index 1b94f5b82654..439325532be1 100644
--- a/arch/arm64/mm/pageattr.c
+++ b/arch/arm64/mm/pageattr.c
@@ -155,7 +155,7 @@ int set_direct_map_invalid_noflush(struct page *page)
.clear_mask = __pgprot(PTE_VALID),
};
- if (!rodata_full)
+ if (!debug_pagealloc_enabled() && !rodata_full)
return 0;
return apply_to_page_range(&init_mm,
@@ -170,7 +170,7 @@ int set_direct_map_default_noflush(struct page *page)
.clear_mask = __pgprot(PTE_RDONLY),
};
- if (!rodata_full)
+ if (!debug_pagealloc_enabled() && !rodata_full)
return 0;
return apply_to_page_range(&init_mm,
@@ -178,6 +178,7 @@ int set_direct_map_default_noflush(struct page *page)
PAGE_SIZE, change_page_range, &data);
}
+#ifdef CONFIG_DEBUG_PAGEALLOC
void __kernel_map_pages(struct page *page, int numpages, int enable)
{
if (!debug_pagealloc_enabled() && !rodata_full)
@@ -186,6 +187,7 @@ void __kernel_map_pages(struct page *page, int numpages, int enable)
set_memory_valid((unsigned long)page_address(page), numpages, enable);
}
+#ifdef CONFIG_HIBERNATION
/*
* This function is used to determine if a linear map page has been marked as
* not-valid. Walk the page table and check the PTE_VALID bit. This is based
@@ -232,3 +234,5 @@ bool kernel_page_present(struct page *page)
ptep = pte_offset_kernel(pmdp, addr);
return pte_valid(READ_ONCE(*ptep));
}
+#endif /* CONFIG_HIBERNATION */
+#endif /* CONFIG_DEBUG_PAGEALLOC */
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index e9f13fe08492..ad8a83f3ddca 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -146,6 +146,7 @@ config PPC
select ARCH_MIGHT_HAVE_PC_SERIO
select ARCH_OPTIONAL_KERNEL_RWX if ARCH_HAS_STRICT_KERNEL_RWX
select ARCH_SUPPORTS_ATOMIC_RMW
+ select ARCH_SUPPORTS_DEBUG_PAGEALLOC if PPC32 || PPC_BOOK3S_64
select ARCH_USE_BUILTIN_BSWAP
select ARCH_USE_CMPXCHG_LOCKREF if PPC64
select ARCH_USE_QUEUED_RWLOCKS if PPC_QUEUED_SPINLOCKS
@@ -355,10 +356,6 @@ config PPC_OF_PLATFORM_PCI
depends on PCI
depends on PPC64 # not supported on 32 bits yet
-config ARCH_SUPPORTS_DEBUG_PAGEALLOC
- depends on PPC32 || PPC_BOOK3S_64
- def_bool y
-
config ARCH_SUPPORTS_UPROBES
def_bool y
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 44377fd7860e..9283c6f9ae2a 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -14,6 +14,7 @@ config RISCV
def_bool y
select ARCH_CLOCKSOURCE_INIT
select ARCH_SUPPORTS_ATOMIC_RMW
+ select ARCH_SUPPORTS_DEBUG_PAGEALLOC if MMU
select ARCH_HAS_BINFMT_FLAT
select ARCH_HAS_DEBUG_VM_PGTABLE
select ARCH_HAS_DEBUG_VIRTUAL if MMU
@@ -153,9 +154,6 @@ config ARCH_SELECT_MEMORY_MODEL
config ARCH_WANT_GENERAL_HUGETLB
def_bool y
-config ARCH_SUPPORTS_DEBUG_PAGEALLOC
- def_bool y
-
config SYS_SUPPORTS_HUGETLBFS
depends on MMU
def_bool y
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 183f1f4b2ae6..41a72861987c 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -461,8 +461,6 @@ static inline int ptep_clear_flush_young(struct vm_area_struct *vma,
#define VMALLOC_START 0
#define VMALLOC_END TASK_SIZE
-static inline void __kernel_map_pages(struct page *page, int numpages, int enable) {}
-
#endif /* !CONFIG_MMU */
#define kern_addr_valid(addr) (1) /* FIXME */
diff --git a/arch/riscv/mm/pageattr.c b/arch/riscv/mm/pageattr.c
index 19fecb362d81..321b09d2e2ea 100644
--- a/arch/riscv/mm/pageattr.c
+++ b/arch/riscv/mm/pageattr.c
@@ -184,6 +184,7 @@ int set_direct_map_default_noflush(struct page *page)
return ret;
}
+#ifdef CONFIG_DEBUG_PAGEALLOC
void __kernel_map_pages(struct page *page, int numpages, int enable)
{
if (!debug_pagealloc_enabled())
@@ -196,3 +197,4 @@ void __kernel_map_pages(struct page *page, int numpages, int enable)
__set_memory((unsigned long)page_address(page), numpages,
__pgprot(0), __pgprot(_PAGE_PRESENT));
}
+#endif
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 4a2a12be04c9..991a850a6c0b 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -35,9 +35,6 @@ config GENERIC_LOCKBREAK
config PGSTE
def_bool y if KVM
-config ARCH_SUPPORTS_DEBUG_PAGEALLOC
- def_bool y
-
config AUDIT_ARCH
def_bool y
@@ -106,6 +103,7 @@ config S390
select ARCH_INLINE_WRITE_UNLOCK_IRQRESTORE
select ARCH_STACKWALK
select ARCH_SUPPORTS_ATOMIC_RMW
+ select ARCH_SUPPORTS_DEBUG_PAGEALLOC
select ARCH_SUPPORTS_NUMA_BALANCING
select ARCH_USE_BUILTIN_BSWAP
select ARCH_USE_CMPXCHG_LOCKREF
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index a6ca135442f9..2c729b8d097a 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -88,6 +88,7 @@ config SPARC64
select HAVE_C_RECORDMCOUNT
select HAVE_ARCH_AUDITSYSCALL
select ARCH_SUPPORTS_ATOMIC_RMW
+ select ARCH_SUPPORTS_DEBUG_PAGEALLOC
select HAVE_NMI
select HAVE_REGS_AND_STACK_ACCESS_API
select ARCH_USE_QUEUED_RWLOCKS
@@ -148,9 +149,6 @@ config GENERIC_ISA_DMA
bool
default y if SPARC32
-config ARCH_SUPPORTS_DEBUG_PAGEALLOC
- def_bool y if SPARC64
-
config PGTABLE_LEVELS
default 4 if 64BIT
default 3
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index f6946b81f74a..0db3fb1da70c 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -91,6 +91,7 @@ config X86
select ARCH_STACKWALK
select ARCH_SUPPORTS_ACPI
select ARCH_SUPPORTS_ATOMIC_RMW
+ select ARCH_SUPPORTS_DEBUG_PAGEALLOC
select ARCH_SUPPORTS_NUMA_BALANCING if X86_64
select ARCH_USE_BUILTIN_BSWAP
select ARCH_USE_QUEUED_RWLOCKS
@@ -329,9 +330,6 @@ config ZONE_DMA32
config AUDIT_ARCH
def_bool y if X86_64
-config ARCH_SUPPORTS_DEBUG_PAGEALLOC
- def_bool y
-
config KASAN_SHADOW_OFFSET
hex
depends on KASAN
diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index 40baa90e74f4..bc9be96b777f 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -2194,6 +2194,7 @@ int set_direct_map_default_noflush(struct page *page)
return __set_pages_p(page, 1);
}
+#ifdef CONFIG_DEBUG_PAGEALLOC
void __kernel_map_pages(struct page *page, int numpages, int enable)
{
if (PageHighMem(page))
@@ -2239,6 +2240,7 @@ bool kernel_page_present(struct page *page)
return (pte_val(*pte) & _PAGE_PRESENT);
}
#endif /* CONFIG_HIBERNATION */
+#endif /* CONFIG_DEBUG_PAGEALLOC */
int __init kernel_map_pages_in_pgd(pgd_t *pgd, u64 pfn, unsigned long address,
unsigned numpages, unsigned long page_flags)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 14e397f3752c..ab0ef6bd351d 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2924,7 +2924,11 @@ static inline bool debug_pagealloc_enabled_static(void)
return static_branch_unlikely(&_debug_pagealloc_enabled);
}
-#if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_ARCH_HAS_SET_DIRECT_MAP)
+#ifdef CONFIG_DEBUG_PAGEALLOC
+/*
+ * To support DEBUG_PAGEALLOC architecture must ensure that
+ * __kernel_map_pages() never fails
+ */
extern void __kernel_map_pages(struct page *page, int numpages, int enable);
static inline void debug_pagealloc_map_pages(struct page *page,
@@ -2937,13 +2941,13 @@ static inline void debug_pagealloc_map_pages(struct page *page,
#ifdef CONFIG_HIBERNATION
extern bool kernel_page_present(struct page *page);
#endif /* CONFIG_HIBERNATION */
-#else /* CONFIG_DEBUG_PAGEALLOC || CONFIG_ARCH_HAS_SET_DIRECT_MAP */
+#else /* CONFIG_DEBUG_PAGEALLOC */
static inline void debug_pagealloc_map_pages(struct page *page,
int numpages, int enable) {}
#ifdef CONFIG_HIBERNATION
static inline bool kernel_page_present(struct page *page) { return true; }
#endif /* CONFIG_HIBERNATION */
-#endif /* CONFIG_DEBUG_PAGEALLOC || CONFIG_ARCH_HAS_SET_DIRECT_MAP */
+#endif /* CONFIG_DEBUG_PAGEALLOC */
#ifdef __HAVE_ARCH_GATE_AREA
extern struct vm_area_struct *get_gate_vma(struct mm_struct *mm);
--
2.28.0
^ permalink raw reply related
* [PATCH v4 2/4] PM: hibernate: make direct map manipulations more explicit
From: Mike Rapoport @ 2020-11-03 16:20 UTC (permalink / raw)
To: Andrew Morton
Cc: Rafael J . Wysocki, David Hildenbrand, Peter Zijlstra,
Dave Hansen, linux-mm, Paul Mackerras, Pavel Machek,
H. Peter Anvin, sparclinux, Christoph Lameter, Will Deacon,
linux-riscv, linux-s390, x86, Mike Rapoport,
Christian Borntraeger, Ingo Molnar, Catalin Marinas, Len Brown,
Albert Ou, Vasily Gorbik, linux-pm, Heiko Carstens,
David Rientjes, Borislav Petkov, Andy Lutomirski, Paul Walmsley,
Kirill A. Shutemov, Thomas Gleixner, linux-arm-kernel,
Rafael J. Wysocki, linux-kernel, Pekka Enberg, Palmer Dabbelt,
Kirill A . Shutemov, Joonsoo Kim, Edgecombe, Rick P, linuxppc-dev,
David S. Miller, Mike Rapoport
In-Reply-To: <20201103162057.22916-1-rppt@kernel.org>
From: Mike Rapoport <rppt@linux.ibm.com>
When DEBUG_PAGEALLOC or ARCH_HAS_SET_DIRECT_MAP is enabled a page may be
not present in the direct map and has to be explicitly mapped before it
could be copied.
Introduce hibernate_map_page() that will explicitly use
set_direct_map_{default,invalid}_noflush() for ARCH_HAS_SET_DIRECT_MAP case
and debug_pagealloc_map_pages() for DEBUG_PAGEALLOC case.
The remapping of the pages in safe_copy_page() presumes that it only
changes protection bits in an existing PTE and so it is safe to ignore
return value of set_direct_map_{default,invalid}_noflush().
Still, add a pr_warn() so that future changes in set_memory APIs will not
silently break hibernation.
Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
include/linux/mm.h | 12 ------------
kernel/power/snapshot.c | 32 ++++++++++++++++++++++++++++++--
2 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 1fc0609056dc..14e397f3752c 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2927,16 +2927,6 @@ static inline bool debug_pagealloc_enabled_static(void)
#if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_ARCH_HAS_SET_DIRECT_MAP)
extern void __kernel_map_pages(struct page *page, int numpages, int enable);
-/*
- * When called in DEBUG_PAGEALLOC context, the call should most likely be
- * guarded by debug_pagealloc_enabled() or debug_pagealloc_enabled_static()
- */
-static inline void
-kernel_map_pages(struct page *page, int numpages, int enable)
-{
- __kernel_map_pages(page, numpages, enable);
-}
-
static inline void debug_pagealloc_map_pages(struct page *page,
int numpages, int enable)
{
@@ -2948,8 +2938,6 @@ static inline void debug_pagealloc_map_pages(struct page *page,
extern bool kernel_page_present(struct page *page);
#endif /* CONFIG_HIBERNATION */
#else /* CONFIG_DEBUG_PAGEALLOC || CONFIG_ARCH_HAS_SET_DIRECT_MAP */
-static inline void
-kernel_map_pages(struct page *page, int numpages, int enable) {}
static inline void debug_pagealloc_map_pages(struct page *page,
int numpages, int enable) {}
#ifdef CONFIG_HIBERNATION
diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index 46b1804c1ddf..57d54b9d84bb 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -76,6 +76,34 @@ static inline void hibernate_restore_protect_page(void *page_address) {}
static inline void hibernate_restore_unprotect_page(void *page_address) {}
#endif /* CONFIG_STRICT_KERNEL_RWX && CONFIG_ARCH_HAS_SET_MEMORY */
+static inline void hibernate_map_page(struct page *page, int enable)
+{
+ if (IS_ENABLED(CONFIG_ARCH_HAS_SET_DIRECT_MAP)) {
+ unsigned long addr = (unsigned long)page_address(page);
+ int ret;
+
+ /*
+ * This should not fail because remapping a page here means
+ * that we only update protection bits in an existing PTE.
+ * It is still worth to have a warning here if something
+ * changes and this will no longer be the case.
+ */
+ if (enable)
+ ret = set_direct_map_default_noflush(page);
+ else
+ ret = set_direct_map_invalid_noflush(page);
+
+ if (ret) {
+ pr_warn_once("Failed to remap page\n");
+ return;
+ }
+
+ flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
+ } else {
+ debug_pagealloc_map_pages(page, 1, enable);
+ }
+}
+
static int swsusp_page_is_free(struct page *);
static void swsusp_set_page_forbidden(struct page *);
static void swsusp_unset_page_forbidden(struct page *);
@@ -1355,9 +1383,9 @@ static void safe_copy_page(void *dst, struct page *s_page)
if (kernel_page_present(s_page)) {
do_copy_page(dst, page_address(s_page));
} else {
- kernel_map_pages(s_page, 1, 1);
+ hibernate_map_page(s_page, 1);
do_copy_page(dst, page_address(s_page));
- kernel_map_pages(s_page, 1, 0);
+ hibernate_map_page(s_page, 0);
}
}
--
2.28.0
^ permalink raw reply related
* [PATCH v4 1/4] mm: introduce debug_pagealloc_map_pages() helper
From: Mike Rapoport @ 2020-11-03 16:20 UTC (permalink / raw)
To: Andrew Morton
Cc: David Hildenbrand, Peter Zijlstra, Dave Hansen, linux-mm,
Paul Mackerras, Pavel Machek, H. Peter Anvin, sparclinux,
Christoph Lameter, Will Deacon, linux-riscv, linux-s390, x86,
Mike Rapoport, Christian Borntraeger, Ingo Molnar,
Catalin Marinas, Len Brown, Albert Ou, Vasily Gorbik, linux-pm,
Heiko Carstens, David Rientjes, Borislav Petkov, Andy Lutomirski,
Paul Walmsley, Kirill A. Shutemov, Thomas Gleixner,
linux-arm-kernel, Rafael J. Wysocki, linux-kernel, Pekka Enberg,
Palmer Dabbelt, Kirill A . Shutemov, Joonsoo Kim,
Edgecombe, Rick P, linuxppc-dev, David S. Miller, Mike Rapoport
In-Reply-To: <20201103162057.22916-1-rppt@kernel.org>
From: Mike Rapoport <rppt@linux.ibm.com>
When CONFIG_DEBUG_PAGEALLOC is enabled, it unmaps pages from the kernel
direct mapping after free_pages(). The pages than need to be mapped back
before they could be used. Theese mapping operations use
__kernel_map_pages() guarded with with debug_pagealloc_enabled().
The only place that calls __kernel_map_pages() without checking whether
DEBUG_PAGEALLOC is enabled is the hibernation code that presumes
availability of this function when ARCH_HAS_SET_DIRECT_MAP is set.
Still, on arm64, __kernel_map_pages() will bail out when DEBUG_PAGEALLOC is
not enabled but set_direct_map_invalid_noflush() may render some pages not
present in the direct map and hibernation code won't be able to save such
pages.
To make page allocation debugging and hibernation interaction more robust,
the dependency on DEBUG_PAGEALLOC or ARCH_HAS_SET_DIRECT_MAP has to be made
more explicit.
Start with combining the guard condition and the call to
__kernel_map_pages() into a single debug_pagealloc_map_pages() function to
emphasize that __kernel_map_pages() should not be called without
DEBUG_PAGEALLOC and use this new function to map/unmap pages when page
allocation debug is enabled.
Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
include/linux/mm.h | 10 ++++++++++
mm/memory_hotplug.c | 3 +--
mm/page_alloc.c | 6 ++----
mm/slab.c | 8 +++-----
4 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index ef360fe70aaf..1fc0609056dc 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2936,12 +2936,22 @@ kernel_map_pages(struct page *page, int numpages, int enable)
{
__kernel_map_pages(page, numpages, enable);
}
+
+static inline void debug_pagealloc_map_pages(struct page *page,
+ int numpages, int enable)
+{
+ if (debug_pagealloc_enabled_static())
+ __kernel_map_pages(page, numpages, enable);
+}
+
#ifdef CONFIG_HIBERNATION
extern bool kernel_page_present(struct page *page);
#endif /* CONFIG_HIBERNATION */
#else /* CONFIG_DEBUG_PAGEALLOC || CONFIG_ARCH_HAS_SET_DIRECT_MAP */
static inline void
kernel_map_pages(struct page *page, int numpages, int enable) {}
+static inline void debug_pagealloc_map_pages(struct page *page,
+ int numpages, int enable) {}
#ifdef CONFIG_HIBERNATION
static inline bool kernel_page_present(struct page *page) { return true; }
#endif /* CONFIG_HIBERNATION */
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index b44d4c7ba73b..e2b6043a4428 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -614,8 +614,7 @@ void generic_online_page(struct page *page, unsigned int order)
* so we should map it first. This is better than introducing a special
* case in page freeing fast path.
*/
- if (debug_pagealloc_enabled_static())
- kernel_map_pages(page, 1 << order, 1);
+ debug_pagealloc_map_pages(page, 1 << order, 1);
__free_pages_core(page, order);
totalram_pages_add(1UL << order);
#ifdef CONFIG_HIGHMEM
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 23f5066bd4a5..9a66a1ff9193 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1272,8 +1272,7 @@ static __always_inline bool free_pages_prepare(struct page *page,
*/
arch_free_page(page, order);
- if (debug_pagealloc_enabled_static())
- kernel_map_pages(page, 1 << order, 0);
+ debug_pagealloc_map_pages(page, 1 << order, 0);
kasan_free_nondeferred_pages(page, order);
@@ -2270,8 +2269,7 @@ inline void post_alloc_hook(struct page *page, unsigned int order,
set_page_refcounted(page);
arch_alloc_page(page, order);
- if (debug_pagealloc_enabled_static())
- kernel_map_pages(page, 1 << order, 1);
+ debug_pagealloc_map_pages(page, 1 << order, 1);
kasan_alloc_pages(page, order);
kernel_poison_pages(page, 1 << order, 1);
set_page_owner(page, order, gfp_flags);
diff --git a/mm/slab.c b/mm/slab.c
index b1113561b98b..340db0ce74c4 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1431,10 +1431,8 @@ static bool is_debug_pagealloc_cache(struct kmem_cache *cachep)
#ifdef CONFIG_DEBUG_PAGEALLOC
static void slab_kernel_map(struct kmem_cache *cachep, void *objp, int map)
{
- if (!is_debug_pagealloc_cache(cachep))
- return;
-
- kernel_map_pages(virt_to_page(objp), cachep->size / PAGE_SIZE, map);
+ debug_pagealloc_map_pages(virt_to_page(objp),
+ cachep->size / PAGE_SIZE, map);
}
#else
@@ -2062,7 +2060,7 @@ int __kmem_cache_create(struct kmem_cache *cachep, slab_flags_t flags)
#if DEBUG
/*
- * If we're going to use the generic kernel_map_pages()
+ * If we're going to use the generic debug_pagealloc_map_pages()
* poisoning, then it's going to smash the contents of
* the redzone and userword anyhow, so switch them off.
*/
--
2.28.0
^ permalink raw reply related
* [PATCH v4 0/4] arch, mm: improve robustness of direct map manipulation
From: Mike Rapoport @ 2020-11-03 16:20 UTC (permalink / raw)
To: Andrew Morton
Cc: David Hildenbrand, Peter Zijlstra, Dave Hansen, linux-mm,
Paul Mackerras, Pavel Machek, H. Peter Anvin, sparclinux,
Christoph Lameter, Will Deacon, linux-riscv, linux-s390, x86,
Mike Rapoport, Christian Borntraeger, Ingo Molnar,
Catalin Marinas, Len Brown, Albert Ou, Vasily Gorbik, linux-pm,
Heiko Carstens, David Rientjes, Borislav Petkov, Andy Lutomirski,
Paul Walmsley, Kirill A. Shutemov, Thomas Gleixner,
linux-arm-kernel, Rafael J. Wysocki, linux-kernel, Pekka Enberg,
Palmer Dabbelt, Kirill A . Shutemov, Joonsoo Kim,
Edgecombe, Rick P, linuxppc-dev, David S. Miller, Mike Rapoport
From: Mike Rapoport <rppt@linux.ibm.com>
Hi,
During recent discussion about KVM protected memory, David raised a concern
about usage of __kernel_map_pages() outside of DEBUG_PAGEALLOC scope [1].
Indeed, for architectures that define CONFIG_ARCH_HAS_SET_DIRECT_MAP it is
possible that __kernel_map_pages() would fail, but since this function is
void, the failure will go unnoticed.
Moreover, there's lack of consistency of __kernel_map_pages() semantics
across architectures as some guard this function with
#ifdef DEBUG_PAGEALLOC, some refuse to update the direct map if page
allocation debugging is disabled at run time and some allow modifying the
direct map regardless of DEBUG_PAGEALLOC settings.
This set straightens this out by restoring dependency of
__kernel_map_pages() on DEBUG_PAGEALLOC and updating the call sites
accordingly.
Since currently the only user of __kernel_map_pages() outside
DEBUG_PAGEALLOC is hibernation, it is updated to make direct map accesses
there more explicit.
[1] https://lore.kernel.org/lkml/2759b4bf-e1e3-d006-7d86-78a40348269d@redhat.com
v4 changes:
* s/WARN_ON/pr_warn_once/ per David and Kirill
* rebase on v5.10-rc2
* add Acked/Reviewed tags
v3 changes:
* update arm64 changes to avoid regression, per Rick's comments
* fix bisectability
https://lore.kernel.org/lkml/20201101170815.9795-1-rppt@kernel.org
v2 changes:
* Rephrase patch 2 changelog to better describe the change intentions and
implications
* Move removal of kernel_map_pages() from patch 1 to patch 2, per David
https://lore.kernel.org/lkml/20201029161902.19272-1-rppt@kernel.org
v1:
https://lore.kernel.org/lkml/20201025101555.3057-1-rppt@kernel.org
Mike Rapoport (4):
mm: introduce debug_pagealloc_map_pages() helper
PM: hibernate: make direct map manipulations more explicit
arch, mm: restore dependency of __kernel_map_pages() of DEBUG_PAGEALLOC
arch, mm: make kernel_page_present() always available
arch/Kconfig | 3 +++
arch/arm64/Kconfig | 4 +---
arch/arm64/include/asm/cacheflush.h | 1 +
arch/arm64/mm/pageattr.c | 6 +++--
arch/powerpc/Kconfig | 5 +----
arch/riscv/Kconfig | 4 +---
arch/riscv/include/asm/pgtable.h | 2 --
arch/riscv/include/asm/set_memory.h | 1 +
arch/riscv/mm/pageattr.c | 31 +++++++++++++++++++++++++
arch/s390/Kconfig | 4 +---
arch/sparc/Kconfig | 4 +---
arch/x86/Kconfig | 4 +---
arch/x86/include/asm/set_memory.h | 1 +
arch/x86/mm/pat/set_memory.c | 4 ++--
include/linux/mm.h | 35 +++++++++++++----------------
include/linux/set_memory.h | 5 +++++
kernel/power/snapshot.c | 30 +++++++++++++++++++++++--
mm/memory_hotplug.c | 3 +--
mm/page_alloc.c | 6 ++---
mm/slab.c | 8 +++----
20 files changed, 103 insertions(+), 58 deletions(-)
--
2.28.0
*** BLURB HERE ***
Mike Rapoport (4):
mm: introduce debug_pagealloc_map_pages() helper
PM: hibernate: make direct map manipulations more explicit
arch, mm: restore dependency of __kernel_map_pages() of
DEBUG_PAGEALLOC
arch, mm: make kernel_page_present() always available
arch/Kconfig | 3 +++
arch/arm64/Kconfig | 4 +---
arch/arm64/include/asm/cacheflush.h | 1 +
arch/arm64/mm/pageattr.c | 6 +++--
arch/powerpc/Kconfig | 5 +----
arch/riscv/Kconfig | 4 +---
arch/riscv/include/asm/pgtable.h | 2 --
arch/riscv/include/asm/set_memory.h | 1 +
arch/riscv/mm/pageattr.c | 31 +++++++++++++++++++++++++
arch/s390/Kconfig | 4 +---
arch/sparc/Kconfig | 4 +---
arch/x86/Kconfig | 4 +---
arch/x86/include/asm/set_memory.h | 1 +
arch/x86/mm/pat/set_memory.c | 4 ++--
include/linux/mm.h | 35 +++++++++++++----------------
include/linux/set_memory.h | 5 +++++
kernel/power/snapshot.c | 32 ++++++++++++++++++++++++--
mm/memory_hotplug.c | 3 +--
mm/page_alloc.c | 6 ++---
mm/slab.c | 8 +++----
20 files changed, 105 insertions(+), 58 deletions(-)
base-commit: 3cea11cd5e3b00d91caf0b4730194039b45c5891
--
2.28.0
^ permalink raw reply
* Re: [PATCH 11/11 v2.2] ftrace: Add recording of functions that caused recursion
From: Steven Rostedt @ 2020-11-03 16:14 UTC (permalink / raw)
To: Petr Mladek
Cc: Anton Vorontsov, linux-doc, Peter Zijlstra,
Sebastian Andrzej Siewior, Kamalesh Babulal, James E.J. Bottomley,
Guo Ren, H. Peter Anvin, live-patching, Miroslav Benes,
Ingo Molnar, linux-s390, Joe Lawrence, Jonathan Corbet,
Mauro Carvalho Chehab, Helge Deller, x86, linux-csky,
Christian Borntraeger, Kees Cook, Vasily Gorbik, Heiko Carstens,
Jiri Kosina, Borislav Petkov, Josh Poimboeuf, Thomas Gleixner,
Tony Luck, linux-parisc, linux-kernel, Masami Hiramatsu,
Colin Cross, Paul Mackerras, Andrew Morton, linuxppc-dev
In-Reply-To: <20201103141043.GO20201@alley>
On Tue, 3 Nov 2020 15:10:43 +0100
Petr Mladek <pmladek@suse.com> wrote:
> BTW: What is actually the purpose of paranoid_test, please?
>
> It prevents nested ftrace_record_recursion() calls on the same CPU
> (recursion, nesting from IRQ, NMI context).
>
> Parallel calls from different CPUs are still possible:
>
> CPU0 CPU1
> if (!atomic_read(¶noid_test)) if (!atomic_read(¶noid_test))
> // passes // passes
> atomic_inc(¶noid_test); atomic_inc(¶noid_test);
>
>
> I do not see how a nested call could cause crash while a parallel
> one would be OK.
>
Yeah, I should make that per cpu, but was lazy. ;-)
It was added at the end.
I'll update that to a per cpu, and local inc operations.
-- Steve
^ permalink raw reply
* Re: [PATCH v3 2/4] PM: hibernate: make direct map manipulations more explicit
From: Mike Rapoport @ 2020-11-03 15:56 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Rafael J . Wysocki, David Hildenbrand, Peter Zijlstra,
Dave Hansen, linux-mm, Paul Mackerras, Pavel Machek,
H. Peter Anvin, sparclinux, Christoph Lameter, Will Deacon,
linux-riscv, linux-s390, x86, Mike Rapoport,
Christian Borntraeger, Ingo Molnar, Catalin Marinas, Len Brown,
Albert Ou, Vasily Gorbik, linux-pm, Heiko Carstens,
David Rientjes, Borislav Petkov, Andy Lutomirski, Paul Walmsley,
Thomas Gleixner, Joonsoo Kim, linux-arm-kernel, Rafael J. Wysocki,
linux-kernel, Pekka Enberg, Palmer Dabbelt, Andrew Morton,
Edgecombe, Rick P, linuxppc-dev, David S. Miller
In-Reply-To: <20201103143916.otz2o4h2dlmewn3h@box>
On Tue, Nov 03, 2020 at 05:39:16PM +0300, Kirill A. Shutemov wrote:
> On Tue, Nov 03, 2020 at 02:13:50PM +0200, Mike Rapoport wrote:
> > > > +
> > > > + if (WARN_ON(ret))
> > >
> > > _ONCE?
> >
> > I've changed it to pr_warn() after David said people enable panic on
> > warn in production kernels.
>
> pr_warn_once()? :P
Sure :)
--
Sincerely yours,
Mike.
^ permalink raw reply
* [PATCH 25/25] soc: fsl: qbman: qman: Remove unused variable 'dequeue_wq'
From: Lee Jones @ 2020-11-03 15:28 UTC (permalink / raw)
To: lee.jones
Cc: linuxppc-dev, YueHaibing, linux-kernel, linux-arm-kernel, Li Yang
In-Reply-To: <20201103152838.1290217-1-lee.jones@linaro.org>
Fixes the following W=1 kernel build warning(s):
drivers/soc/fsl/qbman/qman.c: In function ‘qman_shutdown_fq’:
drivers/soc/fsl/qbman/qman.c:2700:8: warning: variable ‘dequeue_wq’ set but not used [-Wunused-but-set-variable]
Cc: Li Yang <leoyang.li@nxp.com>
Cc: YueHaibing <yuehaibing@huawei.com>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
drivers/soc/fsl/qbman/qman.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
index 9888a70618730..62b182c3a8b04 100644
--- a/drivers/soc/fsl/qbman/qman.c
+++ b/drivers/soc/fsl/qbman/qman.c
@@ -2622,7 +2622,7 @@ int qman_shutdown_fq(u32 fqid)
union qm_mc_command *mcc;
union qm_mc_result *mcr;
int orl_empty, drain = 0, ret = 0;
- u32 channel, wq, res;
+ u32 channel, res;
u8 state;
p = get_affine_portal();
@@ -2655,7 +2655,7 @@ int qman_shutdown_fq(u32 fqid)
DPAA_ASSERT((mcr->verb & QM_MCR_VERB_MASK) == QM_MCR_VERB_QUERYFQ);
/* Need to store these since the MCR gets reused */
channel = qm_fqd_get_chan(&mcr->queryfq.fqd);
- wq = qm_fqd_get_wq(&mcr->queryfq.fqd);
+ qm_fqd_get_wq(&mcr->queryfq.fqd);
if (channel < qm_channel_pool1) {
channel_portal = get_portal_for_channel(channel);
@@ -2697,7 +2697,6 @@ int qman_shutdown_fq(u32 fqid)
* to dequeue from the channel the FQ is scheduled on
*/
int found_fqrn = 0;
- u16 dequeue_wq = 0;
/* Flag that we need to drain FQ */
drain = 1;
@@ -2705,11 +2704,8 @@ int qman_shutdown_fq(u32 fqid)
if (channel >= qm_channel_pool1 &&
channel < qm_channel_pool1 + 15) {
/* Pool channel, enable the bit in the portal */
- dequeue_wq = (channel -
- qm_channel_pool1 + 1)<<4 | wq;
} else if (channel < qm_channel_pool1) {
/* Dedicated channel */
- dequeue_wq = wq;
} else {
dev_err(dev, "Can't recover FQ 0x%x, ch: 0x%x",
fqid, channel);
--
2.25.1
^ permalink raw reply related
* [PATCH 11/25] soc: fsl: qe: qe_common: Fix misnamed function attribute 'addr'
From: Lee Jones @ 2020-11-03 15:28 UTC (permalink / raw)
To: lee.jones
Cc: Software, Inc, linux-kernel, Li Yang, act, Dan Malek,
Vitaly Bordug, Scott Wood, linuxppc-dev, linux-arm-kernel,
Qiang Zhao
In-Reply-To: <20201103152838.1290217-1-lee.jones@linaro.org>
Fixes the following W=1 kernel build warning(s):
drivers/soc/fsl/qe/qe_common.c:237: warning: Function parameter or member 'addr' not described in 'cpm_muram_dma'
drivers/soc/fsl/qe/qe_common.c:237: warning: Excess function parameter 'offset' description in 'cpm_muram_dma'
Cc: Qiang Zhao <qiang.zhao@nxp.com>
Cc: Li Yang <leoyang.li@nxp.com>
Cc: Scott Wood <scottwood@freescale.com>
Cc: act <dmalek@jlc.net>
Cc: Dan Malek <dan@embeddedalley.com>
Cc: "Software, Inc" <source@mvista.com>
Cc: Vitaly Bordug <vbordug@ru.mvista.com>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
drivers/soc/fsl/qe/qe_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/fsl/qe/qe_common.c b/drivers/soc/fsl/qe/qe_common.c
index 75075591f6308..497a7e0fd0272 100644
--- a/drivers/soc/fsl/qe/qe_common.c
+++ b/drivers/soc/fsl/qe/qe_common.c
@@ -231,7 +231,7 @@ EXPORT_SYMBOL(cpm_muram_offset);
/**
* cpm_muram_dma - turn a muram virtual address into a DMA address
- * @offset: virtual address from cpm_muram_addr() to convert
+ * @addr: virtual address from cpm_muram_addr() to convert
*/
dma_addr_t cpm_muram_dma(void __iomem *addr)
{
--
2.25.1
^ permalink raw reply related
* [PATCH 04/25] soc: fsl: dpio: qbman-portal: Fix a bunch of kernel-doc misdemeanours
From: Lee Jones @ 2020-11-03 15:28 UTC (permalink / raw)
To: lee.jones
Cc: Roy Pledge, linuxppc-dev, linux-kernel, linux-arm-kernel, Li Yang
In-Reply-To: <20201103152838.1290217-1-lee.jones@linaro.org>
Fixes the following W=1 kernel build warning(s):
drivers/soc/fsl/dpio/qbman-portal.c:430: warning: Function parameter or member 'inhibit' not described in 'qbman_swp_interrupt_set_inhibit'
drivers/soc/fsl/dpio/qbman-portal.c:430: warning: Excess function parameter 'mask' description in 'qbman_swp_interrupt_set_inhibit'
drivers/soc/fsl/dpio/qbman-portal.c:518: warning: Function parameter or member 'd' not described in 'qbman_eq_desc_clear'
drivers/soc/fsl/dpio/qbman-portal.c:529: warning: Function parameter or member 'respond_success' not described in 'qbman_eq_desc_set_no_orp'
drivers/soc/fsl/dpio/qbman-portal.c:529: warning: Excess function parameter 'response_success' description in 'qbman_eq_desc_set_no_orp'
drivers/soc/fsl/dpio/qbman-portal.c:941: warning: Function parameter or member 's' not described in 'qbman_swp_push_get'
drivers/soc/fsl/dpio/qbman-portal.c:941: warning: Excess function parameter 'p' description in 'qbman_swp_push_get'
drivers/soc/fsl/dpio/qbman-portal.c:955: warning: Function parameter or member 's' not described in 'qbman_swp_push_set'
drivers/soc/fsl/dpio/qbman-portal.c:955: warning: Excess function parameter 'p' description in 'qbman_swp_push_set'
drivers/soc/fsl/dpio/qbman-portal.c:1052: warning: Function parameter or member 'd' not described in 'qbman_pull_desc_set_fq'
drivers/soc/fsl/dpio/qbman-portal.c:1065: warning: Function parameter or member 'd' not described in 'qbman_pull_desc_set_wq'
drivers/soc/fsl/dpio/qbman-portal.c:1079: warning: Function parameter or member 'd' not described in 'qbman_pull_desc_set_channel'
drivers/soc/fsl/dpio/qbman-portal.c:1403: warning: Function parameter or member 'd' not described in 'qbman_release_desc_clear'
drivers/soc/fsl/dpio/qbman-portal.c:1412: warning: Function parameter or member 'd' not described in 'qbman_release_desc_set_bpid'
drivers/soc/fsl/dpio/qbman-portal.c:1412: warning: Function parameter or member 'bpid' not described in 'qbman_release_desc_set_bpid'
drivers/soc/fsl/dpio/qbman-portal.c:1421: warning: Function parameter or member 'd' not described in 'qbman_release_desc_set_rcdi'
drivers/soc/fsl/dpio/qbman-portal.c:1421: warning: Function parameter or member 'enable' not described in 'qbman_release_desc_set_rcdi'
Cc: Roy Pledge <Roy.Pledge@nxp.com>
Cc: Li Yang <leoyang.li@nxp.com>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
drivers/soc/fsl/dpio/qbman-portal.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/soc/fsl/dpio/qbman-portal.c b/drivers/soc/fsl/dpio/qbman-portal.c
index 659b4a570d5b5..f13da4d7d1c52 100644
--- a/drivers/soc/fsl/dpio/qbman-portal.c
+++ b/drivers/soc/fsl/dpio/qbman-portal.c
@@ -424,7 +424,7 @@ int qbman_swp_interrupt_get_inhibit(struct qbman_swp *p)
/**
* qbman_swp_interrupt_set_inhibit() - write interrupt mask register
* @p: the given software portal object
- * @mask: The mask to set in SWP_IIR register
+ * @inhibit: whether to inhibit the IRQs
*/
void qbman_swp_interrupt_set_inhibit(struct qbman_swp *p, int inhibit)
{
@@ -510,7 +510,7 @@ enum qb_enqueue_commands {
#define QB_ENQUEUE_CMD_TARGET_TYPE_SHIFT 4
#define QB_ENQUEUE_CMD_DCA_EN_SHIFT 7
-/**
+/*
* qbman_eq_desc_clear() - Clear the contents of a descriptor to
* default/starting state.
*/
@@ -522,7 +522,7 @@ void qbman_eq_desc_clear(struct qbman_eq_desc *d)
/**
* qbman_eq_desc_set_no_orp() - Set enqueue descriptor without orp
* @d: the enqueue descriptor.
- * @response_success: 1 = enqueue with response always; 0 = enqueue with
+ * @respond_success: 1 = enqueue with response always; 0 = enqueue with
* rejections returned on a FQ.
*/
void qbman_eq_desc_set_no_orp(struct qbman_eq_desc *d, int respond_success)
@@ -932,7 +932,7 @@ int qbman_swp_enqueue_multiple_desc_mem_back(struct qbman_swp *s,
/**
* qbman_swp_push_get() - Get the push dequeue setup
- * @p: the software portal object
+ * @s: the software portal object
* @channel_idx: the channel index to query
* @enabled: returned boolean to show whether the push dequeue is enabled
* for the given channel
@@ -947,7 +947,7 @@ void qbman_swp_push_get(struct qbman_swp *s, u8 channel_idx, int *enabled)
/**
* qbman_swp_push_set() - Enable or disable push dequeue
- * @p: the software portal object
+ * @s: the software portal object
* @channel_idx: the channel index (0 to 15)
* @enable: enable or disable push dequeue
*/
@@ -1046,6 +1046,7 @@ void qbman_pull_desc_set_numframes(struct qbman_pull_desc *d, u8 numframes)
/**
* qbman_pull_desc_set_fq() - Set fqid from which the dequeue command dequeues
+ * @d: the pull dequeue descriptor to be set
* @fqid: the frame queue index of the given FQ
*/
void qbman_pull_desc_set_fq(struct qbman_pull_desc *d, u32 fqid)
@@ -1057,6 +1058,7 @@ void qbman_pull_desc_set_fq(struct qbman_pull_desc *d, u32 fqid)
/**
* qbman_pull_desc_set_wq() - Set wqid from which the dequeue command dequeues
+ * @d: the pull dequeue descriptor to be set
* @wqid: composed of channel id and wqid within the channel
* @dct: the dequeue command type
*/
@@ -1071,6 +1073,7 @@ void qbman_pull_desc_set_wq(struct qbman_pull_desc *d, u32 wqid,
/**
* qbman_pull_desc_set_channel() - Set channelid from which the dequeue command
* dequeues
+ * @d: the pull dequeue descriptor to be set
* @chid: the channel id to be dequeued
* @dct: the dequeue command type
*/
@@ -1398,6 +1401,7 @@ int qbman_result_has_new_result(struct qbman_swp *s, const struct dpaa2_dq *dq)
/**
* qbman_release_desc_clear() - Clear the contents of a descriptor to
* default/starting state.
+ * @d: the pull dequeue descriptor to be cleared
*/
void qbman_release_desc_clear(struct qbman_release_desc *d)
{
@@ -1407,6 +1411,8 @@ void qbman_release_desc_clear(struct qbman_release_desc *d)
/**
* qbman_release_desc_set_bpid() - Set the ID of the buffer pool to release to
+ * @d: the pull dequeue descriptor to be set
+ * @bpid: the bpid value to be set
*/
void qbman_release_desc_set_bpid(struct qbman_release_desc *d, u16 bpid)
{
@@ -1416,6 +1422,8 @@ void qbman_release_desc_set_bpid(struct qbman_release_desc *d, u16 bpid)
/**
* qbman_release_desc_set_rcdi() - Determines whether or not the portal's RCDI
* interrupt source should be asserted after the release command is completed.
+ * @d: the pull dequeue descriptor to be set
+ * @enable: enable (1) or disable (0) value
*/
void qbman_release_desc_set_rcdi(struct qbman_release_desc *d, int enable)
{
--
2.25.1
^ permalink raw reply related
* [PATCH 00/25] Rid W=1 warnings in SoC
From: Lee Jones @ 2020-11-03 15:28 UTC (permalink / raw)
To: lee.jones
Cc: Heiko Stuebner, Roy Pledge, Liam Girdwood, Scott Wood,
Thierry Reding, Li Yang, Qiang Zhao, linux-samsung-soc,
Rafael J. Wysocki, YueHaibing, Sandeep Nair, Krzysztof Kozlowski,
Jonathan Hunter, linux-rockchip, act, Andy Gross,
bcm-kernel-feedback-list, Cyril Chemparathy, linux-arm-msm,
Florian Fainelli, Santosh Shilimkar, linux-tegra, Bjorn Andersson,
linux-arm-kernel, Software, Inc, Dave Gerlach, Doug Anderson,
linux-kernel, Ben Dooks, Mark Brown, Dan Malek, Vitaly Bordug,
linuxppc-dev
This set is part of a larger effort attempting to clean-up W=1
kernel builds, which are currently overwhelmingly riddled with
niggly little warnings.
Lee Jones (25):
soc: bcm: brcmstb: pm: pm-arm: Provide prototype for
brcmstb_pm_s3_finish()
soc: qcom: qcom_aoss: Remove set but unused variable 'tlen'
soc: qcom: qcom_aoss: Add missing description for 'cooling_devs'
soc: fsl: dpio: qbman-portal: Fix a bunch of kernel-doc misdemeanours
soc: rockchip: io-domain: Remove incorrect and incomplete comment
header
soc: ti: knav_qmss_queue: Remove set but unchecked variable 'ret'
soc: ti: knav_qmss_queue: Fix a whole host of function documentation
issues
soc: ti: knav_dma: Fix a kernel function doc formatting issue
soc: ti: pm33xx: Remove set but unused variable 'ret'
soc: ti: wkup_m3_ipc: Document 'm3_ipc' parameter throughout
soc: fsl: qe: qe_common: Fix misnamed function attribute 'addr'
soc: qcom: qcom-geni-se: Fix misnamed function parameter 'rx_rfr'
soc: tegra: fuse: speedo-tegra124: Remove some set but unused
variables
soc: samsung: s3c-pm-check: Fix incorrectly named variable 'val'
soc: qcom: rpmh: Fix possible doc-rot in rpmh_write()'s header
soc: qcom: smem: Fix formatting and missing documentation issues
soc: qcom: smsm: Fix some kernel-doc formatting and naming problems
soc: qcom: wcnss_ctrl: Demote non-conformant struct header and fix
function headers
soc: qcom: smp2p: Remove unused struct attribute provide another
soc: qcom: llcc-qcom: Fix expected kernel-doc formatting
soc: qcom: rpmhpd: Provide some missing struct member descriptions
soc: qcom: kryo-l2-accessors: Fix misnaming of 'val'
soc: ti: k3-ringacc: Provide documentation for 'k3_ring's 'state'
soc: tegra: fuse: speedo-tegra210: Remove a group of set but unused
variables
soc: fsl: qbman: qman: Remove unused variable 'dequeue_wq'
drivers/soc/bcm/brcmstb/pm/pm-arm.c | 2 +
drivers/soc/fsl/dpio/qbman-portal.c | 18 +++++--
drivers/soc/fsl/qbman/qman.c | 8 +--
drivers/soc/fsl/qe/qe_common.c | 2 +-
drivers/soc/qcom/kryo-l2-accessors.c | 2 +-
drivers/soc/qcom/llcc-qcom.c | 2 +-
drivers/soc/qcom/qcom-geni-se.c | 5 +-
drivers/soc/qcom/qcom_aoss.c | 4 +-
drivers/soc/qcom/rpmh.c | 2 +-
drivers/soc/qcom/rpmhpd.c | 3 ++
drivers/soc/qcom/smem.c | 3 +-
drivers/soc/qcom/smp2p.c | 3 +-
drivers/soc/qcom/smsm.c | 4 +-
drivers/soc/qcom/wcnss_ctrl.c | 8 +--
drivers/soc/rockchip/io-domain.c | 3 --
drivers/soc/samsung/s3c-pm-check.c | 2 +-
drivers/soc/tegra/fuse/speedo-tegra124.c | 7 ++-
drivers/soc/tegra/fuse/speedo-tegra210.c | 8 +--
drivers/soc/ti/k3-ringacc.c | 1 +
drivers/soc/ti/knav_dma.c | 2 +-
drivers/soc/ti/knav_qmss_queue.c | 62 ++++++++++++------------
drivers/soc/ti/pm33xx.c | 4 +-
drivers/soc/ti/wkup_m3_ipc.c | 8 ++-
23 files changed, 86 insertions(+), 77 deletions(-)
Cc: act <dmalek@jlc.net>
Cc: Andy Gross <agross@kernel.org>
Cc: bcm-kernel-feedback-list@broadcom.com
Cc: Ben Dooks <ben@simtec.co.uk>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Cyril Chemparathy <cyril@ti.com>
Cc: Dan Malek <dan@embeddedalley.com>
Cc: Dave Gerlach <d-gerlach@ti.com>
Cc: Doug Anderson <dianders@chromium.org>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Jonathan Hunter <jonathanh@nvidia.com>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: linux-arm-msm@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-rockchip@lists.infradead.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: linux-tegra@vger.kernel.org
Cc: Li Yang <leoyang.li@nxp.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Qiang Zhao <qiang.zhao@nxp.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Roy Pledge <Roy.Pledge@nxp.com>
Cc: Sandeep Nair <sandeep_n@ti.com>
Cc: Santosh Shilimkar <ssantosh@kernel.org>
Cc: Scott Wood <scottwood@freescale.com>
Cc: "Software, Inc" <source@mvista.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Vitaly Bordug <vbordug@ru.mvista.com>
Cc: YueHaibing <yuehaibing@huawei.com>
--
2.25.1
^ permalink raw reply
* Re: [PATCH 30/33] docs: ABI: cleanup several ABI documents
From: Bjorn Andersson @ 2020-11-03 15:24 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: linux-stm32, Linux Doc Mailing List, linux-iio, netdev, coresight,
linux-pm, linux-remoteproc, linux-kernel, dri-devel,
linux-f2fs-devel, linux-acpi, linux-gpio, linux-i3c, linuxppc-dev,
linux-fpga, linux-arm-kernel, linux-media
In-Reply-To: <95ef2cf3a58f4e50f17d9e58e0d9440ad14d0427.1603893146.git.mchehab+huawei@kernel.org>
On Wed 28 Oct 09:23 CDT 2020, Mauro Carvalho Chehab wrote:
[..]
> .../ABI/testing/sysfs-class-remoteproc | 14 +-
for this:
Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Thanks,
Bjorn
^ permalink raw reply
* Re: [PATCH] powerpc: Don't use asm goto for put_user() with GCC 4.9
From: Christophe Leroy @ 2020-11-03 14:43 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: schwab
In-Reply-To: <20201103132915.529337-1-mpe@ellerman.id.au>
Le 03/11/2020 à 14:29, Michael Ellerman a écrit :
> Andreas reported that commit ee0a49a6870e ("powerpc/uaccess: Switch
> __put_user_size_allowed() to __put_user_asm_goto()") broke
> CLONE_CHILD_SETTID.
>
> Further inspection showed that the put_user() in schedule_tail() was
> missing entirely, the store not emitted by the compiler.
>
>
> Notice there are no stores other than to the stack. There should be a
> stw in there for the store to current->set_child_tid.
>
> This is only seen with GCC 4.9 era compilers (tested with 4.9.3 and
> 4.9.4), and only when CONFIG_PPC_KUAP is disabled.
>
> When CONFIG_PPC_KUAP=y, the memory clobber that's part of the isync()
> and mtspr() inlined via allow_user_access() seems to be enough to
> avoid the bug.
>
> For now though let's just not use asm goto with GCC 4.9, to avoid this
> bug and any other issues we haven't noticed yet. Possibly in future we
> can find a smaller workaround.
Is that https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58670 ?
Should we use asm_volatile_goto() defined in include/linux/compiler-gcc.h ?
Christophe
>
> This is basically a revert of commit ee0a49a6870e ("powerpc/uaccess:
> Switch __put_user_size_allowed() to __put_user_asm_goto()") and commit
> 7fdf966bed15 ("powerpc/uaccess: Remove __put_user_asm() and
> __put_user_asm2()"), but only for GCC 4.9.
>
> With this applied the code generation looks more like it will work:
>
>
> Fixes: ee0a49a6870e ("powerpc/uaccess: Switch __put_user_size_allowed() to __put_user_asm_goto()")
> Reported-by: Andreas Schwab <schwab@linux-m68k.org>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> arch/powerpc/include/asm/uaccess.h | 48 ++++++++++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> index ef5bbb705c08..526a6658946b 100644
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -110,6 +110,52 @@ static inline bool __access_ok(unsigned long addr, unsigned long size)
>
> extern long __put_user_bad(void);
>
> +#if defined(GCC_VERSION) && GCC_VERSION < 50000
> +#define __put_user_asm(x, addr, err, op) \
> + __asm__ __volatile__( \
> + "1: " op "%U2%X2 %1,%2 # put_user\n" \
> + "2:\n" \
> + ".section .fixup,\"ax\"\n" \
> + "3: li %0,%3\n" \
> + " b 2b\n" \
> + ".previous\n" \
> + EX_TABLE(1b, 3b) \
> + : "=r" (err) \
> + : "r" (x), "m<>" (*addr), "i" (-EFAULT), "0" (err))
> +
> +#ifdef __powerpc64__
> +#define __put_user_asm2(x, ptr, retval) \
> + __put_user_asm(x, ptr, retval, "std")
> +#else /* __powerpc64__ */
> +#define __put_user_asm2(x, addr, err) \
> + __asm__ __volatile__( \
> + "1: stw%X2 %1,%2\n" \
> + "2: stw%X2 %L1,%L2\n" \
> + "3:\n" \
> + ".section .fixup,\"ax\"\n" \
> + "4: li %0,%3\n" \
> + " b 3b\n" \
> + ".previous\n" \
> + EX_TABLE(1b, 4b) \
> + EX_TABLE(2b, 4b) \
> + : "=r" (err) \
> + : "r" (x), "m" (*addr), "i" (-EFAULT), "0" (err))
> +#endif /* __powerpc64__ */
> +
> +#define __put_user_size_allowed(x, ptr, size, retval) \
> +do { \
> + retval = 0; \
> + switch (size) { \
> + case 1: __put_user_asm(x, ptr, retval, "stb"); break; \
> + case 2: __put_user_asm(x, ptr, retval, "sth"); break; \
> + case 4: __put_user_asm(x, ptr, retval, "stw"); break; \
> + case 8: __put_user_asm2(x, ptr, retval); break; \
> + default: __put_user_bad(); \
> + } \
> +} while (0)
> +
> +#else
> +
> #define __put_user_size_allowed(x, ptr, size, retval) \
> do { \
> __label__ __pu_failed; \
> @@ -122,6 +168,8 @@ __pu_failed: \
> retval = -EFAULT; \
> } while (0)
>
> +#endif /* GCC_VERSION */
> +
> #define __put_user_size(x, ptr, size, retval) \
> do { \
> allow_write_to_user(ptr, size); \
>
^ permalink raw reply
* Re: [PATCH v3 2/4] PM: hibernate: make direct map manipulations more explicit
From: Kirill A. Shutemov @ 2020-11-03 14:39 UTC (permalink / raw)
To: Mike Rapoport
Cc: Rafael J . Wysocki, David Hildenbrand, Peter Zijlstra,
Dave Hansen, linux-mm, Paul Mackerras, Pavel Machek,
H. Peter Anvin, sparclinux, Christoph Lameter, Will Deacon,
linux-riscv, linux-s390, x86, Mike Rapoport,
Christian Borntraeger, Ingo Molnar, Catalin Marinas, Len Brown,
Albert Ou, Vasily Gorbik, linux-pm, Heiko Carstens,
David Rientjes, Borislav Petkov, Andy Lutomirski, Paul Walmsley,
Thomas Gleixner, Joonsoo Kim, linux-arm-kernel, Rafael J. Wysocki,
linux-kernel, Pekka Enberg, Palmer Dabbelt, Andrew Morton,
Edgecombe, Rick P, linuxppc-dev, David S. Miller
In-Reply-To: <20201103121350.GI4879@kernel.org>
On Tue, Nov 03, 2020 at 02:13:50PM +0200, Mike Rapoport wrote:
> On Tue, Nov 03, 2020 at 02:08:16PM +0300, Kirill A. Shutemov wrote:
> > On Sun, Nov 01, 2020 at 07:08:13PM +0200, Mike Rapoport wrote:
> > > diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
> > > index 46b1804c1ddf..054c8cce4236 100644
> > > --- a/kernel/power/snapshot.c
> > > +++ b/kernel/power/snapshot.c
> > > @@ -76,6 +76,32 @@ static inline void hibernate_restore_protect_page(void *page_address) {}
> > > static inline void hibernate_restore_unprotect_page(void *page_address) {}
> > > #endif /* CONFIG_STRICT_KERNEL_RWX && CONFIG_ARCH_HAS_SET_MEMORY */
> > >
> > > +static inline void hibernate_map_page(struct page *page, int enable)
> > > +{
> > > + if (IS_ENABLED(CONFIG_ARCH_HAS_SET_DIRECT_MAP)) {
> > > + unsigned long addr = (unsigned long)page_address(page);
> > > + int ret;
> > > +
> > > + /*
> > > + * This should not fail because remapping a page here means
> > > + * that we only update protection bits in an existing PTE.
> > > + * It is still worth to have WARN_ON() here if something
> > > + * changes and this will no longer be the case.
> > > + */
> > > + if (enable)
> > > + ret = set_direct_map_default_noflush(page);
> > > + else
> > > + ret = set_direct_map_invalid_noflush(page);
> > > +
> > > + if (WARN_ON(ret))
> >
> > _ONCE?
>
> I've changed it to pr_warn() after David said people enable panic on
> warn in production kernels.
pr_warn_once()? :P
--
Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH 11/11 v2.2] ftrace: Add recording of functions that caused recursion
From: Petr Mladek @ 2020-11-03 14:10 UTC (permalink / raw)
To: Steven Rostedt
Cc: Anton Vorontsov, linux-doc, Peter Zijlstra,
Sebastian Andrzej Siewior, Kamalesh Babulal, James E.J. Bottomley,
Guo Ren, H. Peter Anvin, live-patching, Miroslav Benes,
Ingo Molnar, linux-s390, Joe Lawrence, Jonathan Corbet,
Mauro Carvalho Chehab, Helge Deller, x86, linux-csky,
Christian Borntraeger, Kees Cook, Vasily Gorbik, Heiko Carstens,
Jiri Kosina, Borislav Petkov, Josh Poimboeuf, Thomas Gleixner,
Tony Luck, linux-parisc, linux-kernel, Masami Hiramatsu,
Colin Cross, Paul Mackerras, Andrew Morton, linuxppc-dev
In-Reply-To: <20201102142254.7e148f8a@gandalf.local.home>
On Mon 2020-11-02 14:23:14, Steven Rostedt wrote:
> From c532ff6b048dd4a12943b05c7b8ce30666c587c8 Mon Sep 17 00:00:00 2001
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> Date: Thu, 29 Oct 2020 15:27:06 -0400
> Subject: [PATCH] ftrace: Add recording of functions that caused recursion
>
> This adds CONFIG_FTRACE_RECORD_RECURSION that will record to a file
> "recursed_functions" all the functions that caused recursion while a
> callback to the function tracer was running.
>
> diff --git a/include/linux/trace_recursion.h b/include/linux/trace_recursion.h
> index ac3d73484cb2..1cba5fe8777a 100644
> --- a/include/linux/trace_recursion.h
> +++ b/include/linux/trace_recursion.h
> @@ -142,7 +142,28 @@ static __always_inline int trace_get_context_bit(void)
> pc & HARDIRQ_MASK ? TRACE_CTX_IRQ : TRACE_CTX_SOFTIRQ;
> }
>
> -static __always_inline int trace_test_and_set_recursion(int start, int max)
> +#ifdef CONFIG_FTRACE_RECORD_RECURSION
> +extern void ftrace_record_recursion(unsigned long ip, unsigned long parent_ip);
> +/*
> +* The paranoid_test check can cause dropped reports (unlikely), but
> +* if the recursion is common, it will likely still be recorded later.
> +* But the paranoid_test is needed to make sure we don't crash.
> +*/
> +# define do_ftrace_record_recursion(ip, pip) \
> + do { \
> + static atomic_t paranoid_test; \
> + if (!atomic_read(¶noid_test)) { \
> + atomic_inc(¶noid_test); \
> + ftrace_record_recursion(ip, pip); \
> + atomic_dec(¶noid_test); \
BTW: What is actually the purpose of paranoid_test, please?
It prevents nested ftrace_record_recursion() calls on the same CPU
(recursion, nesting from IRQ, NMI context).
Parallel calls from different CPUs are still possible:
CPU0 CPU1
if (!atomic_read(¶noid_test)) if (!atomic_read(¶noid_test))
// passes // passes
atomic_inc(¶noid_test); atomic_inc(¶noid_test);
I do not see how a nested call could cause crash while a parallel
one would be OK.
> --- /dev/null
> +++ b/kernel/trace/trace_recursion_record.c
> @@ -0,0 +1,236 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/seq_file.h>
> +#include <linux/kallsyms.h>
> +#include <linux/module.h>
> +#include <linux/ftrace.h>
> +#include <linux/fs.h>
> +
> +#include "trace_output.h"
> +
> +struct recursed_functions {
> + unsigned long ip;
> + unsigned long parent_ip;
> +};
> +
> +static struct recursed_functions recursed_functions[CONFIG_FTRACE_RECORD_RECURSION_SIZE];
> +static atomic_t nr_records;
> +
> +/*
> + * Cache the last found function. Yes, updates to this is racey, but
> + * so is memory cache ;-)
> + */
> +static unsigned long cached_function;
> +
> +void ftrace_record_recursion(unsigned long ip, unsigned long parent_ip)
> +{
> + int index = 0;
> + int i;
> + unsigned long old;
> +
> + again:
> + /* First check the last one recorded */
> + if (ip == cached_function)
> + return;
> +
> + i = atomic_read(&nr_records);
> + /* nr_records is -1 when clearing records */
> + smp_mb__after_atomic();
> + if (i < 0)
> + return;
> +
> + /*
> + * If there's two writers and this writer comes in second,
> + * the cmpxchg() below to update the ip will fail. Then this
> + * writer will try again. It is possible that index will now
> + * be greater than nr_records. This is because the writer
> + * that succeeded has not updated the nr_records yet.
> + * This writer could keep trying again until the other writer
> + * updates nr_records. But if the other writer takes an
> + * interrupt, and that interrupt locks up that CPU, we do
> + * not want this CPU to lock up due to the recursion protection,
> + * and have a bug report showing this CPU as the cause of
> + * locking up the computer. To not lose this record, this
> + * writer will simply use the next position to update the
> + * recursed_functions, and it will update the nr_records
> + * accordingly.
> + */
> + if (index < i)
> + index = i;
> + if (index >= CONFIG_FTRACE_RECORD_RECURSION_SIZE)
> + return;
> +
> + for (i = index - 1; i >= 0; i--) {
> + if (recursed_functions[i].ip == ip) {
> + cached_function = ip;
> + return;
> + }
> + }
> +
> + cached_function = ip;
> +
> + /*
> + * We only want to add a function if it hasn't been added before.
> + * Add to the current location before incrementing the count.
> + * If it fails to add, then increment the index (save in i)
> + * and try again.
> + */
> + old = cmpxchg(&recursed_functions[index].ip, 0, ip);
> + if (old != 0) {
> + /* Did something else already added this for us? */
> + if (old == ip)
> + return;
> + /* Try the next location (use i for the next index) */
> + index++;
> + goto again;
> + }
> +
> + recursed_functions[index].parent_ip = parent_ip;
> +
> + /*
> + * It's still possible that we could race with the clearing
> + * CPU0 CPU1
> + * ---- ----
> + * ip = func
> + * nr_records = -1;
> + * recursed_functions[0] = 0;
> + * i = -1
> + * if (i < 0)
> + * nr_records = 0;
> + * (new recursion detected)
> + * recursed_functions[0] = func
> + * cmpxchg(recursed_functions[0],
> + * func, 0)
> + *
> + * But the worse that could happen is that we get a zero in
> + * the recursed_functions array, and it's likely that "func" will
> + * be recorded again.
> + */
> + i = atomic_read(&nr_records);
> + smp_mb__after_atomic();
> + if (i < 0)
> + cmpxchg(&recursed_functions[index].ip, ip, 0);
> + else if (i <= index)
> + atomic_cmpxchg(&nr_records, i, index + 1);
Are you aware of the following race, please?
CPU0 CPU1
ftrace_record_recursion()
i = atomic_read(&nr_records);
// i = 20 (for example)
if (i < index)
index = i;
// index = 20;
recursed_function_open()
atomic_set(&nr_records, -1);
memset(recursed_functions, 0, );
atomic_set(&nr_records, 0);
// successfully store ip at index == 20
cmpxchg(&recursed_functions[index].ip, 0, ip);
recursed_functions[index].parent_ip = parent_ip;
// check race with clearing
i = atomic_read(&nr_records);
// i == 0
if (i < 0)
// no
else
atomic_cmpxchg(&nr_records, i, index + 1);
RESULT:
+ nr_records == 21
+ and slots 0..19 are zeroed
I played with the code and ended with head entangled by chicken & egg
like problems.
I believe that a solution might be a combined atomic variable from
nr_records + cleanup_count.
ftrace_record_recursion() would be allowed to increase nr_records
only when cleanup_count is still the same. cleanup_count would
be incremented together with clearing nr_records.
Well, I am not sure if it is worth the effort. The race is rather
theoretical. In the worst case, the cache might contain many
zero values.
Anyway, it is yet another experience for me that lockless algorithms
are more tricky that one would expect.
Best Regards,
Petr
^ 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