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

[Rafael J. Wysocki]

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.