Messages in this thread | | | From | Kazuki <> | Date | Wed, 8 Feb 2023 23:43:27 +0900 | Subject | Re: s2idle breaks on machines without cpuidle support |
| |
On Wed, Feb 08, 2023 at 02:16:58PM +0000, Sudeep Holla wrote: > On Wed, Feb 08, 2023 at 08:20:31PM +0900, Kazuki wrote: > > On Wed, Feb 08, 2023 at 10:35:11AM +0000, Sudeep Holla wrote: > > > On Wed, Feb 08, 2023 at 04:48:18AM +0900, Kazuki wrote: > > > > On Mon, Feb 06, 2023 at 10:12:39AM +0000, Sudeep Holla wrote: > > > > > > > > > > What do you mean by break ? More details on the observation would be helpful. > > > > For example, CLOCK_MONOTONIC doesn't stop even after suspend since > > > > these chain of commands don't get called. > > > > > > > > call_cpuidle_s2idle->cpuidle_enter_s2idle->enter_s2idle_proper->tick_freeze->sched_clock_suspend (Function that pauses CLOCK_MONOTONIC) > > > > > > > > Which in turn causes programs like systemd to crash since it doesn't > > > > expect this. > > > > > > Yes expected IIUC. The per-cpu timers and counters continue to tick in > > > WFI and hence CLOCK_MONOTONIC can't stop. > > Yes, but it shouldn't be the case when suspending[1]. Currently that's what > > happens when we enter s2idle without a cpuidle driver. This doesn't seem > > to happen with S3 sleep [2]. > > > > Correct, but check the requirements to use syscore operations(mainly > syscore_suspend/resume where only one CPU is online with interrupts > disabled. In case of s2idle, all CPUs are idling and not offlined as > required by the syscore operations and hence it can't be used. > > I was about ask you earlier as why can't you implement just system > suspend in PSCI where the last cpu just calls WFI if you are interested > in system sleep state. Or you can implement CPU_SUSPEND with an additional > retention state which enters PSCI implementation just to make sure there is > an active cpuidle driver and the s2idle state machinery works as expected. The machine I have (Macbook with Apple M1) doesn't have PSCI. > It is built with those requirements and trying to work it out for WFI without > any idle driver or firmware implementation to back it up is just not going > to work. Any reason why that's the case?
I guess we should ensure that systems without a cpuidle driver will not suspend maybe around here then.
kernel/power/main.c:
static ssize_t mem_sleep_store(struct kobject *kobj, struct kobj_attribute *attr, const char *buf, size_t n) { suspend_state_t state; int error;
error = pm_autosleep_lock(); if (error) return error;
if (pm_autosleep_state() > PM_SUSPEND_ON) { error = -EBUSY; goto out; }
state = decode_suspend_state(buf, n); if (state < PM_SUSPEND_MAX && state > PM_SUSPEND_ON) mem_sleep_current = state; else error = -EINVAL;
out: pm_autosleep_unlock(); return error ? error : n; } > > -- > Regards, > Sudeep Thanks, Kazuki
| |