Re: [PATCH 1/3] cpuidle: Add enable_cpuidle() interface

[Rafael J. Wysocki]

On Tue, Mar 31, 2026 at 2:48 PM lihuisong (C) <lihuisong@xxxxxxxxxx> wrote:
>
>
> On 3/31/2026 8:10 PM, Rafael J. Wysocki wrote:
> > On Tue, Mar 31, 2026 at 2:05 PM Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote:
> >> Hi,
> >>
> >> On Tue, Mar 31, 2026 at 2:01 PM lihuisong (C) <lihuisong@xxxxxxxxxx> wrote:
> >>>
> >>> On 3/28/2026 12:06 PM, lihuisong (C) wrote:
> >>>> On 3/27/2026 7:33 PM, Rafael J. Wysocki wrote:
> >>>>> On Fri, Mar 27, 2026 at 7:23 AM lihuisong (C) <lihuisong@xxxxxxxxxx>
> >>>>> wrote:
> >>>>>> On 3/26/2026 9:39 PM, Rafael J. Wysocki wrote:
> >>>>>>> On Thu, Mar 26, 2026 at 1:17 PM lihuisong (C)
> >>>>>>> <lihuisong@xxxxxxxxxx> wrote:
> >>>>>>>> Hi Rafael,
> >>>>>>>>
> >>>>>>>> On 1/30/2026 9:59 AM, lihuisong (C) wrote:
> >>>>>>>>> Hi Rafael,
> >>>>>>>>>
> >>>>>>>>> On 1/15/2026 8:18 PM, lihuisong (C) wrote:
> >>>>>>>>>> On 1/15/2026 3:18 AM, Rafael J. Wysocki wrote:
> >>>>>>>>>>> On Tue, Nov 25, 2025 at 8:29 AM Huisong Li <lihuisong@xxxxxxxxxx>
> >>>>>>>>>>> wrote:
> >>>>>>>>>>>> The global switch of cpuidle can be turned back on in some case.
> >>>>>>>>>>>> So add enable_cpuidle().
> >>>>>>>>>>> No, this is not going to work.  The "off" switch only affects
> >>>>>>>>>>> initialization AFAICS.
> >>>>>>>>>> I think it would be work.
> >>>>>>>>>> The cpuidle_not_available() also see the "off" on do_idle().
> >>>>>>>>>> And cpuidle_idle_call() check this function first and then select
> >>>>>>>>>> idle state.
> >>>>>>>>>> Cpuidle doesn't select and enter idle state if this fuction
> >>>>>>>>>> return true.
> >>>>>>>>> I verified that disable_cpuidle() effectively prevents all CPUs from
> >>>>>>>>> entering any idle states and the cpuidle function is correctly
> >>>>>>>>> restored after calling enable_cpuidle().
> >>>>>>>>> What do you think?
> >>>>>>>> Could you pleasetake a look atmy reply?
> >>>>>>>> If not ok, I will drop this from my upstream list.
> >>>>>>> Sorry, can you please remind me what problem you wanted to address
> >>>>>>> with the help of this?
> >>>>>> Regarding the discussion in the link[1], if driver fail to get power
> >>>>>> info in power notify,
> >>>>>> the old idle states may no longer be reliable. Therefore, patch 2/3
> >>>>>> disables ACPI idle
> >>>>>> via the new interface introduced in patch 1/3.
> >>>>>>
> >>>>>> However, our discussion on whether this new interface can disable ACPI
> >>>>>> idle has not yet reached a conclusion.
> >>>>>> Could you please revisit this thread? It's quite brief, and I'd
> >>>>>> appreciate your further input.
> >>>>>>
> >>>>>> [1]
> >>>>>> https://lore.kernel.org/all/20251103084244.2654432-1-lihuisong@xxxxxxxxxx
> >>>>>>
> >>>>> The "off" variable has been intended for disabling cpuidle via kernel
> >>>>> command line (note that the corresponding module param is read-only).
> >>>>>
> >>>>> disable_cpuidle() is only used by Xen now and only at the setup/init
> >>>>> stage.
> >>>>>
> >>>>> I don't think that using it on idle state list change notifications is
> >>>>> a good idea.
> >>>> Understand.
> >>>>
> >>>>> Something like cpuidle_pause_and_lock() would be a better match I
> >>>>> think.  acpi_processor_hotplug() uses it already for a similar
> >>>>> purpose.
> >>>> Interfaces like cpuidle_pause_and_lock() and cpuidle_pause() disable
> >>>> cpuidle by clearing the global "initialized" flag,
> >>>> which requires "enabled_devices" to be non-zero.
> >>>> IIUC, cpuidle_disable_device() isn't called when a CPU goes offline;
> >>>> instead, it's handled in acpi_processor_hotplug() during online.
> >>>> This means "enabled_devices" stays above zero even if some CPUs are
> >>>> offline.
> >>>> In this case, the driver can still successfully set initialized to
> >>>> zero when get power information failed in power notify.
> >>>> So we can disable APCI idle on all CPUs.
> >>>> But we need to ensure that other threads wouldn't resume the
> >>>> "initialized" flag by interfaces like cpuidle_resume().
> >>>>
> >>>> I have found a scenario for that where acpi_processor_hotplug enables
> >>>> the cpuidle_device and restores the "initialized" value.
> >>>> In this case, special processing may be required.
> >>>> For example, the cpuidle state of the cpuidle driver also need
> >>>> reinitialize or the disable cpuilde state is still maintained.
> >>>>
> >>> Hi Rafel,
> >>>
> >>> I have thought about this issue for a long time. I feel that it is a bit
> >>> tricky to handle.
> >>>
> >>> First, if the power information fails to be obtained from the power
> >>> notify, we want to disable the ACPI idle of all CPUs by calling
> >>> cpuidle_pause(). However, in the CPU hotplug scenario, when the idle
> >>> state may be unavailable, the idle state needs to be set up again.
> >>>
> >>> Second, I found that the current driver only calls
> >>> acpi_processor_setup_cpuidle_states() to update the idle states in
> >>> acpi_idle_driver.
> >>> Other variables in acpi_idle_driver also need to be initialized again.
> >>> For example, target_residency_ns and exit_latency_ns need to be updated.
> >>> For details, see the implementation of __cpuidle_register_driver().
> >>> These variables are used when selecting the state to enter (please see
> >>> cpuidle_enter_state()).
> >>> Therefore, when the idle states information changes, the idle states we
> >>> actually use are still the old values.
> >>> This is also a problem.
> >>>
> >>> I am not sure whether we need to remove all cpuidle_device, remove
> >>> acpi_idle_driver, and then re-initialize and register acpi_idle_driver
> >>> and register all cpuidle_device in power notify.
> >>> I think it is very likely that we need to do so.
> >> Well, yes, we do, but only if the list of available states has
> >> actually changed (that is, the number of states has changed or the
> >> latency values have changed).
> > What actually needs to be done there is to unregister the cpuidle
> > driver and register it again because the list of idle states is a
> > property of the driver.
>
> We also need to unregister and register cpuidle device again.
> Because the state count of ACPI idle driver may be changed, some
> releated sysfs need to be recreated.

Ah, right.  So all cpuidle needs to be torn down and re-created from
scratch then.

I'm starting to wonder if that's really worth it.