Re: [PATCH 34/36] tty: serial: pmac_zilog: Make disposable variable __always_unused
On 05. 11. 20, 8:04, Christophe Leroy wrote:
Le 04/11/2020 à 20:35, Lee Jones a écrit :
Fixes the following W=1 kernel build warning(s):
drivers/tty/serial/pmac_zilog.h:365:58: warning: variable ‘garbage’
set but not used [-Wunused-but-set-variable]
Explain how you are fixing this warning.
Setting __always_unused is usually not the good solution for fixing
this warning, but here I guess this is likely the good solution. But it
should be explained why.
Or, why is the "garbage =" needed in the first place? read_zsdata is not
defined with __warn_unused_result__. And even if it was, would
(void)!read_zsdata(port) fix it?
Cc: Greg Kroah-Hartman
Cc: Jiri Slaby
Cc: Michael Ellerman
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: linux-ser...@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Lee Jones
---
drivers/tty/serial/pmac_zilog.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/pmac_zilog.h
b/drivers/tty/serial/pmac_zilog.h
index bb874e76810e0..968aec7c1cf82 100644
--- a/drivers/tty/serial/pmac_zilog.h
+++ b/drivers/tty/serial/pmac_zilog.h
@@ -362,7 +362,7 @@ static inline void zssync(struct uart_pmac_port
*port)
/* Misc macros */
#define ZS_CLEARERR(port) (write_zsreg(port, 0, ERR_RES))
-#define ZS_CLEARFIFO(port) do { volatile unsigned char garbage; \
+#define ZS_CLEARFIFO(port) do { volatile unsigned char
__always_unused garbage; \
garbage = read_zsdata(port); \
garbage = read_zsdata(port); \
garbage = read_zsdata(port); \
thanks,
--
js
Re: [PATCH 31/36] powerpc: asm: hvconsole: Move 'hvc_vio_init_early's prototype to shared location
Le 04/11/2020 à 20:35, Lee Jones a écrit : Fixes the following W=1 kernel build warning(s): drivers/tty/hvc/hvc_vio.c:385:13: warning: no previous prototype for ‘hvc_vio_init_early’ [-Wmissing-prototypes] 385 | void __init hvc_vio_init_early(void) | ^~ Cc: Michael Ellerman Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: linuxppc-dev@lists.ozlabs.org Signed-off-by: Lee Jones --- arch/powerpc/include/asm/hvconsole.h | 3 +++ arch/powerpc/platforms/pseries/pseries.h | 3 --- arch/powerpc/platforms/pseries/setup.c | 1 + 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/include/asm/hvconsole.h b/arch/powerpc/include/asm/hvconsole.h index 999ed5ac90531..936a1ee1ac786 100644 --- a/arch/powerpc/include/asm/hvconsole.h +++ b/arch/powerpc/include/asm/hvconsole.h @@ -24,5 +24,8 @@ extern int hvc_get_chars(uint32_t vtermno, char *buf, int count); extern int hvc_put_chars(uint32_t vtermno, const char *buf, int count); +/* Provided by HVC VIO */ +extern void hvc_vio_init_early(void); + Declaring a prototype 'extern' is pointless. Don't add new misuse of 'extern' keyword. #endif /* __KERNEL__ */ #endif /* _PPC64_HVCONSOLE_H */ diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h index 13fa370a87e4e..7be5b054dfc36 100644 --- a/arch/powerpc/platforms/pseries/pseries.h +++ b/arch/powerpc/platforms/pseries/pseries.h @@ -43,9 +43,6 @@ extern void pSeries_final_fixup(void); /* Poweron flag used for enabling auto ups restart */ extern unsigned long rtas_poweron_auto; -/* Provided by HVC VIO */ -extern void hvc_vio_init_early(void); - /* Dynamic logical Partitioning/Mobility */ extern void dlpar_free_cc_nodes(struct device_node *); extern void dlpar_free_cc_property(struct property *); diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c index 633c45ec406da..6999b83f06612 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -71,6 +71,7 @@ #include #include #include +#include #include "pseries.h" #include "../../../../drivers/pci/pci.h" Christophe
Re: [PATCH 34/36] tty: serial: pmac_zilog: Make disposable variable __always_unused
Le 04/11/2020 à 20:35, Lee Jones a écrit :
Fixes the following W=1 kernel build warning(s):
drivers/tty/serial/pmac_zilog.h:365:58: warning: variable ‘garbage’ set but
not used [-Wunused-but-set-variable]
Explain how you are fixing this warning.
Setting __always_unused is usually not the good solution for fixing this warning, but here I guess
this is likely the good solution. But it should be explained why.
Christophe
Cc: Greg Kroah-Hartman
Cc: Jiri Slaby
Cc: Michael Ellerman
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: linux-ser...@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Lee Jones
---
drivers/tty/serial/pmac_zilog.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/pmac_zilog.h b/drivers/tty/serial/pmac_zilog.h
index bb874e76810e0..968aec7c1cf82 100644
--- a/drivers/tty/serial/pmac_zilog.h
+++ b/drivers/tty/serial/pmac_zilog.h
@@ -362,7 +362,7 @@ static inline void zssync(struct uart_pmac_port *port)
/* Misc macros */
#define ZS_CLEARERR(port)(write_zsreg(port, 0, ERR_RES))
-#define ZS_CLEARFIFO(port) do { volatile unsigned char garbage; \
+#define ZS_CLEARFIFO(port) do { volatile unsigned char __always_unused
garbage; \
garbage = read_zsdata(port); \
garbage = read_zsdata(port); \
garbage = read_zsdata(port); \
[PATCH v2 7/8] powerpc/signal64: Rewrite handle_rt_signal64() to minimise uaccess switches
From: Daniel Axtens
Add uaccess blocks and use the 'unsafe' versions of functions doing user
access where possible to reduce the number of times uaccess has to be
opened/closed.
There is no 'unsafe' version of copy_siginfo_to_user, so move it
slightly to allow for a "longer" uaccess block.
Signed-off-by: Daniel Axtens
Co-developed-by: Christopher M. Riedl
Signed-off-by: Christopher M. Riedl
---
arch/powerpc/kernel/signal_64.c | 54 +
1 file changed, 34 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index d72153825719..d17f2d5436d2 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -848,44 +848,51 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t
*set,
unsigned long msr __maybe_unused = regs->msr;
frame = get_sigframe(ksig, tsk, sizeof(*frame), 0);
- if (!access_ok(frame, sizeof(*frame)))
- goto badframe;
- err |= __put_user(&frame->info, &frame->pinfo);
- err |= __put_user(&frame->uc, &frame->puc);
- err |= copy_siginfo_to_user(&frame->info, &ksig->info);
- if (err)
+ /* This only applies when calling unsafe_setup_sigcontext() and must be
+* called before opening the uaccess window.
+*/
+ if (!MSR_TM_ACTIVE(msr))
+ prepare_setup_sigcontext(tsk, 1);
+
+ if (!user_write_access_begin(frame, sizeof(*frame)))
goto badframe;
+ unsafe_put_user(&frame->info, &frame->pinfo, badframe_block);
+ unsafe_put_user(&frame->uc, &frame->puc, badframe_block);
+
/* Create the ucontext. */
- err |= __put_user(0, &frame->uc.uc_flags);
- err |= __save_altstack(&frame->uc.uc_stack, regs->gpr[1]);
+ unsafe_put_user(0, &frame->uc.uc_flags, badframe_block);
+ unsafe_save_altstack(&frame->uc.uc_stack, regs->gpr[1], badframe_block);
if (MSR_TM_ACTIVE(msr)) {
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
/* The ucontext_t passed to userland points to the second
* ucontext_t (for transactional state) with its uc_link ptr.
*/
- err |= __put_user(&frame->uc_transact, &frame->uc.uc_link);
+ unsafe_put_user(&frame->uc_transact, &frame->uc.uc_link,
badframe_block);
+
+ user_write_access_end();
+
err |= setup_tm_sigcontexts(&frame->uc.uc_mcontext,
&frame->uc_transact.uc_mcontext,
tsk, ksig->sig, NULL,
(unsigned
long)ksig->ka.sa.sa_handler,
msr);
+
+ if (!user_write_access_begin(frame, sizeof(struct rt_sigframe)))
+ goto badframe;
+
#endif
} else {
- err |= __put_user(0, &frame->uc.uc_link);
- prepare_setup_sigcontext(tsk, 1);
- if (!user_write_access_begin(frame, sizeof(struct rt_sigframe)))
- return -EFAULT;
- err |= __unsafe_setup_sigcontext(&frame->uc.uc_mcontext, tsk,
- ksig->sig, NULL,
- (unsigned
long)ksig->ka.sa.sa_handler, 1);
- user_write_access_end();
+ unsafe_put_user(0, &frame->uc.uc_link, badframe_block);
+ unsafe_setup_sigcontext(&frame->uc.uc_mcontext, tsk, ksig->sig,
+ NULL, (unsigned
long)ksig->ka.sa.sa_handler,
+ 1, badframe_block);
}
- err |= __copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set));
- if (err)
- goto badframe;
+
+ unsafe_copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set),
badframe_block);
+ user_write_access_end();
/* Make sure signal handler doesn't get spurious FP exceptions */
tsk->thread.fp_state.fpscr = 0;
@@ -900,6 +907,11 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
regs->nip = (unsigned long) &frame->tramp[0];
}
+
+ /* Save the siginfo outside of the unsafe block. */
+ if (copy_siginfo_to_user(&frame->info, &ksig->info))
+ goto badframe;
+
/* Allocate a dummy caller frame for the signal handler. */
newsp = ((unsigned long)frame) - __SIGNAL_FRAMESIZE;
err |= put_user(regs->gpr[1], (unsigned long __user *)newsp);
@@ -939,6 +951,8 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
return 0;
+badframe_block:
+ user_write_access_end();
badframe:
signal_fault(current, regs, "handle_rt_signal64", frame);
--
2.29.0
[PATCH v2 5/8] powerpc/signal64: Replace setup_sigcontext() w/ unsafe_setup_sigcontext()
Previously setup_sigcontext() performed a costly KUAP switch on every
uaccess operation. These repeated uaccess switches cause a significant
drop in signal handling performance.
Rewrite setup_sigcontext() to assume that a userspace write access window
is open. Replace all uaccess functions with their 'unsafe' versions
which avoid the repeated uaccess switches.
Signed-off-by: Christopher M. Riedl
---
arch/powerpc/kernel/signal_64.c | 70 -
1 file changed, 43 insertions(+), 27 deletions(-)
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index d3e9519b2e62..3f25309826b6 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -101,9 +101,13 @@ static void prepare_setup_sigcontext(struct task_struct
*tsk, int ctx_has_vsx_re
* Set up the sigcontext for the signal frame.
*/
-static long setup_sigcontext(struct sigcontext __user *sc,
- struct task_struct *tsk, int signr, sigset_t *set,
- unsigned long handler, int ctx_has_vsx_region)
+#define unsafe_setup_sigcontext(sc, tsk, signr, set, handler, \
+ ctx_has_vsx_region, e) \
+ unsafe_op_wrap(__unsafe_setup_sigcontext(sc, tsk, signr, set, \
+ handler, ctx_has_vsx_region), e)
+static long notrace __unsafe_setup_sigcontext(struct sigcontext __user *sc,
+ struct task_struct *tsk, int signr,
sigset_t *set,
+ unsigned long handler, int
ctx_has_vsx_region)
{
/* When CONFIG_ALTIVEC is set, we _always_ setup v_regs even if the
* process never used altivec yet (MSR_VEC is zero in pt_regs of
@@ -118,20 +122,19 @@ static long setup_sigcontext(struct sigcontext __user *sc,
#endif
struct pt_regs *regs = tsk->thread.regs;
unsigned long msr = regs->msr;
- long err = 0;
/* Force usr to alway see softe as 1 (interrupts enabled) */
unsigned long softe = 0x1;
BUG_ON(tsk != current);
#ifdef CONFIG_ALTIVEC
- err |= __put_user(v_regs, &sc->v_regs);
+ unsafe_put_user(v_regs, &sc->v_regs, efault_out);
/* save altivec registers */
if (tsk->thread.used_vr) {
/* Copy 33 vec registers (vr0..31 and vscr) to the stack */
- err |= __copy_to_user(v_regs, &tsk->thread.vr_state,
- 33 * sizeof(vector128));
+ unsafe_copy_to_user(v_regs, &tsk->thread.vr_state,
+ 33 * sizeof(vector128), efault_out);
/* set MSR_VEC in the MSR value in the frame to indicate that
sc->v_reg)
* contains valid data.
*/
@@ -140,12 +143,12 @@ static long setup_sigcontext(struct sigcontext __user *sc,
/* We always copy to/from vrsave, it's 0 if we don't have or don't
* use altivec.
*/
- err |= __put_user(tsk->thread.vrsave, (u32 __user *)&v_regs[33]);
+ unsafe_put_user(tsk->thread.vrsave, (u32 __user *)&v_regs[33],
efault_out);
#else /* CONFIG_ALTIVEC */
- err |= __put_user(0, &sc->v_regs);
+ unsafe_put_user(0, &sc->v_regs, efault_out);
#endif /* CONFIG_ALTIVEC */
/* copy fpr regs and fpscr */
- err |= copy_fpr_to_user(&sc->fp_regs, tsk);
+ unsafe_copy_fpr_to_user(&sc->fp_regs, tsk, efault_out);
/*
* Clear the MSR VSX bit to indicate there is no valid state attached
@@ -160,24 +163,27 @@ static long setup_sigcontext(struct sigcontext __user *sc,
*/
if (tsk->thread.used_vsr && ctx_has_vsx_region) {
v_regs += ELF_NVRREG;
- err |= copy_vsx_to_user(v_regs, tsk);
+ unsafe_copy_vsx_to_user(v_regs, tsk, efault_out);
/* set MSR_VSX in the MSR value in the frame to
* indicate that sc->vs_reg) contains valid data.
*/
msr |= MSR_VSX;
}
#endif /* CONFIG_VSX */
- err |= __put_user(&sc->gp_regs, &sc->regs);
+ unsafe_put_user(&sc->gp_regs, &sc->regs, efault_out);
WARN_ON(!FULL_REGS(regs));
- err |= __copy_to_user(&sc->gp_regs, regs, GP_REGS_SIZE);
- err |= __put_user(msr, &sc->gp_regs[PT_MSR]);
- err |= __put_user(softe, &sc->gp_regs[PT_SOFTE]);
- err |= __put_user(signr, &sc->signal);
- err |= __put_user(handler, &sc->handler);
+ unsafe_copy_to_user(&sc->gp_regs, regs, GP_REGS_SIZE, efault_out);
+ unsafe_put_user(msr, &sc->gp_regs[PT_MSR], efault_out);
+ unsafe_put_user(softe, &sc->gp_regs[PT_SOFTE], efault_out);
+ unsafe_put_user(signr, &sc->signal, efault_out);
+ unsafe_put_user(handler, &sc->handler, efault_out);
if (set != NULL)
- err |= __put_user(set->sig[0], &sc->oldmask);
+ unsafe_put_user(set->sig[0], &sc->oldmask, efault_out);
- return
[PATCH v2 1/8] powerpc/uaccess: Add unsafe_copy_from_user
Implement raw_copy_from_user_allowed() which assumes that userspace read
access is open. Use this new function to implement raw_copy_from_user().
Finally, wrap the new function to follow the usual "unsafe_" convention
of taking a label argument. The new raw_copy_from_user_allowed() calls
__copy_tofrom_user() internally, but this is still safe to call in user
access blocks formed with user_*_access_begin()/user_*_access_end()
since asm functions are not instrumented for tracing.
Signed-off-by: Christopher M. Riedl
---
arch/powerpc/include/asm/uaccess.h | 28 +++-
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h
b/arch/powerpc/include/asm/uaccess.h
index ef5bbb705c08..96b4abab4f5a 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -403,38 +403,45 @@ raw_copy_in_user(void __user *to, const void __user
*from, unsigned long n)
}
#endif /* __powerpc64__ */
-static inline unsigned long raw_copy_from_user(void *to,
- const void __user *from, unsigned long n)
+static inline unsigned long
+raw_copy_from_user_allowed(void *to, const void __user *from, unsigned long n)
{
- unsigned long ret;
if (__builtin_constant_p(n) && (n <= 8)) {
- ret = 1;
+ unsigned long ret = 1;
switch (n) {
case 1:
barrier_nospec();
- __get_user_size(*(u8 *)to, from, 1, ret);
+ __get_user_size_allowed(*(u8 *)to, from, 1, ret);
break;
case 2:
barrier_nospec();
- __get_user_size(*(u16 *)to, from, 2, ret);
+ __get_user_size_allowed(*(u16 *)to, from, 2, ret);
break;
case 4:
barrier_nospec();
- __get_user_size(*(u32 *)to, from, 4, ret);
+ __get_user_size_allowed(*(u32 *)to, from, 4, ret);
break;
case 8:
barrier_nospec();
- __get_user_size(*(u64 *)to, from, 8, ret);
+ __get_user_size_allowed(*(u64 *)to, from, 8, ret);
break;
}
if (ret == 0)
return 0;
}
+ return __copy_tofrom_user((__force void __user *)to, from, n);
+}
+
+static inline unsigned long
+raw_copy_from_user(void *to, const void __user *from, unsigned long n)
+{
+ unsigned long ret;
+
barrier_nospec();
allow_read_from_user(from, n);
- ret = __copy_tofrom_user((__force void __user *)to, from, n);
+ ret = raw_copy_from_user_allowed(to, from, n);
prevent_read_from_user(from, n);
return ret;
}
@@ -542,6 +549,9 @@ user_write_access_begin(const void __user *ptr, size_t len)
#define unsafe_get_user(x, p, e) unsafe_op_wrap(__get_user_allowed(x, p), e)
#define unsafe_put_user(x, p, e) __put_user_goto(x, p, e)
+#define unsafe_copy_from_user(d, s, l, e) \
+ unsafe_op_wrap(raw_copy_from_user_allowed(d, s, l), e)
+
#define unsafe_copy_to_user(d, s, l, e) \
do { \
u8 __user *_dst = (u8 __user *)(d); \
--
2.29.0
[PATCH v2 6/8] powerpc/signal64: Replace restore_sigcontext() w/ unsafe_restore_sigcontext()
Previously restore_sigcontext() performed a costly KUAP switch on every
uaccess operation. These repeated uaccess switches cause a significant
drop in signal handling performance.
Rewrite restore_sigcontext() to assume that a userspace read access
window is open. Replace all uaccess functions with their 'unsafe'
versions which avoid the repeated uaccess switches.
Signed-off-by: Christopher M. Riedl
---
arch/powerpc/kernel/signal_64.c | 68 -
1 file changed, 41 insertions(+), 27 deletions(-)
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index 3f25309826b6..d72153825719 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -326,14 +326,14 @@ static long setup_tm_sigcontexts(struct sigcontext __user
*sc,
/*
* Restore the sigcontext from the signal frame.
*/
-
-static long restore_sigcontext(struct task_struct *tsk, sigset_t *set, int sig,
- struct sigcontext __user *sc)
+#define unsafe_restore_sigcontext(tsk, set, sig, sc, e) \
+ unsafe_op_wrap(__unsafe_restore_sigcontext(tsk, set, sig, sc), e)
+static long notrace __unsafe_restore_sigcontext(struct task_struct *tsk,
sigset_t *set,
+ int sig, struct sigcontext
__user *sc)
{
#ifdef CONFIG_ALTIVEC
elf_vrreg_t __user *v_regs;
#endif
- unsigned long err = 0;
unsigned long save_r13 = 0;
unsigned long msr;
struct pt_regs *regs = tsk->thread.regs;
@@ -348,27 +348,28 @@ static long restore_sigcontext(struct task_struct *tsk,
sigset_t *set, int sig,
save_r13 = regs->gpr[13];
/* copy the GPRs */
- err |= __copy_from_user(regs->gpr, sc->gp_regs, sizeof(regs->gpr));
- err |= __get_user(regs->nip, &sc->gp_regs[PT_NIP]);
+ unsafe_copy_from_user(regs->gpr, sc->gp_regs, sizeof(regs->gpr),
+ efault_out);
+ unsafe_get_user(regs->nip, &sc->gp_regs[PT_NIP], efault_out);
/* get MSR separately, transfer the LE bit if doing signal return */
- err |= __get_user(msr, &sc->gp_regs[PT_MSR]);
+ unsafe_get_user(msr, &sc->gp_regs[PT_MSR], efault_out);
if (sig)
regs->msr = (regs->msr & ~MSR_LE) | (msr & MSR_LE);
- err |= __get_user(regs->orig_gpr3, &sc->gp_regs[PT_ORIG_R3]);
- err |= __get_user(regs->ctr, &sc->gp_regs[PT_CTR]);
- err |= __get_user(regs->link, &sc->gp_regs[PT_LNK]);
- err |= __get_user(regs->xer, &sc->gp_regs[PT_XER]);
- err |= __get_user(regs->ccr, &sc->gp_regs[PT_CCR]);
+ unsafe_get_user(regs->orig_gpr3, &sc->gp_regs[PT_ORIG_R3], efault_out);
+ unsafe_get_user(regs->ctr, &sc->gp_regs[PT_CTR], efault_out);
+ unsafe_get_user(regs->link, &sc->gp_regs[PT_LNK], efault_out);
+ unsafe_get_user(regs->xer, &sc->gp_regs[PT_XER], efault_out);
+ unsafe_get_user(regs->ccr, &sc->gp_regs[PT_CCR], efault_out);
/* Don't allow userspace to set SOFTE */
set_trap_norestart(regs);
- err |= __get_user(regs->dar, &sc->gp_regs[PT_DAR]);
- err |= __get_user(regs->dsisr, &sc->gp_regs[PT_DSISR]);
- err |= __get_user(regs->result, &sc->gp_regs[PT_RESULT]);
+ unsafe_get_user(regs->dar, &sc->gp_regs[PT_DAR], efault_out);
+ unsafe_get_user(regs->dsisr, &sc->gp_regs[PT_DSISR], efault_out);
+ unsafe_get_user(regs->result, &sc->gp_regs[PT_RESULT], efault_out);
if (!sig)
regs->gpr[13] = save_r13;
if (set != NULL)
- err |= __get_user(set->sig[0], &sc->oldmask);
+ unsafe_get_user(set->sig[0], &sc->oldmask, efault_out);
/*
* Force reload of FP/VEC.
@@ -378,29 +379,28 @@ static long restore_sigcontext(struct task_struct *tsk,
sigset_t *set, int sig,
regs->msr &= ~(MSR_FP | MSR_FE0 | MSR_FE1 | MSR_VEC | MSR_VSX);
#ifdef CONFIG_ALTIVEC
- err |= __get_user(v_regs, &sc->v_regs);
- if (err)
- return err;
+ unsafe_get_user(v_regs, &sc->v_regs, efault_out);
if (v_regs && !access_ok(v_regs, 34 * sizeof(vector128)))
return -EFAULT;
/* Copy 33 vec registers (vr0..31 and vscr) from the stack */
if (v_regs != NULL && (msr & MSR_VEC) != 0) {
- err |= __copy_from_user(&tsk->thread.vr_state, v_regs,
- 33 * sizeof(vector128));
+ unsafe_copy_from_user(&tsk->thread.vr_state, v_regs,
+ 33 * sizeof(vector128), efault_out);
tsk->thread.used_vr = true;
} else if (tsk->thread.used_vr) {
memset(&tsk->thread.vr_state, 0, 33 * sizeof(vector128));
}
/* Always get VRSAVE back */
if (v_regs != NULL)
- err |= __get_user(tsk->thread.vrsave, (u32 __user
*)&v_regs[33]);
+ unsafe_get_user(tsk->thread.vrsave, (u32 __user *)&v_regs[33
[PATCH v2 8/8] powerpc/signal64: Rewrite rt_sigreturn() to minimise uaccess switches
From: Daniel Axtens
Add uaccess blocks and use the 'unsafe' versions of functions doing user
access where possible to reduce the number of times uaccess has to be
opened/closed.
Signed-off-by: Daniel Axtens
Co-developed-by: Christopher M. Riedl
Signed-off-by: Christopher M. Riedl
---
arch/powerpc/kernel/signal_64.c | 24 ++--
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index d17f2d5436d2..82e68a508e5c 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -784,8 +784,11 @@ SYSCALL_DEFINE0(rt_sigreturn)
regs->msr &= ~MSR_TS_MASK;
#endif
- if (__get_user(msr, &uc->uc_mcontext.gp_regs[PT_MSR]))
+ if (!user_read_access_begin(uc, sizeof(*uc)))
goto badframe;
+
+ unsafe_get_user(msr, &uc->uc_mcontext.gp_regs[PT_MSR], badframe_block);
+
if (MSR_TM_ACTIVE(msr)) {
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
/* We recheckpoint on return. */
@@ -793,10 +796,12 @@ SYSCALL_DEFINE0(rt_sigreturn)
/* Trying to start TM on non TM system */
if (!cpu_has_feature(CPU_FTR_TM))
- goto badframe;
+ goto badframe_block;
+
+ unsafe_get_user(uc_transact, &uc->uc_link, badframe_block);
+
+ user_read_access_end();
- if (__get_user(uc_transact, &uc->uc_link))
- goto badframe;
if (restore_tm_sigcontexts(current, &uc->uc_mcontext,
&uc_transact->uc_mcontext))
goto badframe;
@@ -815,12 +820,9 @@ SYSCALL_DEFINE0(rt_sigreturn)
* causing a TM bad thing.
*/
current->thread.regs->msr &= ~MSR_TS_MASK;
- if (!user_read_access_begin(uc, sizeof(*uc)))
- return -EFAULT;
- if (__unsafe_restore_sigcontext(current, NULL, 1,
&uc->uc_mcontext)) {
- user_read_access_end();
- goto badframe;
- }
+ unsafe_restore_sigcontext(current, NULL, 1, &uc->uc_mcontext,
+ badframe_block);
+
user_read_access_end();
}
@@ -830,6 +832,8 @@ SYSCALL_DEFINE0(rt_sigreturn)
set_thread_flag(TIF_RESTOREALL);
return 0;
+badframe_block:
+ user_read_access_end();
badframe:
signal_fault(current, regs, "rt_sigreturn", uc);
--
2.29.0
[PATCH v2 0/8] Improve signal performance on PPC64 with KUAP
As reported by Anton, there is a large penalty to signal handling
performance on radix systems using KUAP. The signal handling code
performs many user access operations, each of which needs to switch the
KUAP permissions bit to open and then close user access. This involves a
costly 'mtspr' operation [0].
There is existing work done on x86 and by Christopher Leroy for PPC32 to
instead open up user access in "blocks" using user_*_access_{begin,end}.
We can do the same in PPC64 to bring performance back up on KUAP-enabled
radix systems.
This series applies on top of Christophe Leroy's work for PPC32 [1] (I'm
sure patchwork won't be too happy about that).
The first two patches add some needed 'unsafe' versions of copy-from
functions. While these do not make use of asm-goto they still allow for
avoiding the repeated uaccess switches.
The third patch moves functions called by setup_sigcontext() into a new
prepare_setup_sigcontext() to simplify converting setup_sigcontext()
into an 'unsafe' version which assumes an open uaccess window later.
The fourth patch cleans-up some of the Transactional Memory ifdef stuff
to simplify using uaccess blocks later.
The next two patches rewrite some of the signal64 helper functions to
be 'unsafe'. Finally, the last two patches update the main signal
handling functions to make use of the new 'unsafe' helpers and eliminate
some additional uaccess switching.
I used the will-it-scale signal1 benchmark to measure and compare
performance [2]. The below results are from a P9 Blackbird system. Note
that currently hash does not support KUAP and is therefore used as the
"baseline" comparison. Bigger numbers are better:
signal1_threads -t1 -s10
| | hash | radix |
| --- | -- | -- |
| linuxppc/next | 289014 | 158408 |
| unsafe-signal64 | 298506 | 253053 |
[0]: https://github.com/linuxppc/issues/issues/277
[1]: https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=196278
[2]: https://github.com/antonblanchard/will-it-scale/blob/master/tests/signal1.c
v2: * Rebase on latest linuxppc/next + Christophe Leroy's PPC32
signal series
* Simplify/remove TM ifdefery similar to PPC32 series and clean
up the uaccess begin/end calls
* Isolate non-inline functions so they are not called when
uaccess window is open
Christopher M. Riedl (6):
powerpc/uaccess: Add unsafe_copy_from_user
powerpc/signal: Add unsafe_copy_{vsx,fpr}_from_user()
powerpc/signal64: Move non-inline functions out of setup_sigcontext()
powerpc/signal64: Remove TM ifdefery in middle of if/else block
powerpc/signal64: Replace setup_sigcontext() w/
unsafe_setup_sigcontext()
powerpc/signal64: Replace restore_sigcontext() w/
unsafe_restore_sigcontext()
Daniel Axtens (2):
powerpc/signal64: Rewrite handle_rt_signal64() to minimise uaccess
switches
powerpc/signal64: Rewrite rt_sigreturn() to minimise uaccess switches
arch/powerpc/include/asm/uaccess.h | 28 ++--
arch/powerpc/kernel/signal.h | 33
arch/powerpc/kernel/signal_64.c| 239 ++---
3 files changed, 201 insertions(+), 99 deletions(-)
--
2.29.0
[PATCH v2 4/8] powerpc/signal64: Remove TM ifdefery in middle of if/else block
Similar to commit 1c32940f5220 ("powerpc/signal32: Remove ifdefery in
middle of if/else") for PPC32, remove the messy ifdef. Unlike PPC32, the
ifdef cannot be removed entirely since the uc_transact member of the
sigframe depends on CONFIG_PPC_TRANSACTIONAL_MEM=y.
Signed-off-by: Christopher M. Riedl
---
arch/powerpc/kernel/signal_64.c | 17 +++--
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index ece1f982dd05..d3e9519b2e62 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -710,9 +710,7 @@ SYSCALL_DEFINE0(rt_sigreturn)
struct pt_regs *regs = current_pt_regs();
struct ucontext __user *uc = (struct ucontext __user *)regs->gpr[1];
sigset_t set;
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
unsigned long msr;
-#endif
/* Always make any pending restarted system calls return -EINTR */
current->restart_block.fn = do_no_restart_syscall;
@@ -762,10 +760,12 @@ SYSCALL_DEFINE0(rt_sigreturn)
* restore_tm_sigcontexts.
*/
regs->msr &= ~MSR_TS_MASK;
+#endif
if (__get_user(msr, &uc->uc_mcontext.gp_regs[PT_MSR]))
goto badframe;
if (MSR_TM_ACTIVE(msr)) {
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
/* We recheckpoint on return. */
struct ucontext __user *uc_transact;
@@ -778,9 +778,8 @@ SYSCALL_DEFINE0(rt_sigreturn)
if (restore_tm_sigcontexts(current, &uc->uc_mcontext,
&uc_transact->uc_mcontext))
goto badframe;
- } else
#endif
- {
+ } else {
/*
* Fall through, for non-TM restore
*
@@ -818,10 +817,8 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
unsigned long newsp = 0;
long err = 0;
struct pt_regs *regs = tsk->thread.regs;
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
/* Save the thread's msr before get_tm_stackpointer() changes it */
- unsigned long msr = regs->msr;
-#endif
+ unsigned long msr __maybe_unused = regs->msr;
frame = get_sigframe(ksig, tsk, sizeof(*frame), 0);
if (!access_ok(frame, sizeof(*frame)))
@@ -836,8 +833,9 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
/* Create the ucontext. */
err |= __put_user(0, &frame->uc.uc_flags);
err |= __save_altstack(&frame->uc.uc_stack, regs->gpr[1]);
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+
if (MSR_TM_ACTIVE(msr)) {
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
/* The ucontext_t passed to userland points to the second
* ucontext_t (for transactional state) with its uc_link ptr.
*/
@@ -847,9 +845,8 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
tsk, ksig->sig, NULL,
(unsigned
long)ksig->ka.sa.sa_handler,
msr);
- } else
#endif
- {
+ } else {
err |= __put_user(0, &frame->uc.uc_link);
prepare_setup_sigcontext(tsk, 1);
err |= setup_sigcontext(&frame->uc.uc_mcontext, tsk, ksig->sig,
--
2.29.0
[PATCH v2 3/8] powerpc/signal64: Move non-inline functions out of setup_sigcontext()
There are non-inline functions which get called in setup_sigcontext() to
save register state to the thread struct. Move these functions into a
separate prepare_setup_sigcontext() function so that
setup_sigcontext() can be refactored later into an "unsafe" version
which assumes an open uaccess window. Non-inline functions should be
avoided when uaccess is open.
Signed-off-by: Christopher M. Riedl
---
arch/powerpc/kernel/signal_64.c | 32 +---
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index 7df088b9ad0f..ece1f982dd05 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -79,6 +79,24 @@ static elf_vrreg_t __user *sigcontext_vmx_regs(struct
sigcontext __user *sc)
}
#endif
+static void prepare_setup_sigcontext(struct task_struct *tsk, int
ctx_has_vsx_region)
+{
+#ifdef CONFIG_ALTIVEC
+ /* save altivec registers */
+ if (tsk->thread.used_vr)
+ flush_altivec_to_thread(tsk);
+ if (cpu_has_feature(CPU_FTR_ALTIVEC))
+ tsk->thread.vrsave = mfspr(SPRN_VRSAVE);
+#endif /* CONFIG_ALTIVEC */
+
+ flush_fp_to_thread(tsk);
+
+#ifdef CONFIG_VSX
+ if (tsk->thread.used_vsr && ctx_has_vsx_region)
+ flush_vsx_to_thread(tsk);
+#endif /* CONFIG_VSX */
+}
+
/*
* Set up the sigcontext for the signal frame.
*/
@@ -97,7 +115,6 @@ static long setup_sigcontext(struct sigcontext __user *sc,
*/
#ifdef CONFIG_ALTIVEC
elf_vrreg_t __user *v_regs = sigcontext_vmx_regs(sc);
- unsigned long vrsave;
#endif
struct pt_regs *regs = tsk->thread.regs;
unsigned long msr = regs->msr;
@@ -112,7 +129,6 @@ static long setup_sigcontext(struct sigcontext __user *sc,
/* save altivec registers */
if (tsk->thread.used_vr) {
- flush_altivec_to_thread(tsk);
/* Copy 33 vec registers (vr0..31 and vscr) to the stack */
err |= __copy_to_user(v_regs, &tsk->thread.vr_state,
33 * sizeof(vector128));
@@ -124,17 +140,10 @@ static long setup_sigcontext(struct sigcontext __user *sc,
/* We always copy to/from vrsave, it's 0 if we don't have or don't
* use altivec.
*/
- vrsave = 0;
- if (cpu_has_feature(CPU_FTR_ALTIVEC)) {
- vrsave = mfspr(SPRN_VRSAVE);
- tsk->thread.vrsave = vrsave;
- }
-
- err |= __put_user(vrsave, (u32 __user *)&v_regs[33]);
+ err |= __put_user(tsk->thread.vrsave, (u32 __user *)&v_regs[33]);
#else /* CONFIG_ALTIVEC */
err |= __put_user(0, &sc->v_regs);
#endif /* CONFIG_ALTIVEC */
- flush_fp_to_thread(tsk);
/* copy fpr regs and fpscr */
err |= copy_fpr_to_user(&sc->fp_regs, tsk);
@@ -150,7 +159,6 @@ static long setup_sigcontext(struct sigcontext __user *sc,
* VMX data.
*/
if (tsk->thread.used_vsr && ctx_has_vsx_region) {
- flush_vsx_to_thread(tsk);
v_regs += ELF_NVRREG;
err |= copy_vsx_to_user(v_regs, tsk);
/* set MSR_VSX in the MSR value in the frame to
@@ -655,6 +663,7 @@ SYSCALL_DEFINE3(swapcontext, struct ucontext __user *,
old_ctx,
ctx_has_vsx_region = 1;
if (old_ctx != NULL) {
+ prepare_setup_sigcontext(current, ctx_has_vsx_region);
if (!access_ok(old_ctx, ctx_size)
|| setup_sigcontext(&old_ctx->uc_mcontext, current, 0,
NULL, 0,
ctx_has_vsx_region)
@@ -842,6 +851,7 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
#endif
{
err |= __put_user(0, &frame->uc.uc_link);
+ prepare_setup_sigcontext(tsk, 1);
err |= setup_sigcontext(&frame->uc.uc_mcontext, tsk, ksig->sig,
NULL, (unsigned
long)ksig->ka.sa.sa_handler,
1);
--
2.29.0
[PATCH v2 2/8] powerpc/signal: Add unsafe_copy_{vsx,fpr}_from_user()
Reuse the "safe" implementation from signal.c except for calling
unsafe_copy_from_user() to copy into a local buffer. Unlike the
unsafe_copy_{vsx,fpr}_to_user() functions the "copy from" functions
cannot use unsafe_get_user() directly to bypass the local buffer since
doing so significantly reduces signal handling performance.
Signed-off-by: Christopher M. Riedl
---
arch/powerpc/kernel/signal.h | 33 +
1 file changed, 33 insertions(+)
diff --git a/arch/powerpc/kernel/signal.h b/arch/powerpc/kernel/signal.h
index 2559a681536e..e9aaeac0da37 100644
--- a/arch/powerpc/kernel/signal.h
+++ b/arch/powerpc/kernel/signal.h
@@ -53,6 +53,33 @@ unsigned long copy_ckfpr_from_user(struct task_struct *task,
void __user *from);
&buf[i], label);\
} while (0)
+#define unsafe_copy_fpr_from_user(task, from, label) do {\
+ struct task_struct *__t = task; \
+ u64 __user *__f = (u64 __user *)from; \
+ u64 buf[ELF_NFPREG];\
+ int i; \
+ \
+ unsafe_copy_from_user(buf, __f, ELF_NFPREG * sizeof(double),\
+ label); \
+ for (i = 0; i < ELF_NFPREG - 1; i++)\
+ __t->thread.TS_FPR(i) = buf[i]; \
+ __t->thread.fp_state.fpscr = buf[i];\
+} while (0)
+
+#define unsafe_copy_vsx_from_user(task, from, label) do {\
+ struct task_struct *__t = task; \
+ u64 __user *__f = (u64 __user *)from; \
+ u64 buf[ELF_NVSRHALFREG]; \
+ int i; \
+ \
+ unsafe_copy_from_user(buf, __f, \
+ ELF_NVSRHALFREG * sizeof(double), \
+ label); \
+ for (i = 0; i < ELF_NVSRHALFREG ; i++) \
+ __t->thread.fp_state.fpr[i][TS_VSRLOWOFFSET] = buf[i]; \
+} while (0)
+
+
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
#define unsafe_copy_ckfpr_to_user(to, task, label) do {\
struct task_struct *__t = task; \
@@ -80,6 +107,10 @@ unsigned long copy_ckfpr_from_user(struct task_struct
*task, void __user *from);
unsafe_copy_to_user(to, (task)->thread.fp_state.fpr,\
ELF_NFPREG * sizeof(double), label)
+#define unsafe_copy_fpr_from_user(task, from, label) \
+ unsafe_copy_from_user((task)->thread.fp_state.fpr, from \
+ ELF_NFPREG * sizeof(double), label)
+
static inline unsigned long
copy_fpr_to_user(void __user *to, struct task_struct *task)
{
@@ -115,6 +146,8 @@ copy_ckfpr_from_user(struct task_struct *task, void __user
*from)
#else
#define unsafe_copy_fpr_to_user(to, task, label) do { } while (0)
+#define unsafe_copy_fpr_from_user(task, from, label) do { } while (0)
+
static inline unsigned long
copy_fpr_to_user(void __user *to, struct task_struct *task)
{
--
2.29.0
Re: [PATCH v1 4/4] powernv/memtrace: don't abuse memory hot(un)plug infrastructure for memory allocations
David Hildenbrand writes:
> Let's use alloc_contig_pages() for allocating memory and remove the
> linear mapping manually via arch_remove_linear_mapping(). Mark all pages
> PG_offline, such that they will definitely not get touched - e.g.,
> when hibernating. When freeing memory, try to revert what we did.
>
> The original idea was discussed in:
> https://lkml.kernel.org/r/48340e96-7e6b-736f-9e23-d3111b915...@redhat.com
>
> This is similar to CONFIG_DEBUG_PAGEALLOC handling on other
> architectures, whereby only single pages are unmapped from the linear
> mapping. Let's mimic what memory hot(un)plug would do with the linear
> mapping.
>
> We now need MEMORY_HOTPLUG and CONTIG_ALLOC as dependencies.
>
> Simple test under QEMU TCG (10GB RAM, single NUMA node):
>
> sh-5.0# mount -t debugfs none /sys/kernel/debug/
> sh-5.0# cat /sys/devices/system/memory/block_size_bytes
> 4000
> sh-5.0# echo 0x4000 > /sys/kernel/debug/powerpc/memtrace/enable
> [ 71.052836][ T356] memtrace: Allocated trace memory on node 0 at
> 0x8000
> sh-5.0# echo 0x8000 > /sys/kernel/debug/powerpc/memtrace/enable
> [ 75.424302][ T356] radix-mmu: Mapped
> 0x8000-0xc000 with 64.0 KiB pages
> [ 75.430549][ T356] memtrace: Freed trace memory back on node 0
> [ 75.604520][ T356] memtrace: Allocated trace memory on node 0 at
> 0x8000
> sh-5.0# echo 0x1 > /sys/kernel/debug/powerpc/memtrace/enable
> [ 80.418835][ T356] radix-mmu: Mapped
> 0x8000-0x0001 with 64.0 KiB pages
> [ 80.430493][ T356] memtrace: Freed trace memory back on node 0
> [ 80.433882][ T356] memtrace: Failed to allocate trace memory on node 0
> sh-5.0# echo 0x4000 > /sys/kernel/debug/powerpc/memtrace/enable
> [ 91.920158][ T356] memtrace: Allocated trace memory on node 0 at
> 0x8000
I gave this a quick spin on a real machine, seems to work OK.
I don't have the actual memtrace tools setup to do an actual trace, will
try and get someone to test that also.
One observation is that previously the memory was zeroed when enabling
the memtrace, whereas now it's not.
eg, before:
# hexdump -C /sys/kernel/debug/powerpc/memtrace//trace
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ||
*
1000
whereas after:
# hexdump -C /sys/kernel/debug/powerpc/memtrace//trace
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ||
*
0080 e0 fd 43 00 00 00 00 00 e0 fd 43 00 00 00 00 00 |..C...C.|
0090 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ||
*
0830 98 bf 39 00 00 00 00 00 98 bf 39 00 00 00 00 00 |..9...9.|
0840 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ||
*
08a0 b0 c8 47 00 00 00 00 00 b0 c8 47 00 00 00 00 00 |..G...G.|
08b0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ||
...
0f70 78 53 49 7d 00 00 29 2e 88 00 92 41 01 00 49 39 |xSI}..)A..I9|
0f80 b4 07 4a 7d 28 f8 00 7d 00 48 08 7c 0c 00 c2 40 |..J}(..}.H.|...@|
0f90 2d f9 40 7d f0 ff c2 40 b4 07 0a 7d 00 48 8a 7f |-.@}...@...}.H..|
0fa0 70 fe 9e 41 cc ff ff 4b 00 00 00 60 00 00 00 60 |p..A...K...`...`|
0fb0 01 00 00 48 00 00 00 60 00 00 a3 2f 0c fd 9e 40 |...H...`.../...@|
0fc0 00 00 a2 3c 00 00 a5 e8 00 00 62 3c 00 00 63 e8 |...<..b<..c.|
0fd0 01 00 20 39 83 02 80 38 00 00 3c 99 01 00 00 48 |.. 9...8..
Re: [PATCH 31/36] powerpc: asm: hvconsole: Move 'hvc_vio_init_early's prototype to shared location
Lee Jones writes: > Fixes the following W=1 kernel build warning(s): > > drivers/tty/hvc/hvc_vio.c:385:13: warning: no previous prototype for > ‘hvc_vio_init_early’ [-Wmissing-prototypes] > 385 | void __init hvc_vio_init_early(void) > | ^~ > > Cc: Michael Ellerman > Cc: Benjamin Herrenschmidt > Cc: Paul Mackerras > Cc: linuxppc-dev@lists.ozlabs.org > Signed-off-by: Lee Jones > --- > arch/powerpc/include/asm/hvconsole.h | 3 +++ > arch/powerpc/platforms/pseries/pseries.h | 3 --- > arch/powerpc/platforms/pseries/setup.c | 1 + > 3 files changed, 4 insertions(+), 3 deletions(-) Acked-by: Michael Ellerman > diff --git a/arch/powerpc/include/asm/hvconsole.h > b/arch/powerpc/include/asm/hvconsole.h > index 999ed5ac90531..936a1ee1ac786 100644 > --- a/arch/powerpc/include/asm/hvconsole.h > +++ b/arch/powerpc/include/asm/hvconsole.h > @@ -24,5 +24,8 @@ > extern int hvc_get_chars(uint32_t vtermno, char *buf, int count); > extern int hvc_put_chars(uint32_t vtermno, const char *buf, int count); > > +/* Provided by HVC VIO */ > +extern void hvc_vio_init_early(void); extern isn't needed, but don't feel you need to respin just to drop it. cheers
[Bug 209869] Kernel 5.10-rc1 fails to boot on a PowerMac G4 3,6 at an early stage
https://bugzilla.kernel.org/show_bug.cgi?id=209869 --- Comment #8 from Erhard F. (erhar...@mailbox.org) --- Created attachment 293501 --> https://bugzilla.kernel.org/attachment.cgi?id=293501&action=edit dmesg (5.9.3 + KASAN, PowerMac G4 DP) -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 209869] Kernel 5.10-rc1 fails to boot on a PowerMac G4 3,6 at an early stage
https://bugzilla.kernel.org/show_bug.cgi?id=209869 --- Comment #7 from Erhard F. (erhar...@mailbox.org) --- Created attachment 293499 --> https://bugzilla.kernel.org/attachment.cgi?id=293499&action=edit kernel .config (5.9.3 + KASAN, PowerMac G4 DP) -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 209869] Kernel 5.10-rc1 fails to boot on a PowerMac G4 3,6 at an early stage
https://bugzilla.kernel.org/show_bug.cgi?id=209869 --- Comment #6 from Erhard F. (erhar...@mailbox.org) --- Created attachment 293497 --> https://bugzilla.kernel.org/attachment.cgi?id=293497&action=edit dmesg (5.10-rc1 + patch reverted + KASAN, PowerMac G4 DP) -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 209869] Kernel 5.10-rc1 fails to boot on a PowerMac G4 3,6 at an early stage
https://bugzilla.kernel.org/show_bug.cgi?id=209869 --- Comment #5 from Erhard F. (erhar...@mailbox.org) --- Created attachment 293495 --> https://bugzilla.kernel.org/attachment.cgi?id=293495&action=edit kernel .config (5.10-rc1 + KASAN, PowerMac G4 DP) -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 209869] Kernel 5.10-rc1 fails to boot on a PowerMac G4 3,6 at an early stage
https://bugzilla.kernel.org/show_bug.cgi?id=209869 --- Comment #4 from Erhard F. (erhar...@mailbox.org) --- (In reply to Christophe Leroy from comment #3) > Could you test whether CONFIG_KASAN works: > - on 5.10-rc1 with that commit reverted ? > - on 5.9 ? KASAN works in both cases. I'll attach kernel dmesg and config. -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH 33/36] tty: hvc: hvc_opal: Staticify function invoked by reference
Fixes the following W=1 kernel build warning(s):
drivers/tty/hvc/hvc_opal.c:106:6: warning: no previous prototype for
‘hvc_opal_hvsi_hangup’ [-Wmissing-prototypes]
Cc: Michael Ellerman
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Greg Kroah-Hartman
Cc: Jiri Slaby
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Lee Jones
---
drivers/tty/hvc/hvc_opal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/hvc/hvc_opal.c b/drivers/tty/hvc/hvc_opal.c
index c66412566efce..056ae21a51214 100644
--- a/drivers/tty/hvc/hvc_opal.c
+++ b/drivers/tty/hvc/hvc_opal.c
@@ -103,7 +103,7 @@ static void hvc_opal_hvsi_close(struct hvc_struct *hp, int
data)
notifier_del_irq(hp, data);
}
-void hvc_opal_hvsi_hangup(struct hvc_struct *hp, int data)
+static void hvc_opal_hvsi_hangup(struct hvc_struct *hp, int data)
{
struct hvc_opal_priv *pv = hvc_opal_privs[hp->vtermno];
--
2.25.1
[PATCH 34/36] tty: serial: pmac_zilog: Make disposable variable __always_unused
Fixes the following W=1 kernel build warning(s):
drivers/tty/serial/pmac_zilog.h:365:58: warning: variable ‘garbage’ set but
not used [-Wunused-but-set-variable]
Cc: Greg Kroah-Hartman
Cc: Jiri Slaby
Cc: Michael Ellerman
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: linux-ser...@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Lee Jones
---
drivers/tty/serial/pmac_zilog.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/pmac_zilog.h b/drivers/tty/serial/pmac_zilog.h
index bb874e76810e0..968aec7c1cf82 100644
--- a/drivers/tty/serial/pmac_zilog.h
+++ b/drivers/tty/serial/pmac_zilog.h
@@ -362,7 +362,7 @@ static inline void zssync(struct uart_pmac_port *port)
/* Misc macros */
#define ZS_CLEARERR(port)(write_zsreg(port, 0, ERR_RES))
-#define ZS_CLEARFIFO(port) do { volatile unsigned char garbage; \
+#define ZS_CLEARFIFO(port) do { volatile unsigned char __always_unused
garbage; \
garbage = read_zsdata(port); \
garbage = read_zsdata(port); \
garbage = read_zsdata(port); \
--
2.25.1
[PATCH 32/36] tty: hvc: hvc_vio: Staticify function invoked only by reference
Fixes the following W=1 kernel build warning(s):
drivers/tty/hvc/hvc_vio.c:181:6: warning: no previous prototype for
‘hvterm_hvsi_hangup’ [-Wmissing-prototypes]
181 | void hvterm_hvsi_hangup(struct hvc_struct *hp, int data)
| ^~
Cc: Greg Kroah-Hartman
Cc: Jiri Slaby
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Lee Jones
---
drivers/tty/hvc/hvc_vio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/hvc/hvc_vio.c b/drivers/tty/hvc/hvc_vio.c
index 7af54d6ed5b84..798f27f40cc2d 100644
--- a/drivers/tty/hvc/hvc_vio.c
+++ b/drivers/tty/hvc/hvc_vio.c
@@ -178,7 +178,7 @@ static void hvterm_hvsi_close(struct hvc_struct *hp, int
data)
notifier_del_irq(hp, data);
}
-void hvterm_hvsi_hangup(struct hvc_struct *hp, int data)
+static void hvterm_hvsi_hangup(struct hvc_struct *hp, int data)
{
struct hvterm_priv *pv = hvterm_privs[hp->vtermno];
--
2.25.1
[PATCH 31/36] powerpc: asm: hvconsole: Move 'hvc_vio_init_early's prototype to shared location
Fixes the following W=1 kernel build warning(s): drivers/tty/hvc/hvc_vio.c:385:13: warning: no previous prototype for ‘hvc_vio_init_early’ [-Wmissing-prototypes] 385 | void __init hvc_vio_init_early(void) | ^~ Cc: Michael Ellerman Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: linuxppc-dev@lists.ozlabs.org Signed-off-by: Lee Jones --- arch/powerpc/include/asm/hvconsole.h | 3 +++ arch/powerpc/platforms/pseries/pseries.h | 3 --- arch/powerpc/platforms/pseries/setup.c | 1 + 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/include/asm/hvconsole.h b/arch/powerpc/include/asm/hvconsole.h index 999ed5ac90531..936a1ee1ac786 100644 --- a/arch/powerpc/include/asm/hvconsole.h +++ b/arch/powerpc/include/asm/hvconsole.h @@ -24,5 +24,8 @@ extern int hvc_get_chars(uint32_t vtermno, char *buf, int count); extern int hvc_put_chars(uint32_t vtermno, const char *buf, int count); +/* Provided by HVC VIO */ +extern void hvc_vio_init_early(void); + #endif /* __KERNEL__ */ #endif /* _PPC64_HVCONSOLE_H */ diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h index 13fa370a87e4e..7be5b054dfc36 100644 --- a/arch/powerpc/platforms/pseries/pseries.h +++ b/arch/powerpc/platforms/pseries/pseries.h @@ -43,9 +43,6 @@ extern void pSeries_final_fixup(void); /* Poweron flag used for enabling auto ups restart */ extern unsigned long rtas_poweron_auto; -/* Provided by HVC VIO */ -extern void hvc_vio_init_early(void); - /* Dynamic logical Partitioning/Mobility */ extern void dlpar_free_cc_nodes(struct device_node *); extern void dlpar_free_cc_property(struct property *); diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c index 633c45ec406da..6999b83f06612 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -71,6 +71,7 @@ #include #include #include +#include #include "pseries.h" #include "../../../../drivers/pci/pci.h" -- 2.25.1
[PATCH 00/36] Rid W=1 issues from TTY
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 (36): tty: serdev: core: Remove unused variable 'dummy' tty: serdev: core: Provide missing description for 'owner' tty: tty_baudrate: Add missing description for 'tty' tty: tty_io: Move 'tty_sysctl_init's prototype to shared space tty: tty_buffer: Add missing description for 'limit' tty: tty_port: Demote obvious abuse of kernel-doc formatting tty: tty_jobctrl: Add missing function parameter descriptions tty: tty_ldisc: Fix some kernel-doc related misdemeanours tty: vt: consolemap: Demote weakly documented function header tty: n_tty: Add 2 missing parameter descriptions tty: serial: jsm: jsm_cls: Remove unused variable 'discard' tty: tty_io: Fix some kernel-doc issues tty: serial: 8250: 8250_port: Staticify functions referenced by pointers tty: serial: 8250: serial_cs: Remove unused/unchecked variable 'err' tty: tty_audit: Demote non-conformant kernel-doc headers tty: pty: Provide descriptions for the 'file' parameters tty: serial: amba-pl011: Mark 'sbsa_uart_acpi_match' as __maybe_unused tty: n_gsm: Demote obvious abuse of kernel-doc and supply other missing docss tty: serial: lpc32xx_hs: Remove unused variable 'tmp' tty: serial: msm_serial: Remove set but unused variable 'status' tty: serial: ifx6x60: Fix function documentation headers tty: serial: xilinx_uartps: Supply description for missing member 'cts_override' tty: synclink_gt: Demote one kernel-doc header and repair another tty: serial: serial-tegra: Struct headers should start with 'struct ' tty: serial: sifive: Struct headers should start with 'struct ' tty: synclinkmp: Add missing description for function param 'txqueue' tty: synclinkmp: Mark never checked 'readval' as __always_unused tty: serial: stm32-usart: Remove set but unused 'cookie' variables tty: tty_ldisc: Supply missing description for 'tty_ldisc_get's 'tty' param tty: serial: serial-tegra: Provide some missing struct member descriptions powerpc: asm: hvconsole: Move 'hvc_vio_init_early's prototype to shared location tty: hvc: hvc_vio: Staticify function invoked only by reference tty: hvc: hvc_opal: Staticify function invoked by reference tty: serial: pmac_zilog: Make disposable variable __always_unused tty: synclink: Mark disposable variables as __always_unused tty: synclink: Provide missing description for 'hdlcdev_tx_timeout's 'txqueue' param arch/powerpc/include/asm/hvconsole.h | 3 +++ arch/powerpc/platforms/pseries/pseries.h | 3 --- arch/powerpc/platforms/pseries/setup.c | 1 + drivers/tty/hvc/hvc_opal.c | 2 +- drivers/tty/hvc/hvc_vio.c| 2 +- drivers/tty/n_gsm.c | 4 +++- drivers/tty/n_tty.c | 2 ++ drivers/tty/pty.c| 2 ++ drivers/tty/serdev/core.c| 6 ++ drivers/tty/serial/8250/8250_port.c | 4 ++-- drivers/tty/serial/8250/serial_cs.c | 9 +++-- drivers/tty/serial/amba-pl011.c | 2 +- drivers/tty/serial/ifx6x60.c | 3 ++- drivers/tty/serial/jsm/jsm_cls.c | 4 +--- drivers/tty/serial/lpc32xx_hs.c | 3 +-- drivers/tty/serial/msm_serial.c | 3 +-- drivers/tty/serial/pmac_zilog.h | 2 +- drivers/tty/serial/serial-tegra.c| 7 ++- drivers/tty/serial/sifive.c | 3 ++- drivers/tty/serial/stm32-usart.c | 6 ++ drivers/tty/serial/xilinx_uartps.c | 1 + drivers/tty/synclink.c | 5 +++-- drivers/tty/synclink_gt.c| 3 ++- drivers/tty/synclinkmp.c | 3 ++- drivers/tty/tty_audit.c | 12 ++-- drivers/tty/tty_baudrate.c | 1 + drivers/tty/tty_buffer.c | 1 + drivers/tty/tty_io.c | 12 +++- drivers/tty/tty_jobctrl.c| 3 +++ drivers/tty/tty_ldisc.c | 11 ++- drivers/tty/tty_port.c | 4 ++-- drivers/tty/vt/consolemap.c | 3 +-- include/linux/tty.h | 1 + 33 files changed, 73 insertions(+), 58 deletions(-) Cc: Alexandre Torgue Cc: "Andrew J. Kroll" Cc: Andrew Morton Cc: Andy Gross Cc: Benjamin Herrenschmidt Cc: Bill Hawes Cc: Bjorn Andersson Cc: "Christian König" Cc: Colin Ian King Cc: "C. Scott Ananian" Cc: "David A. Hinds" Cc: dri-de...@lists.freedesktop.org Cc: Filip Aben Cc: Gerald Baeza Cc: Greg Kroah-Hartman Cc: Jakub Jelinek Cc: Jan Dumon Cc: Jiri Slaby Cc: Jonathan Hunter Cc: Joseph Barrow Cc: -- Cc: Kevin Wells Cc: Laxman Dewangan Cc: linaro-mm-...@lists.linaro.org Cc: linux-arm-...@vger.kernel.org Cc: linux-me...@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-ri...@lists.infradead.org Cc: linux-ser...@vger.
Re: [PATCH 11/11 v2.2] ftrace: Add recording of functions that caused recursion
On Tue, 3 Nov 2020 15:10:43 +0100 Petr Mladek 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. The problem was that I don't want something in the recording to get traced, and that detect the recursion, which would then call the recording, which would also be traced and detect the recursion and that would call the recording, which would also be traced and detect the recursion and that would call the recording, which (BOOM!) Anyway, I decided to simply use another bit to protect against that. New patch: From a1f2aae996506169f2561986656f898d067d398b Mon Sep 17 00:00:00 2001 From: "Steven Rostedt (VMware)" 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. Cc: Jonathan Corbet Cc: Guo Ren Cc: "James E.J. Bottomley" Cc: Helge Deller Cc: Michael Ellerman Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Heiko Carstens Cc: Vasily Gorbik Cc: Christian Borntraeger Cc: Thomas Gleixner Cc: Borislav Petkov Cc: x...@kernel.org Cc: "H. Peter Anvin" Cc: Kees Cook Cc: Anton Vorontsov Cc: Colin Cross Cc: Tony Luck Cc: Josh Poimboeuf Cc: Jiri Kosina Cc: Miroslav Benes Cc: Petr Mladek Cc: Joe Lawrence Cc: Kamalesh Babulal Cc: Mauro Carvalho Chehab Cc: Sebastian Andrzej Siewior Cc: linux-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: linux-c...@vger.kernel.org Cc: linux-par...@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-s...@vger.kernel.org Cc: live-patch...@vger.kernel.org Signed-off-by: Steven Rostedt (VMware) --- Documentation/trace/ftrace-uses.rst | 6 +- arch/csky/kernel/probes/ftrace.c | 2 +- arch/parisc/kernel/ftrace.c | 2 +- arch/powerpc/kernel/kprobes-ftrace.c | 2 +- arch/s390/kernel/ftrace.c | 2 +- arch/x86/kernel/kprobes/ftrace.c | 2 +- fs/pstore/ftrace.c| 2 +- include/linux/trace_recursion.h | 29 +++- kernel/livepatch/patch.c | 2 +- kernel/trace/Kconfig | 25 +++ kernel/trace/Makefile | 1 + kernel/trace/ftrace.c | 4 +- kernel/trace/trace_event_perf.c | 2 +- kernel/trace/trace_functions.c| 2 +- kernel/trace/trace_output.c | 6 +- kernel/trace/trace_output.h | 1 + kernel/trace/trace_recursion_record.c | 236 ++ 17 files changed, 306 insertions(+), 20 deletions(-) create mode 100644 kernel/trace/trace_recursion_record.c diff --git a/Documentation/trace/ftrace-uses.rst b/Documentation/trace/ftrace-uses.rst index 86cd14b8e126..5981d5691745 100644 --- a/Documentation/trace/ftrace-uses.rst +++ b/Documentation/trace/ftrace-uses.rst @@ -118,7 +118,7 @@ can help in this regard. If you start your code with: int bit; - bit = ftrace_test_recursion_trylock(); + bit = ftrace_test_recursion_trylock(ip, parent_ip); if (bit < 0) return; @@ -130,7 +130,9 @@ The code in between will be safe to use, even if it ends up calling a function that the callback is tracing. Note, on success, ftrace_test_recursion_trylock() will disable preemption, and the ftrace_test_recursion_unlock() will enable it again (if it was previously -enabled). +enabled). The instruction pointer (ip) and its parent (parent_ip) is passed to +ftrace_test_recursion_trylock() to record where the recursion happened +(if CONFIG_FTRACE_RECORD_RECURSION is set). Alternatively, if the FTRACE_OPS_FL_RECURSION flag is set on the ftrace_ops (as explained below), then a helper trampoline will be used to test diff --git a/arch/csky/kernel/probes/ftrace.c b/arch/csky/kernel/probes/ftrace.c index 5eb2604fdf71..f30b179924ef 100644 --- a/arch/csky/kernel/probes/ftrace.c +++ b/arch/csky/kernel/probes/ftrace.c @@ -18,7 +18,7 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip, struct kprobe *p; struct kprobe_ctlblk *kcb; - bit = ftrace_test_recursion_trylock(); + bit = ftrace_test_recursion_trylock(ip, parent_ip); if (bit < 0) return; diff --git a/arch/parisc/kernel/ftrace.c b/arch/parisc/kernel/ftrace.c index 13d85042810a..1c5d3732bda2 100644 --- a/arch/parisc/kernel/ftrace.c +++ b/arch/parisc/kernel/ftrace.c @@ -2
[PATCH 05/11 v2.1] kprobes/ftrace: Add recursion protection to the ftrace callback
From: "Steven Rostedt (VMware)"
If a ftrace callback does not supply its own recursion protection and
does not set the RECURSION_SAFE flag in its ftrace_ops, then ftrace will
make a helper trampoline to do so before calling the callback instead of
just calling the callback directly.
The default for ftrace_ops is going to change. It will expect that handlers
provide their own recursion protection, unless its ftrace_ops states
otherwise.
Link: https://lkml.kernel.org/r/20201028115613.140212...@goodmis.org
Cc: Andrew Morton
Cc: Masami Hiramatsu
Cc: Guo Ren
Cc: "James E.J. Bottomley"
Cc: Helge Deller
Cc: Michael Ellerman
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Heiko Carstens
Cc: Vasily Gorbik
Cc: Christian Borntraeger
Cc: Thomas Gleixner
Cc: Borislav Petkov
Cc: x...@kernel.org
Cc: "H. Peter Anvin"
Cc: "Naveen N. Rao"
Cc: Anil S Keshavamurthy
Cc: "David S. Miller"
Cc: linux-c...@vger.kernel.org
Cc: linux-par...@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-s...@vger.kernel.org
Signed-off-by: Steven Rostedt (VMware)
---
arch/csky/kernel/probes/ftrace.c | 12 ++--
arch/parisc/kernel/ftrace.c | 16 +---
arch/powerpc/kernel/kprobes-ftrace.c | 11 ++-
arch/s390/kernel/ftrace.c| 16 +---
arch/x86/kernel/kprobes/ftrace.c | 12 ++--
5 files changed, 56 insertions(+), 11 deletions(-)
diff --git a/arch/csky/kernel/probes/ftrace.c b/arch/csky/kernel/probes/ftrace.c
index 5264763d05be..5eb2604fdf71 100644
--- a/arch/csky/kernel/probes/ftrace.c
+++ b/arch/csky/kernel/probes/ftrace.c
@@ -13,16 +13,21 @@ int arch_check_ftrace_location(struct kprobe *p)
void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
struct ftrace_ops *ops, struct pt_regs *regs)
{
+ int bit;
bool lr_saver = false;
struct kprobe *p;
struct kprobe_ctlblk *kcb;
- /* Preempt is disabled by ftrace */
+ bit = ftrace_test_recursion_trylock();
+ if (bit < 0)
+ return;
+
+ preempt_disable_notrace();
p = get_kprobe((kprobe_opcode_t *)ip);
if (!p) {
p = get_kprobe((kprobe_opcode_t *)(ip - MCOUNT_INSN_SIZE));
if (unlikely(!p) || kprobe_disabled(p))
- return;
+ goto out;
lr_saver = true;
}
@@ -56,6 +61,9 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long
parent_ip,
*/
__this_cpu_write(current_kprobe, NULL);
}
+out:
+ preempt_enable_notrace();
+ ftrace_test_recursion_unlock(bit);
}
NOKPROBE_SYMBOL(kprobe_ftrace_handler);
diff --git a/arch/parisc/kernel/ftrace.c b/arch/parisc/kernel/ftrace.c
index 63e3ecb9da81..13d85042810a 100644
--- a/arch/parisc/kernel/ftrace.c
+++ b/arch/parisc/kernel/ftrace.c
@@ -207,14 +207,21 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned
long parent_ip,
struct ftrace_ops *ops, struct pt_regs *regs)
{
struct kprobe_ctlblk *kcb;
- struct kprobe *p = get_kprobe((kprobe_opcode_t *)ip);
+ struct kprobe *p;
+ int bit;
- if (unlikely(!p) || kprobe_disabled(p))
+ bit = ftrace_test_recursion_trylock();
+ if (bit < 0)
return;
+ preempt_disable_notrace();
+ p = get_kprobe((kprobe_opcode_t *)ip);
+ if (unlikely(!p) || kprobe_disabled(p))
+ goto out;
+
if (kprobe_running()) {
kprobes_inc_nmissed_count(p);
- return;
+ goto out;
}
__this_cpu_write(current_kprobe, p);
@@ -235,6 +242,9 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long
parent_ip,
}
}
__this_cpu_write(current_kprobe, NULL);
+out:
+ preempt_enable_notrace();
+ ftrace_test_recursion_unlock(bit);
}
NOKPROBE_SYMBOL(kprobe_ftrace_handler);
diff --git a/arch/powerpc/kernel/kprobes-ftrace.c
b/arch/powerpc/kernel/kprobes-ftrace.c
index 972cb28174b2..5df8d50c65ae 100644
--- a/arch/powerpc/kernel/kprobes-ftrace.c
+++ b/arch/powerpc/kernel/kprobes-ftrace.c
@@ -18,10 +18,16 @@ void kprobe_ftrace_handler(unsigned long nip, unsigned long
parent_nip,
{
struct kprobe *p;
struct kprobe_ctlblk *kcb;
+ int bit;
+ bit = ftrace_test_recursion_trylock();
+ if (bit < 0)
+ return;
+
+ preempt_disable_notrace();
p = get_kprobe((kprobe_opcode_t *)nip);
if (unlikely(!p) || kprobe_disabled(p))
- return;
+ goto out;
kcb = get_kprobe_ctlblk();
if (kprobe_running()) {
@@ -52,6 +58,9 @@ void kprobe_ftrace_handler(unsigned long nip, unsigned long
parent_nip,
*/
__this_cpu_write(current_kprobe, NULL);
}
+out:
+ preempt_enable_notrace();
+ ftrace_test_recursion_unlock(bit);
}
Re: Kernel 5.10-rc1 not mounting NAND flash (Bisected to d7157ff49a5b ("mtd: rawnand: Use the ECC framework user input parsing bits"))
Hi Miquel,
Le 04/11/2020 à 18:38, Miquel Raynal a écrit :
Hi Christophe,
Christophe Leroy wrote on Wed, 04 Nov
2020 18:33:53 +0100:
Hi Miquel,
I'm unable to boot 5.10-rc1 on my boards. I get the following error:
[4.125811] nand: device found, Manufacturer ID: 0xad, Chip ID: 0x76
[4.131992] nand: Hynix NAND 64MiB 3,3V 8-bit
[4.136173] nand: 64 MiB, SLC, erase size: 16 KiB, page size: 512, OOB size:
16
[4.143534] [ cut here ]
[4.147934] Unsupported ECC algorithm!
[4.152142] WARNING: CPU: 0 PID: 1 at drivers/mtd/nand/raw/nand_base.c:5244
nand_scan_with_ids+0x1260/0x1640
...
[4.332052] ---[ end trace e3a36f62cae4ac56 ]---
[4.336882] gpio-nand: probe of c000.nand failed with error -22
Bisected to commit d7157ff49a5b ("mtd: rawnand: Use the ECC framework user input
parsing bits")
My first impression is that with that change, the value set in chip->ecc.algo
by gpio_nand_probe() in drivers/mtd/nand/raw/gpio.c gets overwritten in
rawnand_dt_init()
The following change fixes the problem, though I'm not sure it is the right
fix. Can you have a look ?
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 1f0d542d5923..aa74797cf2da 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5032,7 +5032,8 @@ static int rawnand_dt_init(struct nand_chip *chip)
chip->ecc.engine_type = nand->ecc.defaults.engine_type;
chip->ecc.placement = nand->ecc.user_conf.placement;
- chip->ecc.algo = nand->ecc.user_conf.algo;
+ if (chip->ecc.algo == NAND_ECC_ALGO_UNKNOWN)
+ chip->ecc.algo = nand->ecc.user_conf.algo;
chip->ecc.strength = nand->ecc.user_conf.strength;
chip->ecc.size = nand->ecc.user_conf.step_size;
---
Thanks
Christophe
Sorry for introducing this issue, I didn't had the time to send the
Fixes PR yet but I think this issue has been solved already. Could
you please try with a recent linux-next?
Sorry, same problem with "Linux version 5.10.0-rc2-next-20201104"
Christophe
Re: [PATCH v4 3/4] arch, mm: restore dependency of __kernel_map_pages() of DEBUG_PAGEALLOC
On 11/3/20 5:20 PM, Mike Rapoport wrote: From: Mike Rapoport Subject should have "on DEBUG_PAGEALLOC" ? 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 Acked-by: David Hildenbrand Acked-by: Kirill A. Shutemov --- 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, I don't understand these two hunks. Previous patch calls this for hibernation when CONFIG_ARCH_HAS_SET_DIRECT_MAP, which is true for arm64. Why suddenly this starts to depend on debug_pagealloc_enabled()?
Re: [PATCH 12/12] net: ethernet: ibm: ibmvnic: Fix some kernel-doc issues
On 2020-11-04 01:06, Lee Jones wrote: Fixes the following W=1 kernel build warning(s): from drivers/net/ethernet/ibm/ibmvnic.c:35: inlined from ‘handle_vpd_rsp’ at drivers/net/ethernet/ibm/ibmvnic.c:4124:3: drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or member 'hdr_data' not described in 'build_hdr_data' drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Excess function parameter 'tot_len' description in 'build_hdr_data' drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 'hdr_data' not described in 'create_hdr_descs' drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Excess function parameter 'data' description in 'create_hdr_descs' drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Function parameter or member 'txbuff' not described in 'build_hdr_descs_arr' drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Excess function parameter 'skb' description in 'build_hdr_descs_arr' drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Excess function parameter 'subcrq' description in 'build_hdr_descs_arr' Cc: Michael Ellerman Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Dany Madden Cc: Lijun Pan Cc: Sukadev Bhattiprolu Cc: "David S. Miller" Cc: Jakub Kicinski Cc: Santiago Leon Cc: Thomas Falcon Cc: John Allen Cc: linuxppc-dev@lists.ozlabs.org Cc: net...@vger.kernel.org Signed-off-by: Lee Jones Reviewed-by: Dany Madden Thanks, Lee. Dany --- drivers/net/ethernet/ibm/ibmvnic.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index b30e1f5784bad..08dab7a94b7ea 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1360,7 +1360,7 @@ static int ibmvnic_close(struct net_device *netdev) * @hdr_field: bitfield determining needed headers * @skb: socket buffer * @hdr_len: array of header lengths - * @tot_len: total length of data + * @hdr_data: buffer to write the header to * * Reads hdr_field to determine which headers are needed by firmware. * Builds a buffer containing these headers. Saves individual header @@ -1418,7 +1418,7 @@ static int build_hdr_data(u8 hdr_field, struct sk_buff *skb, /** * create_hdr_descs - create header and header extension descriptors * @hdr_field: bitfield determining needed headers - * @data: buffer containing header data + * @hdr_data: buffer containing header data * @len: length of data buffer * @hdr_len: array of individual header lengths * @scrq_arr: descriptor array @@ -1469,9 +1469,8 @@ static int create_hdr_descs(u8 hdr_field, u8 *hdr_data, int len, int *hdr_len, /** * build_hdr_descs_arr - build a header descriptor array - * @skb: socket buffer + * @txbuff: tx buffer * @num_entries: number of descriptors to be sent - * @subcrq: first TX descriptor * @hdr_field: bit field determining which headers will be sent * * This function will build a TX descriptor array with applicable
Re: [PATCH 09/12] net: ethernet: ibm: ibmvnic: Fix some kernel-doc misdemeanours
On 2020-11-04 05:38, Andrew Lunn wrote: On Wed, Nov 04, 2020 at 09:06:07AM +, Lee Jones wrote: Fixes the following W=1 kernel build warning(s): from drivers/net/ethernet/ibm/ibmvnic.c:35: inlined from ‘handle_vpd_rsp’ at drivers/net/ethernet/ibm/ibmvnic.c:4124:3: drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or member 'hdr_field' not described in 'build_hdr_data' drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or member 'skb' not described in 'build_hdr_data' drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or member 'hdr_len' not described in 'build_hdr_data' drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or member 'hdr_data' not described in 'build_hdr_data' drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 'hdr_field' not described in 'create_hdr_descs' drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 'hdr_data' not described in 'create_hdr_descs' drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 'len' not described in 'create_hdr_descs' drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 'hdr_len' not described in 'create_hdr_descs' drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 'scrq_arr' not described in 'create_hdr_descs' drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Function parameter or member 'txbuff' not described in 'build_hdr_descs_arr' drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Function parameter or member 'num_entries' not described in 'build_hdr_descs_arr' drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Function parameter or member 'hdr_field' not described in 'build_hdr_descs_arr' drivers/net/ethernet/ibm/ibmvnic.c:1832: warning: Function parameter or member 'adapter' not described in 'do_change_param_reset' drivers/net/ethernet/ibm/ibmvnic.c:1832: warning: Function parameter or member 'rwi' not described in 'do_change_param_reset' drivers/net/ethernet/ibm/ibmvnic.c:1832: warning: Function parameter or member 'reset_state' not described in 'do_change_param_reset' drivers/net/ethernet/ibm/ibmvnic.c:1911: warning: Function parameter or member 'adapter' not described in 'do_reset' drivers/net/ethernet/ibm/ibmvnic.c:1911: warning: Function parameter or member 'rwi' not described in 'do_reset' drivers/net/ethernet/ibm/ibmvnic.c:1911: warning: Function parameter or member 'reset_state' not described in 'do_reset' Cc: Dany Madden Cc: Lijun Pan Cc: Sukadev Bhattiprolu Cc: Michael Ellerman Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: "David S. Miller" Cc: Jakub Kicinski Cc: Santiago Leon Cc: Thomas Falcon Cc: John Allen Cc: net...@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Signed-off-by: Lee Jones Reviewed-by: Andrew Lunn Reviewed-by: Dany Madden Thanks, Lee. Dany Andrew
Re: Kernel 5.10-rc1 not mounting NAND flash (Bisected to d7157ff49a5b ("mtd: rawnand: Use the ECC framework user input parsing bits"))
Hi Christophe,
Christophe Leroy wrote on Wed, 04 Nov
2020 18:33:53 +0100:
> Hi Miquel,
>
> I'm unable to boot 5.10-rc1 on my boards. I get the following error:
>
> [4.125811] nand: device found, Manufacturer ID: 0xad, Chip ID: 0x76
> [4.131992] nand: Hynix NAND 64MiB 3,3V 8-bit
> [4.136173] nand: 64 MiB, SLC, erase size: 16 KiB, page size: 512, OOB
> size: 16
> [4.143534] [ cut here ]
> [4.147934] Unsupported ECC algorithm!
> [4.152142] WARNING: CPU: 0 PID: 1 at
> drivers/mtd/nand/raw/nand_base.c:5244 nand_scan_with_ids+0x1260/0x1640
> ...
> [4.332052] ---[ end trace e3a36f62cae4ac56 ]---
> [4.336882] gpio-nand: probe of c000.nand failed with error -22
>
> Bisected to commit d7157ff49a5b ("mtd: rawnand: Use the ECC framework user
> input parsing bits")
>
> My first impression is that with that change, the value set in chip->ecc.algo
> by gpio_nand_probe() in drivers/mtd/nand/raw/gpio.c gets overwritten in
> rawnand_dt_init()
>
> The following change fixes the problem, though I'm not sure it is the right
> fix. Can you have a look ?
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c
> b/drivers/mtd/nand/raw/nand_base.c
> index 1f0d542d5923..aa74797cf2da 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -5032,7 +5032,8 @@ static int rawnand_dt_init(struct nand_chip *chip)
> chip->ecc.engine_type = nand->ecc.defaults.engine_type;
>
> chip->ecc.placement = nand->ecc.user_conf.placement;
> - chip->ecc.algo = nand->ecc.user_conf.algo;
> + if (chip->ecc.algo == NAND_ECC_ALGO_UNKNOWN)
> + chip->ecc.algo = nand->ecc.user_conf.algo;
> chip->ecc.strength = nand->ecc.user_conf.strength;
> chip->ecc.size = nand->ecc.user_conf.step_size;
>
> ---
>
> Thanks
> Christophe
Sorry for introducing this issue, I didn't had the time to send the
Fixes PR yet but I think this issue has been solved already. Could
you please try with a recent linux-next?
Thanks,
Miquèl
Re: [PATCH v4 2/4] PM: hibernate: make direct map manipulations more explicit
On 11/3/20 5:20 PM, Mike Rapoport wrote:
From: Mike Rapoport
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
Acked-by: Rafael J. Wysocki
Reviewed-by: David Hildenbrand
Acked-by: Kirill A. Shutemov
Acked-by: Vlastimil Babka
The bool param is a bit more acceptable here, being a private API. But if
debug_pagealloc_map_pages() becomes split, then it might be easier to split this
one too...
---
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);
}
}
Re: [PATCH v4 1/4] mm: introduce debug_pagealloc_map_pages() helper
On 11/3/20 5:20 PM, Mike Rapoport wrote:
From: Mike Rapoport
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
Reviewed-by: David Hildenbrand
Acked-by: Kirill A. Shutemov
Acked-by: Vlastimil Babka
But, the "enable" param is hideous. I would rather have map and unmap variants
(and just did the same split for page poisoning) and this seems to be a good
opportunity. If David didn't propose it already, I'm surprised ;)
---
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),
+
Kernel 5.10-rc1 not mounting NAND flash (Bisected to d7157ff49a5b ("mtd: rawnand: Use the ECC framework user input parsing bits"))
Hi Miquel,
I'm unable to boot 5.10-rc1 on my boards. I get the following error:
[4.125811] nand: device found, Manufacturer ID: 0xad, Chip ID: 0x76
[4.131992] nand: Hynix NAND 64MiB 3,3V 8-bit
[4.136173] nand: 64 MiB, SLC, erase size: 16 KiB, page size: 512,
OOB size: 16
[4.143534] [ cut here ]
[4.147934] Unsupported ECC algorithm!
[4.152142] WARNING: CPU: 0 PID: 1 at
drivers/mtd/nand/raw/nand_base.c:5244 nand_scan_with_ids+0x1260/0x1640
...
[4.332052] ---[ end trace e3a36f62cae4ac56 ]---
[4.336882] gpio-nand: probe of c000.nand failed with error -22
Bisected to commit d7157ff49a5b ("mtd: rawnand: Use the ECC framework
user input parsing bits")
My first impression is that with that change, the value set in chip->ecc.algo
by gpio_nand_probe() in drivers/mtd/nand/raw/gpio.c gets overwritten
in rawnand_dt_init()
The following change fixes the problem, though I'm not sure it is the
right fix. Can you have a look ?
diff --git a/drivers/mtd/nand/raw/nand_base.c
b/drivers/mtd/nand/raw/nand_base.c
index 1f0d542d5923..aa74797cf2da 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5032,7 +5032,8 @@ static int rawnand_dt_init(struct nand_chip *chip)
chip->ecc.engine_type = nand->ecc.defaults.engine_type;
chip->ecc.placement = nand->ecc.user_conf.placement;
- chip->ecc.algo = nand->ecc.user_conf.algo;
+ if (chip->ecc.algo == NAND_ECC_ALGO_UNKNOWN)
+ chip->ecc.algo = nand->ecc.user_conf.algo;
chip->ecc.strength = nand->ecc.user_conf.strength;
chip->ecc.size = nand->ecc.user_conf.step_size;
---
Thanks
Christophe
Re: [PATCH] powerpc/32s: Setup the early hash table at all time.
>>> To be sure we are not in front of a long lasting bug, could you try >>> CONFIG_KASAN=y on v5.9 ? >> >> Indeed it started to fail somewhere between v5.6 and v5.7. >> >> v5.7 fails early with few messages on the console with reboot, v5.8 and >> later hang right at bootloader. >> >> I'm bisecting now. > > (side note: I tried FB_OF=y instead of DRM_RADEON + DRM_FBDEV_* to speed > up bisection and it turns out in that configuration KASAN never worked, > down to commit 305d600123046, hanging right after bootloader or even > with invalid access in the bootloader itself). My bisection ended up nowhere (at net-next merge with 2k commits), and given the above failure with unrelated configuration change, I conclude that KASAN=y was always broken on this box.
[Bug 209869] Kernel 5.10-rc1 fails to boot on a PowerMac G4 3,6 at an early stage
https://bugzilla.kernel.org/show_bug.cgi?id=209869 --- Comment #3 from Christophe Leroy (christophe.le...@csgroup.eu) --- Ok, thanks for the verification. I'm still having hard time understand what the problem can be. That commit sets up at all time the early hash table that was previously only set up when CONFIG_KASAN was set. As far as I remember, you tried KASAN many times. Could you test whether CONFIG_KASAN works: - on 5.10-rc1 with that commit reverted ? - on 5.9 ? If CONFIG_KASAN is already broken on 5.9, are you able to find the last kernel version on which CONFIG_KASAN works ? Thanks Christophe -- You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH 10/12] net: ethernet: toshiba: ps3_gelic_net: Fix some kernel-doc misdemeanours
On Wed, Nov 04, 2020 at 09:06:08AM +, Lee Jones wrote: > Fixes the following W=1 kernel build warning(s): > > drivers/net/ethernet/toshiba/ps3_gelic_net.c:1107: warning: Function > parameter or member 'irq' not described in 'gelic_card_interrupt' > drivers/net/ethernet/toshiba/ps3_gelic_net.c:1107: warning: Function > parameter or member 'ptr' not described in 'gelic_card_interrupt' > drivers/net/ethernet/toshiba/ps3_gelic_net.c:1407: warning: Function > parameter or member 'txqueue' not described in 'gelic_net_tx_timeout' > drivers/net/ethernet/toshiba/ps3_gelic_net.c:1439: warning: Function > parameter or member 'napi' not described in 'gelic_ether_setup_netdev_ops' > drivers/net/ethernet/toshiba/ps3_gelic_net.c:1639: warning: Function > parameter or member 'dev' not described in 'ps3_gelic_driver_probe' > drivers/net/ethernet/toshiba/ps3_gelic_net.c:1795: warning: Function > parameter or member 'dev' not described in 'ps3_gelic_driver_remove' > > Cc: Geoff Levand > Cc: "David S. Miller" > Cc: Jakub Kicinski > Cc: Michael Ellerman > Cc: Benjamin Herrenschmidt > Cc: Paul Mackerras > Cc: Utz Bacher > Cc: Jens Osterkamp > Cc: net...@vger.kernel.org > Cc: linuxppc-dev@lists.ozlabs.org > Signed-off-by: Lee Jones Reviewed-by: Andrew Lunn Andrew
Re: [PATCH 09/12] net: ethernet: ibm: ibmvnic: Fix some kernel-doc misdemeanours
On Wed, Nov 04, 2020 at 09:06:07AM +, Lee Jones wrote: > Fixes the following W=1 kernel build warning(s): > > from drivers/net/ethernet/ibm/ibmvnic.c:35: > inlined from ‘handle_vpd_rsp’ at drivers/net/ethernet/ibm/ibmvnic.c:4124:3: > drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or > member 'hdr_field' not described in 'build_hdr_data' > drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or > member 'skb' not described in 'build_hdr_data' > drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or > member 'hdr_len' not described in 'build_hdr_data' > drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or > member 'hdr_data' not described in 'build_hdr_data' > drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or > member 'hdr_field' not described in 'create_hdr_descs' > drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or > member 'hdr_data' not described in 'create_hdr_descs' > drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or > member 'len' not described in 'create_hdr_descs' > drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or > member 'hdr_len' not described in 'create_hdr_descs' > drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or > member 'scrq_arr' not described in 'create_hdr_descs' > drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Function parameter or > member 'txbuff' not described in 'build_hdr_descs_arr' > drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Function parameter or > member 'num_entries' not described in 'build_hdr_descs_arr' > drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Function parameter or > member 'hdr_field' not described in 'build_hdr_descs_arr' > drivers/net/ethernet/ibm/ibmvnic.c:1832: warning: Function parameter or > member 'adapter' not described in 'do_change_param_reset' > drivers/net/ethernet/ibm/ibmvnic.c:1832: warning: Function parameter or > member 'rwi' not described in 'do_change_param_reset' > drivers/net/ethernet/ibm/ibmvnic.c:1832: warning: Function parameter or > member 'reset_state' not described in 'do_change_param_reset' > drivers/net/ethernet/ibm/ibmvnic.c:1911: warning: Function parameter or > member 'adapter' not described in 'do_reset' > drivers/net/ethernet/ibm/ibmvnic.c:1911: warning: Function parameter or > member 'rwi' not described in 'do_reset' > drivers/net/ethernet/ibm/ibmvnic.c:1911: warning: Function parameter or > member 'reset_state' not described in 'do_reset' > > Cc: Dany Madden > Cc: Lijun Pan > Cc: Sukadev Bhattiprolu > Cc: Michael Ellerman > Cc: Benjamin Herrenschmidt > Cc: Paul Mackerras > Cc: "David S. Miller" > Cc: Jakub Kicinski > Cc: Santiago Leon > Cc: Thomas Falcon > Cc: John Allen > Cc: net...@vger.kernel.org > Cc: linuxppc-dev@lists.ozlabs.org > Signed-off-by: Lee Jones Reviewed-by: Andrew Lunn Andrew
Re: [PATCH v1 3/4] powerpc/mm: remove linear mapping if __add_pages() fails in arch_add_memory()
On Thu, Oct 29, 2020 at 05:27:17PM +0100, David Hildenbrand wrote: > Let's revert what we did in case seomthing goes wrong and we return an > error. > > Cc: Michael Ellerman > Cc: Benjamin Herrenschmidt > Cc: Paul Mackerras > Cc: Rashmica Gupta > Cc: Andrew Morton > Cc: Mike Rapoport > Cc: Michal Hocko > Cc: Oscar Salvador > Cc: Wei Yang > Signed-off-by: David Hildenbrand Reviewed-by: Oscar Salvador > --- > arch/powerpc/mm/mem.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c > index 685028451dd2..69b3e8072261 100644 > --- a/arch/powerpc/mm/mem.c > +++ b/arch/powerpc/mm/mem.c > @@ -165,7 +165,10 @@ int __ref arch_add_memory(int nid, u64 start, u64 size, > rc = arch_create_linear_mapping(nid, start, size, params); > if (rc) > return rc; > - return __add_pages(nid, start_pfn, nr_pages, params); > + rc = __add_pages(nid, start_pfn, nr_pages, params); > + if (rc) > + arch_remove_linear_mapping(start, size); > + return rc; > } > > void __ref arch_remove_memory(int nid, u64 start, u64 size, > -- > 2.26.2 > -- Oscar Salvador SUSE L3
Re: [PATCH v1 3/4] powerpc/mm: remove linear mapping if __add_pages() fails in arch_add_memory()
On Wed, Nov 04, 2020 at 02:06:51PM +0200, Mike Rapoport wrote: > On Wed, Nov 04, 2020 at 10:50:07AM +0100, osalvador wrote: > > On Thu, Oct 29, 2020 at 05:27:17PM +0100, David Hildenbrand wrote: > > > Let's revert what we did in case seomthing goes wrong and we return an > > > error. > > > > Dumb question, but should not we do this for other arches as well? > > It seems arm64 and s390 already do that. > x86 could have its arch_add_memory() improved though :) Right, I only stared at x86 and see it did not have it. I guess we want to have all arches aligned with this. Thanks -- Oscar Salvador SUSE L3
Re: [PATCH seccomp 3/8] powerpc: Enable seccomp architecture tracking
On Wed, Nov 4, 2020 at 4:22 AM Michael Ellerman wrote: > > +#ifdef __LITTLE_ENDIAN__ > > As Kees mentioned this should (must?!) match the configured endian. > > But I think it would still be better to use the CONFIG symbol, which is > CONFIG_CPU_LITTLE_ENDIAN. My attempt here is to be consistent with asm/syscall.h syscall_get_arch [1]. Would it make sense to change that to CONFIG_CPU_LITTLE_ENDIAN then? [1] https://elixir.bootlin.com/linux/latest/source/arch/powerpc/include/asm/syscall.h#L116 > > +# define SECCOMP_ARCH_NATIVE (AUDIT_ARCH_PPC64 | __SECCOMP_ARCH_LE) > > You use __SECCOMP_ARCH_LE there, but previously you only defined > __SECCOMP_ARCH_LE_BIT. > > Is there some magic somewhere that defines __SECCOMP_ARCH_LE based on > __SECCOMP_ARCH_LE_BIT ? Oops, my bad here. > > +# define SECCOMP_ARCH_NATIVE_NR NR_syscalls > > +# define SECCOMP_ARCH_NATIVE_NAME"ppc64" > > What's the name used for? This is used in the last patch in this series to report in procfs the name of each architecture tracked by the bitmap cache. > Usually we use "ppc64" for 64-bit big endian and "ppc64le" for 64-bit > little endian. > > And usually we use "ppc" for 32-bit. Ok. YiFei Zhu
[PATCH] powerpc: Use the common INIT_DATA_SECTION macro in vmlinux.lds.S
Use the common INIT_DATA_SECTION rule for the linker script in an effort
to regularize the linker script.
Signed-off-by: Youling Tang
---
arch/powerpc/kernel/vmlinux.lds.S | 19 +--
1 file changed, 1 insertion(+), 18 deletions(-)
diff --git a/arch/powerpc/kernel/vmlinux.lds.S
b/arch/powerpc/kernel/vmlinux.lds.S
index e0548b4..5dc05f3 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -186,21 +186,7 @@ SECTIONS
EXIT_TEXT
}
- .init.data : AT(ADDR(.init.data) - LOAD_OFFSET) {
- INIT_DATA
- }
-
- .init.setup : AT(ADDR(.init.setup) - LOAD_OFFSET) {
- INIT_SETUP(16)
- }
-
- .initcall.init : AT(ADDR(.initcall.init) - LOAD_OFFSET) {
- INIT_CALLS
- }
-
- .con_initcall.init : AT(ADDR(.con_initcall.init) - LOAD_OFFSET) {
- CON_INITCALL
- }
+ INIT_DATA_SECTION(16)
. = ALIGN(8);
__ftr_fixup : AT(ADDR(__ftr_fixup) - LOAD_OFFSET) {
@@ -228,9 +214,6 @@ SECTIONS
__stop___fw_ftr_fixup = .;
}
#endif
- .init.ramfs : AT(ADDR(.init.ramfs) - LOAD_OFFSET) {
- INIT_RAM_FS
- }
PERCPU_SECTION(L1_CACHE_BYTES)
--
2.1.0
Re: [PATCH v1 3/4] powerpc/mm: remove linear mapping if __add_pages() fails in arch_add_memory()
On Wed, Nov 04, 2020 at 10:50:07AM +0100, osalvador wrote: > On Thu, Oct 29, 2020 at 05:27:17PM +0100, David Hildenbrand wrote: > > Let's revert what we did in case seomthing goes wrong and we return an > > error. > > Dumb question, but should not we do this for other arches as well? It seems arm64 and s390 already do that. x86 could have its arch_add_memory() improved though :) > -- > Oscar Salvador > SUSE L3 -- Sincerely yours, Mike.
Re: [PATCH] powerpc/eeh_cache: Fix a possible debugfs deadlock
On Wed, 28 Oct 2020 11:27:17 -0400, Qian Cai wrote: > Lockdep complains that a possible deadlock below in > eeh_addr_cache_show() because it is acquiring a lock with IRQ enabled, > but eeh_addr_cache_insert_dev() needs to acquire the same lock with IRQ > disabled. Let's just make eeh_addr_cache_show() acquire the lock with > IRQ disabled as well. > > CPU0CPU1 > >lock(&pci_io_addr_cache_root.piar_lock); > local_irq_disable(); > lock(&tp->lock); > lock(&pci_io_addr_cache_root.piar_lock); > > lock(&tp->lock); > > [...] Applied to powerpc/fixes. [1/1] powerpc/eeh_cache: Fix a possible debugfs deadlock https://git.kernel.org/powerpc/c/fd552e0542b4532483289cce48fdbd27b692984b cheers
Re: [PATCH] powerpc/smp: Move rcu_cpu_starting() earlier
On Wed, 28 Oct 2020 14:23:34 -0400, Qian Cai wrote: > The call to rcu_cpu_starting() in start_secondary() is not early enough > in the CPU-hotplug onlining process, which results in lockdep splats as > follows: > > WARNING: suspicious RCU usage > - > kernel/locking/lockdep.c:3497 RCU-list traversed in non-reader section!! > > [...] Applied to powerpc/fixes. [1/1] powerpc/smp: Call rcu_cpu_starting() earlier https://git.kernel.org/powerpc/c/99f070b62322a4b8c1252952735806d09eb44b68 cheers
Re: [PATCH] powerpc: Use asm_goto_volatile for put_user()
With that patch the kernel is working again. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different."
[PATCH] powerpc: Use asm_goto_volatile for put_user()
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.
<.schedule_tail>:
mflrr0
std r0,16(r1)
stdur1,-112(r1)
bl <.finish_task_switch>
ld r9,2496(r3)
cmpdi cr7,r9,0
bne cr7,<.schedule_tail+0x60>
ld r3,392(r13)
ld r9,1392(r3)
cmpdi cr7,r9,0
beq cr7,<.schedule_tail+0x3c>
li r4,0
li r5,0
bl <.__task_pid_nr_ns>
nop
bl <.calculate_sigpending>
nop
addir1,r1,112
ld r0,16(r1)
mtlrr0
blr
nop
nop
nop
bl <.__balance_callback>
b <.schedule_tail+0x1c>
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 inline asm that's part of the isync()
and mtspr() inlined via allow_user_access() seems to be enough to
avoid the bug.
We already have a macro to work around this (or a similar bug), called
asm_volatile_goto which includes an empty asm block to tickle the
compiler into generating the right code. So use that.
With this applied the code generation looks more like it will work:
<.schedule_tail>:
mflrr0
std r31,-8(r1)
std r0,16(r1)
stdur1,-144(r1)
std r3,112(r1)
bl <._mcount>
nop
ld r3,112(r1)
bl <.finish_task_switch>
ld r9,2624(r3)
cmpdi cr7,r9,0
bne cr7,<.schedule_tail+0xa0>
ld r3,2408(r13)
ld r31,1856(r3)
cmpdi cr7,r31,0
beq cr7,<.schedule_tail+0x80>
li r4,0
li r5,0
bl <.__task_pid_nr_ns>
nop
li r9,-1
clrldi r9,r9,12
cmpld cr7,r31,r9
bgt cr7,<.schedule_tail+0x80>
lis r9,16
rldicr r9,r9,32,31
subfr9,r31,r9
cmpldi cr7,r9,3
ble cr7,<.schedule_tail+0x80>
li r9,0
stw r3,0(r31) <-- stw
nop
bl <.calculate_sigpending>
nop
addir1,r1,144
ld r0,16(r1)
ld r31,-8(r1)
mtlrr0
blr
nop
bl <.__balance_callback>
b <.schedule_tail+0x30>
Fixes: ee0a49a6870e ("powerpc/uaccess: Switch __put_user_size_allowed() to
__put_user_asm_goto()")
Reported-by: Andreas Schwab
Suggested-by: Christophe Leroy
Signed-off-by: Michael Ellerman
---
arch/powerpc/include/asm/uaccess.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
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) \
--
2.25.1
Re: [PATCH] powerpc: Don't use asm goto for put_user() with GCC 4.9
Christophe Leroy writes:
> 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 ?
Ugh. I knew of that work around, but thought we'd dropped it when we
moved up to GCC 4.9. I should have looked more closely.
I'll send a patch to switch to it.
cheers
Re: [PATCH seccomp 3/8] powerpc: Enable seccomp architecture tracking
YiFei Zhu writes: > From: YiFei Zhu > > To enable seccomp constant action bitmaps, we need to have a static > mapping to the audit architecture and system call table size. Add these > for powerpc. > > Signed-off-by: YiFei Zhu > --- > arch/powerpc/include/asm/seccomp.h | 21 + > 1 file changed, 21 insertions(+) > > diff --git a/arch/powerpc/include/asm/seccomp.h > b/arch/powerpc/include/asm/seccomp.h > index 51209f6071c5..3efcc83e9cc6 100644 > --- a/arch/powerpc/include/asm/seccomp.h > +++ b/arch/powerpc/include/asm/seccomp.h > @@ -8,4 +8,25 @@ > > #include > > +#ifdef __LITTLE_ENDIAN__ As Kees mentioned this should (must?!) match the configured endian. But I think it would still be better to use the CONFIG symbol, which is CONFIG_CPU_LITTLE_ENDIAN. > +#define __SECCOMP_ARCH_LE_BIT__AUDIT_ARCH_LE > +#else > +#define __SECCOMP_ARCH_LE_BIT0 > +#endif > + > +#ifdef CONFIG_PPC64 > +# define SECCOMP_ARCH_NATIVE (AUDIT_ARCH_PPC64 | __SECCOMP_ARCH_LE) You use __SECCOMP_ARCH_LE there, but previously you only defined __SECCOMP_ARCH_LE_BIT. Is there some magic somewhere that defines __SECCOMP_ARCH_LE based on __SECCOMP_ARCH_LE_BIT ? > +# define SECCOMP_ARCH_NATIVE_NR NR_syscalls > +# define SECCOMP_ARCH_NATIVE_NAME"ppc64" What's the name used for? Usually we use "ppc64" for 64-bit big endian and "ppc64le" for 64-bit little endian. > +# ifdef CONFIG_COMPAT > +# define SECCOMP_ARCH_COMPAT(AUDIT_ARCH_PPC | > __SECCOMP_ARCH_LE) > +# define SECCOMP_ARCH_COMPAT_NR NR_syscalls > +# define SECCOMP_ARCH_COMPAT_NAME "powerpc" And usually we use "ppc" for 32-bit. > +# endif > +#else /* !CONFIG_PPC64 */ > +# define SECCOMP_ARCH_NATIVE (AUDIT_ARCH_PPC | __SECCOMP_ARCH_LE) > +# define SECCOMP_ARCH_NATIVE_NR NR_syscalls > +# define SECCOMP_ARCH_NATIVE_NAME"powerpc" > +#endif > + > #endif /* _ASM_POWERPC_SECCOMP_H */ > -- > 2.29.2 cheers
Re: [PATCH v1 3/4] powerpc/mm: remove linear mapping if __add_pages() fails in arch_add_memory()
On Thu, Oct 29, 2020 at 05:27:17PM +0100, David Hildenbrand wrote: > Let's revert what we did in case seomthing goes wrong and we return an > error. Dumb question, but should not we do this for other arches as well? -- Oscar Salvador SUSE L3
Re: [PATCH v1 2/4] powerpc/mm: print warning in arch_remove_linear_mapping()
On Thu, Oct 29, 2020 at 05:27:16PM +0100, David Hildenbrand wrote:
> Let's print a warning similar to in arch_add_linear_mapping() instead of
> WARN_ON_ONCE() and eventually crashing the kernel.
>
> Cc: Michael Ellerman
> Cc: Benjamin Herrenschmidt
> Cc: Paul Mackerras
> Cc: Rashmica Gupta
> Cc: Andrew Morton
> Cc: Mike Rapoport
> Cc: Michal Hocko
> Cc: Oscar Salvador
> Cc: Wei Yang
> Signed-off-by: David Hildenbrand
> ---
> arch/powerpc/mm/mem.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index 8a86d81f8df0..685028451dd2 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -145,7 +145,9 @@ void __ref arch_remove_linear_mapping(u64 start, u64 size)
> flush_dcache_range_chunked(start, start + size, FLUSH_CHUNK_SIZE);
>
> ret = remove_section_mapping(start, start + size);
> - WARN_ON_ONCE(ret);
> + if (ret)
> + pr_warn("Unable to remove linear mapping for 0x%llx..0x%llx:
> %d\n",
> + start, start + size, ret);
I guess the fear is to panic on systems that do have panic_on_warn (not
sure how many productions systems have this out there).
But anyway, being coherent with that, I think you should remove the WARN_ON
in hash__remove_section_mapping as well.
Besides that:
Reviewed-by: Oscar Salvador
Re: [PATCH] x86/mpx: fix recursive munmap() corruption
Le 03/11/2020 à 22:08, Dmitry Safonov a écrit : Hi Laurent, Christophe, Michael, all, On 11/3/20 5:11 PM, Laurent Dufour wrote: Le 23/10/2020 à 14:28, Christophe Leroy a écrit : [..] 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. It's probably not necessary anymore. I've sent patches [1], currently in akpm, the last one forbids splitting of vm_special_mapping. So, a user is able munmap() or mremap() vdso as a whole, but not partly. Hi Dmitry, That's a good thing too, but I think my patch is still valid in the PowerPC code, fixing a bad check, even if some corner cases are handled earlier in the code. [1]: https://lore.kernel.org/linux-mm/20201013013416.390574-1-d...@arista.com/ Thanks, Dmitry
Re: [PATCH] powerpc/32s: Setup the early hash table at all time.
>> To be sure we are not in front of a long lasting bug, could you try >> CONFIG_KASAN=y on v5.9 ? > > Indeed it started to fail somewhere between v5.6 and v5.7. > > v5.7 fails early with few messages on the console with reboot, v5.8 and > later hang right at bootloader. > > I'm bisecting now. (side note: I tried FB_OF=y instead of DRM_RADEON + DRM_FBDEV_* to speed up bisection and it turns out in that configuration KASAN never worked, down to commit 305d600123046, hanging right after bootloader or even with invalid access in the bootloader itself).
[PATCH 12/12] net: ethernet: ibm: ibmvnic: Fix some kernel-doc issues
Fixes the following W=1 kernel build warning(s): from drivers/net/ethernet/ibm/ibmvnic.c:35: inlined from ‘handle_vpd_rsp’ at drivers/net/ethernet/ibm/ibmvnic.c:4124:3: drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or member 'hdr_data' not described in 'build_hdr_data' drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Excess function parameter 'tot_len' description in 'build_hdr_data' drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 'hdr_data' not described in 'create_hdr_descs' drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Excess function parameter 'data' description in 'create_hdr_descs' drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Function parameter or member 'txbuff' not described in 'build_hdr_descs_arr' drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Excess function parameter 'skb' description in 'build_hdr_descs_arr' drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Excess function parameter 'subcrq' description in 'build_hdr_descs_arr' Cc: Michael Ellerman Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Dany Madden Cc: Lijun Pan Cc: Sukadev Bhattiprolu Cc: "David S. Miller" Cc: Jakub Kicinski Cc: Santiago Leon Cc: Thomas Falcon Cc: John Allen Cc: linuxppc-dev@lists.ozlabs.org Cc: net...@vger.kernel.org Signed-off-by: Lee Jones --- drivers/net/ethernet/ibm/ibmvnic.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index b30e1f5784bad..08dab7a94b7ea 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1360,7 +1360,7 @@ static int ibmvnic_close(struct net_device *netdev) * @hdr_field: bitfield determining needed headers * @skb: socket buffer * @hdr_len: array of header lengths - * @tot_len: total length of data + * @hdr_data: buffer to write the header to * * Reads hdr_field to determine which headers are needed by firmware. * Builds a buffer containing these headers. Saves individual header @@ -1418,7 +1418,7 @@ static int build_hdr_data(u8 hdr_field, struct sk_buff *skb, /** * create_hdr_descs - create header and header extension descriptors * @hdr_field: bitfield determining needed headers - * @data: buffer containing header data + * @hdr_data: buffer containing header data * @len: length of data buffer * @hdr_len: array of individual header lengths * @scrq_arr: descriptor array @@ -1469,9 +1469,8 @@ static int create_hdr_descs(u8 hdr_field, u8 *hdr_data, int len, int *hdr_len, /** * build_hdr_descs_arr - build a header descriptor array - * @skb: socket buffer + * @txbuff: tx buffer * @num_entries: number of descriptors to be sent - * @subcrq: first TX descriptor * @hdr_field: bit field determining which headers will be sent * * This function will build a TX descriptor array with applicable -- 2.25.1
[PATCH 10/12] net: ethernet: toshiba: ps3_gelic_net: Fix some kernel-doc misdemeanours
Fixes the following W=1 kernel build warning(s):
drivers/net/ethernet/toshiba/ps3_gelic_net.c:1107: warning: Function parameter
or member 'irq' not described in 'gelic_card_interrupt'
drivers/net/ethernet/toshiba/ps3_gelic_net.c:1107: warning: Function parameter
or member 'ptr' not described in 'gelic_card_interrupt'
drivers/net/ethernet/toshiba/ps3_gelic_net.c:1407: warning: Function parameter
or member 'txqueue' not described in 'gelic_net_tx_timeout'
drivers/net/ethernet/toshiba/ps3_gelic_net.c:1439: warning: Function parameter
or member 'napi' not described in 'gelic_ether_setup_netdev_ops'
drivers/net/ethernet/toshiba/ps3_gelic_net.c:1639: warning: Function parameter
or member 'dev' not described in 'ps3_gelic_driver_probe'
drivers/net/ethernet/toshiba/ps3_gelic_net.c:1795: warning: Function parameter
or member 'dev' not described in 'ps3_gelic_driver_remove'
Cc: Geoff Levand
Cc: "David S. Miller"
Cc: Jakub Kicinski
Cc: Michael Ellerman
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Utz Bacher
Cc: Jens Osterkamp
Cc: net...@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Lee Jones
---
drivers/net/ethernet/toshiba/ps3_gelic_net.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
index d9a5722f561b5..f886e23f8ed0a 100644
--- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
+++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
@@ -1100,7 +1100,7 @@ static int gelic_net_poll(struct napi_struct *napi, int
budget)
return packets_done;
}
-/**
+/*
* gelic_card_interrupt - event handler for gelic_net
*/
static irqreturn_t gelic_card_interrupt(int irq, void *ptr)
@@ -1400,6 +1400,7 @@ static void gelic_net_tx_timeout_task(struct work_struct
*work)
/**
* gelic_net_tx_timeout - called when the tx timeout watchdog kicks in.
* @netdev: interface device structure
+ * @txqueue: unused
*
* called, if tx hangs. Schedules a task that resets the interface
*/
@@ -1431,6 +1432,7 @@ static const struct net_device_ops gelic_netdevice_ops = {
/**
* gelic_ether_setup_netdev_ops - initialization of net_device operations
* @netdev: net_device structure
+ * @napi: napi structure
*
* fills out function pointers in the net_device structure
*/
@@ -1632,7 +1634,7 @@ static void gelic_card_get_vlan_info(struct gelic_card
*card)
dev_info(ctodev(card), "internal vlan %s\n",
card->vlan_required? "enabled" : "disabled");
}
-/**
+/*
* ps3_gelic_driver_probe - add a device to the control of this driver
*/
static int ps3_gelic_driver_probe(struct ps3_system_bus_device *dev)
@@ -1787,10 +1789,9 @@ static int ps3_gelic_driver_probe(struct
ps3_system_bus_device *dev)
return result;
}
-/**
+/*
* ps3_gelic_driver_remove - remove a device from the control of this driver
*/
-
static int ps3_gelic_driver_remove(struct ps3_system_bus_device *dev)
{
struct gelic_card *card = ps3_system_bus_get_drvdata(dev);
--
2.25.1
[PATCH 09/12] net: ethernet: ibm: ibmvnic: Fix some kernel-doc misdemeanours
Fixes the following W=1 kernel build warning(s): from drivers/net/ethernet/ibm/ibmvnic.c:35: inlined from ‘handle_vpd_rsp’ at drivers/net/ethernet/ibm/ibmvnic.c:4124:3: drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or member 'hdr_field' not described in 'build_hdr_data' drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or member 'skb' not described in 'build_hdr_data' drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or member 'hdr_len' not described in 'build_hdr_data' drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or member 'hdr_data' not described in 'build_hdr_data' drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 'hdr_field' not described in 'create_hdr_descs' drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 'hdr_data' not described in 'create_hdr_descs' drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 'len' not described in 'create_hdr_descs' drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 'hdr_len' not described in 'create_hdr_descs' drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 'scrq_arr' not described in 'create_hdr_descs' drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Function parameter or member 'txbuff' not described in 'build_hdr_descs_arr' drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Function parameter or member 'num_entries' not described in 'build_hdr_descs_arr' drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Function parameter or member 'hdr_field' not described in 'build_hdr_descs_arr' drivers/net/ethernet/ibm/ibmvnic.c:1832: warning: Function parameter or member 'adapter' not described in 'do_change_param_reset' drivers/net/ethernet/ibm/ibmvnic.c:1832: warning: Function parameter or member 'rwi' not described in 'do_change_param_reset' drivers/net/ethernet/ibm/ibmvnic.c:1832: warning: Function parameter or member 'reset_state' not described in 'do_change_param_reset' drivers/net/ethernet/ibm/ibmvnic.c:1911: warning: Function parameter or member 'adapter' not described in 'do_reset' drivers/net/ethernet/ibm/ibmvnic.c:1911: warning: Function parameter or member 'rwi' not described in 'do_reset' drivers/net/ethernet/ibm/ibmvnic.c:1911: warning: Function parameter or member 'reset_state' not described in 'do_reset' Cc: Dany Madden Cc: Lijun Pan Cc: Sukadev Bhattiprolu Cc: Michael Ellerman Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: "David S. Miller" Cc: Jakub Kicinski Cc: Santiago Leon Cc: Thomas Falcon Cc: John Allen Cc: net...@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Signed-off-by: Lee Jones --- drivers/net/ethernet/ibm/ibmvnic.c | 28 ++-- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 1dc3cfdb38abc..b30e1f5784bad 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1357,10 +1357,10 @@ static int ibmvnic_close(struct net_device *netdev) /** * build_hdr_data - creates L2/L3/L4 header data buffer - * @hdr_field - bitfield determining needed headers - * @skb - socket buffer - * @hdr_len - array of header lengths - * @tot_len - total length of data + * @hdr_field: bitfield determining needed headers + * @skb: socket buffer + * @hdr_len: array of header lengths + * @tot_len: total length of data * * Reads hdr_field to determine which headers are needed by firmware. * Builds a buffer containing these headers. Saves individual header @@ -1417,11 +1417,11 @@ static int build_hdr_data(u8 hdr_field, struct sk_buff *skb, /** * create_hdr_descs - create header and header extension descriptors - * @hdr_field - bitfield determining needed headers - * @data - buffer containing header data - * @len - length of data buffer - * @hdr_len - array of individual header lengths - * @scrq_arr - descriptor array + * @hdr_field: bitfield determining needed headers + * @data: buffer containing header data + * @len: length of data buffer + * @hdr_len: array of individual header lengths + * @scrq_arr: descriptor array * * Creates header and, if needed, header extension descriptors and * places them in a descriptor array, scrq_arr @@ -1469,10 +1469,10 @@ static int create_hdr_descs(u8 hdr_field, u8 *hdr_data, int len, int *hdr_len, /** * build_hdr_descs_arr - build a header descriptor array - * @skb - socket buffer - * @num_entries - number of descriptors to be sent - * @subcrq - first TX descriptor - * @hdr_field - bit field determining which headers will be sent + * @skb: socket buffer + * @num_entries: number of descriptors to be sent + * @subcrq: first TX descriptor + * @hdr_field: bit field determining which headers will be sent * * This function will build a TX descriptor array with applicable * L2/L3/L4 packet
[PATCH 00/12] [Set 2] Rid W=1 warnings in Net
MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. This is the last set. Lee Jones (12): net: usb: lan78xx: Remove lots of set but unused 'ret' variables net: ethernet: smsc: smc911x: Mark 'status' as __maybe_unused net: ethernet: xilinx: xilinx_emaclite: Document 'txqueue' even if it is unused net: ethernet: smsc: smc91x: Demote non-conformant kernel function header net: xen-netback: xenbus: Demote nonconformant kernel-doc headers net: ethernet: ti: am65-cpsw-qos: Demote non-conformant function header net: ethernet: ti: am65-cpts: Document am65_cpts_rx_enable()'s 'en' parameter net: xen-netfront: Demote non-kernel-doc headers to standard comment blocks net: ethernet: ibm: ibmvnic: Fix some kernel-doc misdemeanours net: ethernet: toshiba: ps3_gelic_net: Fix some kernel-doc misdemeanours net: ethernet: toshiba: spider_net: Document a whole bunch of function parameters net: ethernet: ibm: ibmvnic: Fix some kernel-doc issues drivers/net/ethernet/ibm/ibmvnic.c| 27 ++- drivers/net/ethernet/smsc/smc911x.c | 6 +- drivers/net/ethernet/smsc/smc91x.c| 2 +- drivers/net/ethernet/ti/am65-cpsw-qos.c | 2 +- drivers/net/ethernet/ti/am65-cpts.c | 2 +- drivers/net/ethernet/toshiba/ps3_gelic_net.c | 9 +- drivers/net/ethernet/toshiba/spider_net.c | 18 +- drivers/net/ethernet/xilinx/xilinx_emaclite.c | 1 + drivers/net/usb/lan78xx.c | 212 +- drivers/net/xen-netback/xenbus.c | 4 +- drivers/net/xen-netfront.c| 6 +- 11 files changed, 141 insertions(+), 148 deletions(-) Cc: Alexei Starovoitov Cc: Andrii Nakryiko Cc: Benjamin Herrenschmidt Cc: Boris Ostrovsky Cc: b...@vger.kernel.org Cc: Daniel Borkmann Cc: Dany Madden Cc: Daris A Nevil Cc: "David S. Miller" Cc: Dustin McIntire Cc: Erik Stahlman Cc: Geoff Levand Cc: Grygorii Strashko Cc: "Gustavo A. R. Silva" Cc: Ishizaki Kou Cc: Ivan Khoronzhuk Cc: Jakub Kicinski Cc: Jens Osterkamp Cc: Jesper Dangaard Brouer Cc: John Allen Cc: John Fastabend Cc: John Williams Cc: Juergen Gross Cc: KP Singh Cc: Kurt Kanzenbach Cc: Lijun Pan Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: Martin Habets Cc: Martin KaFai Lau Cc: Michael Ellerman Cc: "Michael S. Tsirkin" Cc: Michal Simek Cc: Microchip Linux Driver Support Cc: net...@vger.kernel.org Cc: Nicolas Pitre Cc: Paul Durrant Cc: Paul Mackerras Cc: Peter Cammaert Cc: Russell King Cc: Rusty Russell Cc: Santiago Leon Cc: Shannon Nelson Cc: Song Liu Cc: Stefano Stabellini Cc: Sukadev Bhattiprolu Cc: Thomas Falcon Cc: Utz Bacher Cc: Wei Liu Cc: Woojung Huh Cc: xen-de...@lists.xenproject.org Cc: Yonghong Song -- 2.25.1
Re: [PATCH 4.19] mm: fix exec activate_mm vs TLB shootdown and lazy tlb switching race
On Wed, Nov 04, 2020 at 12:14:06PM +1100, Michael Ellerman wrote:
> From: Nicholas Piggin
>
> commit d53c3dfb23c45f7d4f910c3a3ca84bf0a99c6143 upstream.
>
> Reading and modifying current->mm and current->active_mm and switching
> mm should be done with irqs off, to prevent races seeing an intermediate
> state.
>
> This is similar to commit 38cf307c1f20 ("mm: fix kthread_use_mm() vs TLB
> invalidate"). At exec-time when the new mm is activated, the old one
> should usually be single-threaded and no longer used, unless something
> else is holding an mm_users reference (which may be possible).
>
> Absent other mm_users, there is also a race with preemption and lazy tlb
> switching. Consider the kernel_execve case where the current thread is
> using a lazy tlb active mm:
>
> call_usermodehelper()
> kernel_execve()
> old_mm = current->mm;
> active_mm = current->active_mm;
> *** preempt *** > schedule()
>prev->active_mm = NULL;
>mmdrop(prev active_mm);
> ...
> < schedule()
> current->mm = mm;
> current->active_mm = mm;
> if (!old_mm)
> mmdrop(active_mm);
>
> If we switch back to the kernel thread from a different mm, there is a
> double free of the old active_mm, and a missing free of the new one.
>
> Closing this race only requires interrupts to be disabled while ->mm
> and ->active_mm are being switched, but the TLB problem requires also
> holding interrupts off over activate_mm. Unfortunately not all archs
> can do that yet, e.g., arm defers the switch if irqs are disabled and
> expects finish_arch_post_lock_switch() to be called to complete the
> flush; um takes a blocking lock in activate_mm().
>
> So as a first step, disable interrupts across the mm/active_mm updates
> to close the lazy tlb preempt race, and provide an arch option to
> extend that to activate_mm which allows architectures doing IPI based
> TLB shootdowns to close the second race.
>
> This is a bit ugly, but in the interest of fixing the bug and backporting
> before all architectures are converted this is a compromise.
>
> Signed-off-by: Nicholas Piggin
> Acked-by: Peter Zijlstra (Intel)
> [mpe: Manual backport to 4.19 due to membarrier_exec_mmap(mm) changes]
> Signed-off-by: Michael Ellerman
> Link: https://lore.kernel.org/r/20200914045219.3736466-2-npig...@gmail.com
> ---
> arch/Kconfig | 7 +++
> fs/exec.c| 15 ++-
> 2 files changed, 21 insertions(+), 1 deletion(-)
Now queued up, thanks!
greg k-h
