Re: [PATCH v3 35/51] trace,hardirq: No moar _rcuidle() tracing
On Tue, Jan 17, 2023 at 01:24:46PM +0900, Masami Hiramatsu wrote:
> Hi Peter,
>
> On Thu, 12 Jan 2023 20:43:49 +0100
> Peter Zijlstra wrote:
>
> > Robot reported that trace_hardirqs_{on,off}() tickle the forbidden
> > _rcuidle() tracepoint through local_irq_{en,dis}able().
> >
> > For 'sane' configs, these calls will only happen with RCU enabled and
> > as such can use the regular tracepoint. This also means it's possible
> > to trace them from NMI context again.
> >
> > Signed-off-by: Peter Zijlstra (Intel)
>
> The code looks good to me. I just have a question about comment.
>
> > ---
> > kernel/trace/trace_preemptirq.c | 21 +
> > 1 file changed, 13 insertions(+), 8 deletions(-)
> >
> > --- a/kernel/trace/trace_preemptirq.c
> > +++ b/kernel/trace/trace_preemptirq.c
> > @@ -20,6 +20,15 @@
> > static DEFINE_PER_CPU(int, tracing_irq_cpu);
> >
> > /*
> > + * ...
>
> Is this intended? Wouldn't you leave any comment here?
I indeed forgot to write the comment before posting, my bad :/ Ingo fixed
it up when he applied.
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v3 00/51] cpuidle,rcu: Clean up the mess
On Mon, Jan 16, 2023 at 04:59:04PM +, Mark Rutland wrote:
> I'm sorry to have to bear some bad news on that front. :(
Moo, something had to give..
> IIUC what's happenign here is the PSCI cpuidle driver has entered idle and RCU
> is no longer watching when arm64's cpu_suspend() manipulates DAIF. Our
> local_daif_*() helpers poke lockdep and tracing, hence the call to
> trace_hardirqs_off() and the RCU usage.
Right, strictly speaking not needed at this point, IRQs should have been
traced off a long time ago.
> I think we need RCU to be watching all the way down to cpu_suspend(), and it's
> cpu_suspend() that should actually enter/exit idle context. That and we need
> to
> make cpu_suspend() and the low-level PSCI invocation noinstr.
>
> I'm not sure whether 32-bit will have a similar issue or not.
I'm not seeing 32bit or Risc-V have similar issues here, but who knows,
maybe I missed somsething.
In any case, the below ought to cure the ARM64 case and remove that last
known RCU_NONIDLE() user as a bonus.
---
diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c
index 41974a1a229a..42e19fff40ee 100644
--- a/arch/arm64/kernel/cpuidle.c
+++ b/arch/arm64/kernel/cpuidle.c
@@ -67,10 +67,10 @@ __cpuidle int acpi_processor_ffh_lpi_enter(struct
acpi_lpi_state *lpi)
u32 state = lpi->address;
if (ARM64_LPI_IS_RETENTION_STATE(lpi->arch_flags))
- return
CPU_PM_CPU_IDLE_ENTER_RETENTION_PARAM(psci_cpu_suspend_enter,
+ return
CPU_PM_CPU_IDLE_ENTER_RETENTION_PARAM_RCU(psci_cpu_suspend_enter,
lpi->index, state);
else
- return CPU_PM_CPU_IDLE_ENTER_PARAM(psci_cpu_suspend_enter,
+ return CPU_PM_CPU_IDLE_ENTER_PARAM_RCU(psci_cpu_suspend_enter,
lpi->index, state);
}
#endif
diff --git a/arch/arm64/kernel/suspend.c b/arch/arm64/kernel/suspend.c
index e7163f31f716..0fbdf5fe64d8 100644
--- a/arch/arm64/kernel/suspend.c
+++ b/arch/arm64/kernel/suspend.c
@@ -4,6 +4,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -104,6 +105,10 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned
long))
* From this point debug exceptions are disabled to prevent
* updates to mdscr register (saved and restored along with
* general purpose registers) from kernel debuggers.
+*
+* Strictly speaking the trace_hardirqs_off() here is superfluous,
+* hardirqs should be firmly off by now. This really ought to use
+* something like raw_local_daif_save().
*/
flags = local_daif_save();
@@ -120,6 +125,8 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
*/
arm_cpuidle_save_irq_context(&context);
+ ct_cpuidle_enter();
+
if (__cpu_suspend_enter(&state)) {
/* Call the suspend finisher */
ret = fn(arg);
@@ -133,8 +140,11 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned
long))
*/
if (!ret)
ret = -EOPNOTSUPP;
+
+ ct_cpuidle_exit();
} else {
- RCU_NONIDLE(__cpu_suspend_exit());
+ ct_cpuidle_exit();
+ __cpu_suspend_exit();
}
arm_cpuidle_restore_irq_context(&context);
diff --git a/drivers/cpuidle/cpuidle-psci.c b/drivers/cpuidle/cpuidle-psci.c
index 4fc4e0381944..312a34ef28dc 100644
--- a/drivers/cpuidle/cpuidle-psci.c
+++ b/drivers/cpuidle/cpuidle-psci.c
@@ -69,16 +69,12 @@ static __cpuidle int __psci_enter_domain_idle_state(struct
cpuidle_device *dev,
else
pm_runtime_put_sync_suspend(pd_dev);
- ct_cpuidle_enter();
-
state = psci_get_domain_state();
if (!state)
state = states[idx];
ret = psci_cpu_suspend_enter(state) ? -1 : idx;
- ct_cpuidle_exit();
-
if (s2idle)
dev_pm_genpd_resume(pd_dev);
else
@@ -192,7 +188,7 @@ static __cpuidle int psci_enter_idle_state(struct
cpuidle_device *dev,
{
u32 *state = __this_cpu_read(psci_cpuidle_data.psci_states);
- return CPU_PM_CPU_IDLE_ENTER_PARAM(psci_cpu_suspend_enter, idx,
state[idx]);
+ return CPU_PM_CPU_IDLE_ENTER_PARAM_RCU(psci_cpu_suspend_enter, idx,
state[idx]);
}
static const struct of_device_id psci_idle_state_match[] = {
diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index e7bcfca4159f..f3a044fa4652 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -462,11 +462,22 @@ int psci_cpu_suspend_enter(u32 state)
if (!psci_power_state_loses_context(state)) {
struct arm_cpuidle_irq_context context;
+ ct_cpuidle_enter();
arm_cpuidle_save_irq_context(&context);
ret = psci_ops.cpu_suspend(state, 0);
arm_cpuidle
[PATCH v2] ARC:fpu: add extra capability check before use of sqrt and fma builtins
From: Pavel Kozlov Add extra check for compiler definitions to ensure that compiler provides sqrt and fma hw fpu instructions else use software implementation. As divide/sqrt and FMA hw support from CPU side is optional, the compiler can be configured by options to generate hw FPU instructions, but without use of FDDIV, FDSQRT, FSDIV, FSSQRT, FDMADD and FSMADD instructions. In this case __builtin_sqrt and __builtin_sqrtf provided by compiler can't be used inside the glibc code, as these builtins are used in implementations of sqrt() and sqrtf() functions but at the same time these builtins unfold to sqrt() and sqrtf(). So it is possible to receive code like that: 0001c4b4 <__ieee754_sqrtf>: 1c4b4:0001 b 0 ;1c4b4 <__ieee754_sqrtf> The same is also true for __builtin_fma and __builtin_fmaf. --- Changes in v2: - Fixed macros definitions for FMA sysdeps/arc/fpu/math-use-builtins-fma.h | 14 -- sysdeps/arc/fpu/math-use-builtins-sqrt.h | 14 -- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/sysdeps/arc/fpu/math-use-builtins-fma.h b/sysdeps/arc/fpu/math-use-builtins-fma.h index eede75aa41be..2acd8113ce2c 100644 --- a/sysdeps/arc/fpu/math-use-builtins-fma.h +++ b/sysdeps/arc/fpu/math-use-builtins-fma.h @@ -1,4 +1,14 @@ -#define USE_FMA_BUILTIN 1 -#define USE_FMAF_BUILTIN 1 +#if defined __ARC_FPU_DP_FMA__ +# define USE_FMA_BUILTIN 1 +#else +# define USE_FMA_BUILTIN 0 +#endif + +#if defined __ARC_FPU_SP_FMA__ +# define USE_FMAF_BUILTIN 1 +#else +# define USE_FMAF_BUILTIN 0 +#endif + #define USE_FMAL_BUILTIN 0 #define USE_FMAF128_BUILTIN 0 diff --git a/sysdeps/arc/fpu/math-use-builtins-sqrt.h b/sysdeps/arc/fpu/math-use-builtins-sqrt.h index e94c915ba66a..a449bc609295 100644 --- a/sysdeps/arc/fpu/math-use-builtins-sqrt.h +++ b/sysdeps/arc/fpu/math-use-builtins-sqrt.h @@ -1,4 +1,14 @@ -#define USE_SQRT_BUILTIN 1 -#define USE_SQRTF_BUILTIN 1 +#if defined __ARC_FPU_DP_DIV__ +# define USE_SQRT_BUILTIN 1 +#else +# define USE_SQRT_BUILTIN 0 +#endif + +#if defined __ARC_FPU_SP_DIV__ +# define USE_SQRTF_BUILTIN 1 +#else +# define USE_SQRTF_BUILTIN 0 +#endif + #define USE_SQRTL_BUILTIN 0 #define USE_SQRTF128_BUILTIN 0 -- 2.25.1 ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] ARC:fpu: add extra capability check before use of sqrt and fma builtins
> This is wrong, sqrt use macro do not belong for the fma switch file. Thank you for the review and your notice. I was inattentive when moving changes from the branch I had. This has been fixed in v2 of the patch [1]. I've manually checked (by objdump -d output review) that now expected code for libm is generated in different cases (when compiler provides support for extra instructions and sets macroses __ARC_FPU_DP_DIV__, __ARC_FPU_SP_DIV__, __ARC_FPU_DP_FMA__, __ARC_FPU_SP_FMA__ and when not). [1] http://lists.infradead.org/pipermail/linux-snps-arc/2023-January/006771.html -- Pavel ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v3 00/51] cpuidle,rcu: Clean up the mess
On Tue, Jan 17, 2023 at 11:26:29AM +0100, Peter Zijlstra wrote: > On Mon, Jan 16, 2023 at 04:59:04PM +, Mark Rutland wrote: > > > I'm sorry to have to bear some bad news on that front. :( > > Moo, something had to give.. > > > > IIUC what's happenign here is the PSCI cpuidle driver has entered idle and > > RCU > > is no longer watching when arm64's cpu_suspend() manipulates DAIF. Our > > local_daif_*() helpers poke lockdep and tracing, hence the call to > > trace_hardirqs_off() and the RCU usage. > > Right, strictly speaking not needed at this point, IRQs should have been > traced off a long time ago. > > > I think we need RCU to be watching all the way down to cpu_suspend(), and > > it's > > cpu_suspend() that should actually enter/exit idle context. That and we > > need to > > make cpu_suspend() and the low-level PSCI invocation noinstr. > > > > I'm not sure whether 32-bit will have a similar issue or not. > > I'm not seeing 32bit or Risc-V have similar issues here, but who knows, > maybe I missed somsething. > > In any case, the below ought to cure the ARM64 case and remove that last > known RCU_NONIDLE() user as a bonus. > Thanks for the fix. I tested the series and did observe the same splat with both DT and ACPI boot(they enter idle in different code paths). Thanks to Mark for reminding me about ACPI. With this fix, I see the splat is gone in both DT(cpuidle-psci.c) and ACPI(acpi_processor_idle.c). You can add: Tested-by: Sudeep Holla -- Regards, Sudeep ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v3 00/51] cpuidle,rcu: Clean up the mess
On Tue, Jan 17, 2023 at 11:26:29AM +0100, Peter Zijlstra wrote:
> On Mon, Jan 16, 2023 at 04:59:04PM +, Mark Rutland wrote:
>
> > I'm sorry to have to bear some bad news on that front. :(
>
> Moo, something had to give..
>
>
> > IIUC what's happenign here is the PSCI cpuidle driver has entered idle and
> > RCU
> > is no longer watching when arm64's cpu_suspend() manipulates DAIF. Our
> > local_daif_*() helpers poke lockdep and tracing, hence the call to
> > trace_hardirqs_off() and the RCU usage.
>
> Right, strictly speaking not needed at this point, IRQs should have been
> traced off a long time ago.
True, but there are some other calls around here that *might* end up invoking
RCU stuff (e.g. the MTE code).
That all needs a noinstr cleanup too, which I'll sort out as a follow-up.
> > I think we need RCU to be watching all the way down to cpu_suspend(), and
> > it's
> > cpu_suspend() that should actually enter/exit idle context. That and we
> > need to
> > make cpu_suspend() and the low-level PSCI invocation noinstr.
> >
> > I'm not sure whether 32-bit will have a similar issue or not.
>
> I'm not seeing 32bit or Risc-V have similar issues here, but who knows,
> maybe I missed somsething.
I reckon if they do, the core changes here give us the infrastructure to fix
them if/when we get reports.
> In any case, the below ought to cure the ARM64 case and remove that last
> known RCU_NONIDLE() user as a bonus.
The below works for me testing on a Juno R1 board with PSCI, using defconfig +
CONFIG_PROVE_LOCKING=y + CONFIG_DEBUG_LOCKDEP=y + CONFIG_DEBUG_ATOMIC_SLEEP=y.
I'm not sure how to test the LPI / FFH part, but it looks good to me.
FWIW:
Reviewed-by: Mark Rutland
Tested-by: Mark Rutland
Sudeep, would you be able to give the LPI/FFH side a spin with the kconfig
options above?
Thanks,
Mark.
>
> ---
> diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c
> index 41974a1a229a..42e19fff40ee 100644
> --- a/arch/arm64/kernel/cpuidle.c
> +++ b/arch/arm64/kernel/cpuidle.c
> @@ -67,10 +67,10 @@ __cpuidle int acpi_processor_ffh_lpi_enter(struct
> acpi_lpi_state *lpi)
> u32 state = lpi->address;
>
> if (ARM64_LPI_IS_RETENTION_STATE(lpi->arch_flags))
> - return
> CPU_PM_CPU_IDLE_ENTER_RETENTION_PARAM(psci_cpu_suspend_enter,
> + return
> CPU_PM_CPU_IDLE_ENTER_RETENTION_PARAM_RCU(psci_cpu_suspend_enter,
> lpi->index, state);
> else
> - return CPU_PM_CPU_IDLE_ENTER_PARAM(psci_cpu_suspend_enter,
> + return CPU_PM_CPU_IDLE_ENTER_PARAM_RCU(psci_cpu_suspend_enter,
>lpi->index, state);
> }
> #endif
> diff --git a/arch/arm64/kernel/suspend.c b/arch/arm64/kernel/suspend.c
> index e7163f31f716..0fbdf5fe64d8 100644
> --- a/arch/arm64/kernel/suspend.c
> +++ b/arch/arm64/kernel/suspend.c
> @@ -4,6 +4,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -104,6 +105,10 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned
> long))
>* From this point debug exceptions are disabled to prevent
>* updates to mdscr register (saved and restored along with
>* general purpose registers) from kernel debuggers.
> + *
> + * Strictly speaking the trace_hardirqs_off() here is superfluous,
> + * hardirqs should be firmly off by now. This really ought to use
> + * something like raw_local_daif_save().
>*/
> flags = local_daif_save();
>
> @@ -120,6 +125,8 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned
> long))
>*/
> arm_cpuidle_save_irq_context(&context);
>
> + ct_cpuidle_enter();
> +
> if (__cpu_suspend_enter(&state)) {
> /* Call the suspend finisher */
> ret = fn(arg);
> @@ -133,8 +140,11 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned
> long))
>*/
> if (!ret)
> ret = -EOPNOTSUPP;
> +
> + ct_cpuidle_exit();
> } else {
> - RCU_NONIDLE(__cpu_suspend_exit());
> + ct_cpuidle_exit();
> + __cpu_suspend_exit();
> }
>
> arm_cpuidle_restore_irq_context(&context);
> diff --git a/drivers/cpuidle/cpuidle-psci.c b/drivers/cpuidle/cpuidle-psci.c
> index 4fc4e0381944..312a34ef28dc 100644
> --- a/drivers/cpuidle/cpuidle-psci.c
> +++ b/drivers/cpuidle/cpuidle-psci.c
> @@ -69,16 +69,12 @@ static __cpuidle int
> __psci_enter_domain_idle_state(struct cpuidle_device *dev,
> else
> pm_runtime_put_sync_suspend(pd_dev);
>
> - ct_cpuidle_enter();
> -
> state = psci_get_domain_state();
> if (!state)
> state = states[idx];
>
> ret = psci_cpu_suspend_enter(state) ? -1 : idx;
>
> - ct_cpuidle_exit();
> -
> if (s2idle)
> dev_pm_genpd_resume(pd_dev);
> else
> @@ -192,7 +188,7 @@ static _
Re: [PATCH v3 00/51] cpuidle,rcu: Clean up the mess
On Tue, Jan 17, 2023 at 01:16:21PM +, Mark Rutland wrote: > On Tue, Jan 17, 2023 at 11:26:29AM +0100, Peter Zijlstra wrote: > > On Mon, Jan 16, 2023 at 04:59:04PM +, Mark Rutland wrote: > > > > > I'm sorry to have to bear some bad news on that front. :( > > > > Moo, something had to give.. > > > > > > > IIUC what's happenign here is the PSCI cpuidle driver has entered idle > > > and RCU > > > is no longer watching when arm64's cpu_suspend() manipulates DAIF. Our > > > local_daif_*() helpers poke lockdep and tracing, hence the call to > > > trace_hardirqs_off() and the RCU usage. > > > > Right, strictly speaking not needed at this point, IRQs should have been > > traced off a long time ago. > > True, but there are some other calls around here that *might* end up invoking > RCU stuff (e.g. the MTE code). > > That all needs a noinstr cleanup too, which I'll sort out as a follow-up. > > > > I think we need RCU to be watching all the way down to cpu_suspend(), and > > > it's > > > cpu_suspend() that should actually enter/exit idle context. That and we > > > need to > > > make cpu_suspend() and the low-level PSCI invocation noinstr. > > > > > > I'm not sure whether 32-bit will have a similar issue or not. > > > > I'm not seeing 32bit or Risc-V have similar issues here, but who knows, > > maybe I missed somsething. > > I reckon if they do, the core changes here give us the infrastructure to fix > them if/when we get reports. > > > In any case, the below ought to cure the ARM64 case and remove that last > > known RCU_NONIDLE() user as a bonus. > > The below works for me testing on a Juno R1 board with PSCI, using defconfig + > CONFIG_PROVE_LOCKING=y + CONFIG_DEBUG_LOCKDEP=y + CONFIG_DEBUG_ATOMIC_SLEEP=y. > I'm not sure how to test the LPI / FFH part, but it looks good to me. > > FWIW: > > Reviewed-by: Mark Rutland > Tested-by: Mark Rutland > > Sudeep, would you be able to give the LPI/FFH side a spin with the kconfig > options above? > Not sure if I have messed up something in my mail setup, but I did reply earlier. I did test both DT/cpuidle-psci driver and ACPI/LPI+FFH driver with the fix Peter sent. I was seeing same splat as you in both DT and ACPI boot which the patch fixed it. I used the same config as described by you above. -- Regards, Sudeep ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2] ARC:fpu: add extra capability check before use of sqrt and fma builtins
On 17/01/23 09:12, Pavel.Kozlov--- via Libc-alpha wrote: > From: Pavel Kozlov > > Add extra check for compiler definitions to ensure that compiler provides > sqrt and fma hw fpu instructions else use software implementation. > > As divide/sqrt and FMA hw support from CPU side is optional, > the compiler can be configured by options to generate hw FPU instructions, > but without use of FDDIV, FDSQRT, FSDIV, FSSQRT, FDMADD and FSMADD > instructions. In this case __builtin_sqrt and __builtin_sqrtf provided by > compiler can't be used inside the glibc code, as these builtins are used > in implementations of sqrt() and sqrtf() functions but at the same time > these builtins unfold to sqrt() and sqrtf(). So it is possible to receive > code like that: > > 0001c4b4 <__ieee754_sqrtf>: >1c4b4:0001 b 0 ;1c4b4 <__ieee754_sqrtf> > > The same is also true for __builtin_fma and __builtin_fmaf. LGTM, thanks. You might need to check with Carlos O'Donnel to see it you could install this for 2.36. Reviewed-by: Adhemerval Zanella > > --- > Changes in v2: > - Fixed macros definitions for FMA > > sysdeps/arc/fpu/math-use-builtins-fma.h | 14 -- > sysdeps/arc/fpu/math-use-builtins-sqrt.h | 14 -- > 2 files changed, 24 insertions(+), 4 deletions(-) > > diff --git a/sysdeps/arc/fpu/math-use-builtins-fma.h > b/sysdeps/arc/fpu/math-use-builtins-fma.h > index eede75aa41be..2acd8113ce2c 100644 > --- a/sysdeps/arc/fpu/math-use-builtins-fma.h > +++ b/sysdeps/arc/fpu/math-use-builtins-fma.h > @@ -1,4 +1,14 @@ > -#define USE_FMA_BUILTIN 1 > -#define USE_FMAF_BUILTIN 1 > +#if defined __ARC_FPU_DP_FMA__ > +# define USE_FMA_BUILTIN 1 > +#else > +# define USE_FMA_BUILTIN 0 > +#endif > + > +#if defined __ARC_FPU_SP_FMA__ > +# define USE_FMAF_BUILTIN 1 > +#else > +# define USE_FMAF_BUILTIN 0 > +#endif > + > #define USE_FMAL_BUILTIN 0 > #define USE_FMAF128_BUILTIN 0 > diff --git a/sysdeps/arc/fpu/math-use-builtins-sqrt.h > b/sysdeps/arc/fpu/math-use-builtins-sqrt.h > index e94c915ba66a..a449bc609295 100644 > --- a/sysdeps/arc/fpu/math-use-builtins-sqrt.h > +++ b/sysdeps/arc/fpu/math-use-builtins-sqrt.h > @@ -1,4 +1,14 @@ > -#define USE_SQRT_BUILTIN 1 > -#define USE_SQRTF_BUILTIN 1 > +#if defined __ARC_FPU_DP_DIV__ > +# define USE_SQRT_BUILTIN 1 > +#else > +# define USE_SQRT_BUILTIN 0 > +#endif > + > +#if defined __ARC_FPU_SP_DIV__ > +# define USE_SQRTF_BUILTIN 1 > +#else > +# define USE_SQRTF_BUILTIN 0 > +#endif > + > #define USE_SQRTL_BUILTIN 0 > #define USE_SQRTF128_BUILTIN 0 ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v3 00/51] cpuidle,rcu: Clean up the mess
On Tue, Jan 17, 2023 at 02:21:40PM +, Sudeep Holla wrote: > On Tue, Jan 17, 2023 at 01:16:21PM +, Mark Rutland wrote: > > On Tue, Jan 17, 2023 at 11:26:29AM +0100, Peter Zijlstra wrote: > > > On Mon, Jan 16, 2023 at 04:59:04PM +, Mark Rutland wrote: > > > > > > > I'm sorry to have to bear some bad news on that front. :( > > > > > > Moo, something had to give.. > > > > > > > > > > IIUC what's happenign here is the PSCI cpuidle driver has entered idle > > > > and RCU > > > > is no longer watching when arm64's cpu_suspend() manipulates DAIF. Our > > > > local_daif_*() helpers poke lockdep and tracing, hence the call to > > > > trace_hardirqs_off() and the RCU usage. > > > > > > Right, strictly speaking not needed at this point, IRQs should have been > > > traced off a long time ago. > > > > True, but there are some other calls around here that *might* end up > > invoking > > RCU stuff (e.g. the MTE code). > > > > That all needs a noinstr cleanup too, which I'll sort out as a follow-up. > > > > > > I think we need RCU to be watching all the way down to cpu_suspend(), > > > > and it's > > > > cpu_suspend() that should actually enter/exit idle context. That and we > > > > need to > > > > make cpu_suspend() and the low-level PSCI invocation noinstr. > > > > > > > > I'm not sure whether 32-bit will have a similar issue or not. > > > > > > I'm not seeing 32bit or Risc-V have similar issues here, but who knows, > > > maybe I missed somsething. > > > > I reckon if they do, the core changes here give us the infrastructure to fix > > them if/when we get reports. > > > > > In any case, the below ought to cure the ARM64 case and remove that last > > > known RCU_NONIDLE() user as a bonus. > > > > The below works for me testing on a Juno R1 board with PSCI, using > > defconfig + > > CONFIG_PROVE_LOCKING=y + CONFIG_DEBUG_LOCKDEP=y + > > CONFIG_DEBUG_ATOMIC_SLEEP=y. > > I'm not sure how to test the LPI / FFH part, but it looks good to me. > > > > FWIW: > > > > Reviewed-by: Mark Rutland > > Tested-by: Mark Rutland > > > > Sudeep, would you be able to give the LPI/FFH side a spin with the kconfig > > options above? > > > > Not sure if I have messed up something in my mail setup, but I did reply > earlier. Sorry, that was my bad; I had been drafting my reply for a while and forgot to re-check prior to sending. > I did test both DT/cpuidle-psci driver and ACPI/LPI+FFH driver > with the fix Peter sent. I was seeing same splat as you in both DT and > ACPI boot which the patch fixed it. I used the same config as described by > you above. Perfect; thanks! Mark. ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
