Re: [PATCH 04/36] cpuidle,intel_idle: Fix CPUIDLE_FLAG_IRQ_ENABLE
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
- To: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
- Subject: Re: [PATCH 04/36] cpuidle,intel_idle: Fix CPUIDLE_FLAG_IRQ_ENABLE
- From: Jacob Pan <jacob.jun.pan@xxxxxxxxxxxxxxx>
- Date: Thu, 9 Jun 2022 16:49:21 -0700
- Cc: rth@xxxxxxxxxxxxxxx, ink@xxxxxxxxxxxxxxxxxxxx, mattst88@xxxxxxxxx, vgupta@xxxxxxxxxx, linux@xxxxxxxxxxxxxxx, ulli.kroll@xxxxxxxxxxxxxx, linus.walleij@xxxxxxxxxx, shawnguo@xxxxxxxxxx, Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>, kernel@xxxxxxxxxxxxxx, festevam@xxxxxxxxx, linux-imx@xxxxxxx, tony@xxxxxxxxxxx, khilman@xxxxxxxxxx, catalin.marinas@xxxxxxx, will@xxxxxxxxxx, guoren@xxxxxxxxxx, bcain@xxxxxxxxxxx, chenhuacai@xxxxxxxxxx, kernel@xxxxxxxxxx, geert@xxxxxxxxxxxxxx, sammy@xxxxxxxxx, monstr@xxxxxxxxx, tsbogend@xxxxxxxxxxxxxxxx, dinguyen@xxxxxxxxxx, jonas@xxxxxxxxxxxx, stefan.kristiansson@xxxxxxxxxxxxx, shorne@xxxxxxxxx, James.Bottomley@xxxxxxxxxxxxxxxxxxxxx, deller@xxxxxx, mpe@xxxxxxxxxxxxxx, benh@xxxxxxxxxxxxxxxxxxx, paulus@xxxxxxxxx, paul.walmsley@xxxxxxxxxx, palmer@xxxxxxxxxxx, aou@xxxxxxxxxxxxxxxxx, hca@xxxxxxxxxxxxx, gor@xxxxxxxxxxxxx, agordeev@xxxxxxxxxxxxx, borntraeger@xxxxxxxxxxxxx, svens@xxxxxxxxxxxxx, ysato@xxxxxxxxxxxxx, dalias@xxxxxxxx, davem@xxxxxxxxxxxxx, richard@xxxxxx, anton.ivanov@xxxxxxxxxxxxxxxxxx, johannes@xxxxxxxxxxxxxxxx, tglx@xxxxxxxxxxxxx, mingo@xxxxxxxxxx, bp@xxxxxxxxx, dave.hansen@xxxxxxxxxxxxxxx, x86@xxxxxxxxxx, hpa@xxxxxxxxx, acme@xxxxxxxxxx, mark.rutland@xxxxxxx, alexander.shishkin@xxxxxxxxxxxxxxx, jolsa@xxxxxxxxxx, namhyung@xxxxxxxxxx, jgross@xxxxxxxx, srivatsa@xxxxxxxxxxxxx, amakhalov@xxxxxxxxxx, pv-drivers@xxxxxxxxxx, boris.ostrovsky@xxxxxxxxxx, chris@xxxxxxxxxx, jcmvbkbc@xxxxxxxxx, rafael@xxxxxxxxxx, lenb@xxxxxxxxxx, pavel@xxxxxx, gregkh@xxxxxxxxxxxxxxxxxxx, mturquette@xxxxxxxxxxxx, sboyd@xxxxxxxxxx, daniel.lezcano@xxxxxxxxxx, lpieralisi@xxxxxxxxxx, sudeep.holla@xxxxxxx, agross@xxxxxxxxxx, bjorn.andersson@xxxxxxxxxx, anup@xxxxxxxxxxxxxx, thierry.reding@xxxxxxxxx, jonathanh@xxxxxxxxxx, Arnd Bergmann <arnd@xxxxxxxx>, yury.norov@xxxxxxxxx, andriy.shevchenko@xxxxxxxxxxxxxxx, linux@xxxxxxxxxxxxxxxxxx, rostedt@xxxxxxxxxxx, pmladek@xxxxxxxx, senozhatsky@xxxxxxxxxxxx, john.ogness@xxxxxxxxxxxxx, paulmck@xxxxxxxxxx, frederic@xxxxxxxxxx, quic_neeraju@xxxxxxxxxxx, josh@xxxxxxxxxxxxxxxx, mathieu.desnoyers@xxxxxxxxxxxx, jiangshanlai@xxxxxxxxx, joel@xxxxxxxxxxxxxxxxx, juri.lelli@xxxxxxxxxx, vincent.guittot@xxxxxxxxxx, dietmar.eggemann@xxxxxxx, bsegall@xxxxxxxxxx, mgorman@xxxxxxx, bristot@xxxxxxxxxx, vschneid@xxxxxxxxxx, jpoimboe@xxxxxxxxxx, linux-alpha@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, linux-snps-arc@xxxxxxxxxxxxxxxxxxx, linux-arm-kernel@xxxxxxxxxxxxxxxxxxx, linux-omap@xxxxxxxxxxxxxxx, linux-csky@xxxxxxxxxxxxxxx, linux-hexagon@xxxxxxxxxxxxxxx, linux-ia64@xxxxxxxxxxxxxxx, linux-m68k@xxxxxxxxxxxxxxx, linux-mips@xxxxxxxxxxxxxxx, openrisc@xxxxxxxxxxxxxxxxxxxx, linux-parisc@xxxxxxxxxxxxxxx, linuxppc-dev@xxxxxxxxxxxxxxxx, linux-riscv@xxxxxxxxxxxxxxxxxxx, linux-s390@xxxxxxxxxxxxxxx, linux-sh@xxxxxxxxxxxxxxx, sparclinux@xxxxxxxxxxxxxxx, linux-um@xxxxxxxxxxxxxxxxxxx, linux-perf-users@xxxxxxxxxxxxxxx, virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxxx, linux-xtensa@xxxxxxxxxxxxxxxx, linux-acpi@xxxxxxxxxxxxxxx, linux-pm@xxxxxxxxxxxxxxx, linux-clk@xxxxxxxxxxxxxxx, linux-arm-msm@xxxxxxxxxxxxxxx, linux-tegra@xxxxxxxxxxxxxxx, linux-arch@xxxxxxxxxxxxxxx, rcu@xxxxxxxxxxxxxxx, jacob.jun.pan@xxxxxxxxxxxxxxx
- In-reply-to: <20220608144516.172460444@infradead.org>
- Organization: OTC
- References: <20220608142723.103523089@infradead.org> <20220608144516.172460444@infradead.org>
Hi Peter,
On Wed, 08 Jun 2022 16:27:27 +0200, Peter Zijlstra <peterz@xxxxxxxxxxxxx>
wrote:
> Commit c227233ad64c ("intel_idle: enable interrupts before C1 on
> Xeons") wrecked intel_idle in two ways:
>
> - must not have tracing in idle functions
> - must return with IRQs disabled
>
> Additionally, it added a branch for no good reason.
>
> Fixes: c227233ad64c ("intel_idle: enable interrupts before C1 on Xeons")
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> ---
> drivers/idle/intel_idle.c | 48
> +++++++++++++++++++++++++++++++++++----------- 1 file changed, 37
> insertions(+), 11 deletions(-)
>
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -129,21 +137,37 @@ static unsigned int mwait_substates __in
> *
> * Must be called under local_irq_disable().
> */
nit: this comment is no long true, right?
> +
> -static __cpuidle int intel_idle(struct cpuidle_device *dev,
> - struct cpuidle_driver *drv, int index)
> +static __always_inline int __intel_idle(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int
> index) {
> struct cpuidle_state *state = &drv->states[index];
> unsigned long eax = flg2MWAIT(state->flags);
> unsigned long ecx = 1; /* break on interrupt flag */
>
> - if (state->flags & CPUIDLE_FLAG_IRQ_ENABLE)
> - local_irq_enable();
> -
> mwait_idle_with_hints(eax, ecx);
>
> return index;
> }
>
> +static __cpuidle int intel_idle(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int index)
> +{
> + return __intel_idle(dev, drv, index);
> +}
> +
> +static __cpuidle int intel_idle_irq(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int
> index) +{
> + int ret;
> +
> + raw_local_irq_enable();
> + ret = __intel_idle(dev, drv, index);
> + raw_local_irq_disable();
> +
> + return ret;
> +}
> +
> /**
> * intel_idle_s2idle - Ask the processor to enter the given idle state.
> * @dev: cpuidle device of the target CPU.
> @@ -1801,6 +1824,9 @@ static void __init intel_idle_init_cstat
> /* Structure copy. */
> drv->states[drv->state_count] =
> cpuidle_state_table[cstate];
> + if (cpuidle_state_table[cstate].flags &
> CPUIDLE_FLAG_IRQ_ENABLE)
> + drv->states[drv->state_count].enter =
> intel_idle_irq; +
> if ((disabled_states_mask & BIT(drv->state_count)) ||
> ((icpu->use_acpi || force_use_acpi) &&
> intel_idle_off_by_default(mwait_hint) &&
>
>
Thanks,
Jacob
[Index of Archives]
[Linux Kernel]
[Sparc Linux]
[DCCP]
[Linux ARM]
[Yosemite News]
[Linux SCSI]
[Linux x86_64]
[Linux for Ham Radio]