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

[Rafael J. Wysocki]

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).