From: Kazuki <kazukih0205@gmail.com> To: Sudeep Holla <sudeep.holla@arm.com> Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, "Rafael J. Wysocki" <rafael@kernel.org>, Daniel Lezcano <daniel.lezcano@linaro.org>, Lorenzo Pieralisi <lpieralisi@kernel.org>, Hector Martin <marcan@marcan.st>, Sven Peter <sven@svenpeter.dev>, Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz> Subject: Re: s2idle breaks on machines without cpuidle support Date: Wed, 8 Feb 2023 20:20:31 +0900 [thread overview] Message-ID: <20230208112031.sdfuluajjerf4wwp@kazuki-mac> (raw) In-Reply-To: <20230208103511.w7jzxw6spy6humdn@bogus> 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]. [1] Documentation/core-api/timekeeping.rst: .. c:function:: ktime_t ktime_get( void ) CLOCK_MONOTONIC Useful for reliable timestamps and measuring short time intervals accurately. Starts at system boot time but stops during suspend. [2] kernel/time/sched_clock.c: int sched_clock_suspend(void) { struct clock_read_data *rd = &cd.read_data[0]; update_sched_clock(); hrtimer_cancel(&sched_clock_timer); rd->read_sched_clock = suspended_sched_clock_read; return 0; } void sched_clock_resume(void) { struct clock_read_data *rd = &cd.read_data[0]; rd->epoch_cyc = cd.actual_read_sched_clock(); hrtimer_start(&sched_clock_timer, cd.wrap_kt, HRTIMER_MODE_REL_HARD); rd->read_sched_clock = cd.actual_read_sched_clock; } static struct syscore_ops sched_clock_ops = { .suspend = sched_clock_suspend, .resume = sched_clock_resume, }; static int __init sched_clock_syscore_init(void) { register_syscore_ops(&sched_clock_ops); return 0; } device_initcall(sched_clock_syscore_init); > > > > > > > > 2. Suspend actually works on ARM64 machines even without proper > > > > cpuidle (PSCI cpuidle) since they support wfi, so the assumption here is wrong > > > > on such machines > > > > > > > > > > Sorry I am bit confused here. Your point (2) contradicts the $subject. > > drivers/cpuidle/cpuidle.c: > > > > bool cpuidle_not_available(struct cpuidle_driver *drv, > > struct cpuidle_device *dev) > > { > > return off || !initialized || !drv || !dev || !dev->enabled; > > } > > > > The cpuidle framework reports ARM64 devices without PSCI cpuidle as > > "cpuidle not available" even when they support wfi, which causes suspend > > to fail, which shouldn't be happening since they do support idling. > > Yes with just WFI, there will be no active cpuidle driver. > > [...] > > > > Again, since s2idle is userspace driven, I don't understand what do you > > > mean by unbootable kernel in the context of s2idle. > > > > Sorry, I meant "attempts to fix this bug have all led to an unbootable > > kernel." > > Again I assume you mean kernel hang or crash and nothing to do with boot. > Once you enter s2i state with your changes/fix, it hangs or is unresponsive > as it might have either failed to enter or resume from the state. > > -- > Regards, > Sudeep Thanks, Kazuki _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Kazuki <kazukih0205@gmail.com> To: Sudeep Holla <sudeep.holla@arm.com> Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, "Rafael J. Wysocki" <rafael@kernel.org>, Daniel Lezcano <daniel.lezcano@linaro.org>, Lorenzo Pieralisi <lpieralisi@kernel.org>, Hector Martin <marcan@marcan.st>, Sven Peter <sven@svenpeter.dev>, Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz> Subject: Re: s2idle breaks on machines without cpuidle support Date: Wed, 8 Feb 2023 20:20:31 +0900 [thread overview] Message-ID: <20230208112031.sdfuluajjerf4wwp@kazuki-mac> (raw) In-Reply-To: <20230208103511.w7jzxw6spy6humdn@bogus> 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]. [1] Documentation/core-api/timekeeping.rst: .. c:function:: ktime_t ktime_get( void ) CLOCK_MONOTONIC Useful for reliable timestamps and measuring short time intervals accurately. Starts at system boot time but stops during suspend. [2] kernel/time/sched_clock.c: int sched_clock_suspend(void) { struct clock_read_data *rd = &cd.read_data[0]; update_sched_clock(); hrtimer_cancel(&sched_clock_timer); rd->read_sched_clock = suspended_sched_clock_read; return 0; } void sched_clock_resume(void) { struct clock_read_data *rd = &cd.read_data[0]; rd->epoch_cyc = cd.actual_read_sched_clock(); hrtimer_start(&sched_clock_timer, cd.wrap_kt, HRTIMER_MODE_REL_HARD); rd->read_sched_clock = cd.actual_read_sched_clock; } static struct syscore_ops sched_clock_ops = { .suspend = sched_clock_suspend, .resume = sched_clock_resume, }; static int __init sched_clock_syscore_init(void) { register_syscore_ops(&sched_clock_ops); return 0; } device_initcall(sched_clock_syscore_init); > > > > > > > > 2. Suspend actually works on ARM64 machines even without proper > > > > cpuidle (PSCI cpuidle) since they support wfi, so the assumption here is wrong > > > > on such machines > > > > > > > > > > Sorry I am bit confused here. Your point (2) contradicts the $subject. > > drivers/cpuidle/cpuidle.c: > > > > bool cpuidle_not_available(struct cpuidle_driver *drv, > > struct cpuidle_device *dev) > > { > > return off || !initialized || !drv || !dev || !dev->enabled; > > } > > > > The cpuidle framework reports ARM64 devices without PSCI cpuidle as > > "cpuidle not available" even when they support wfi, which causes suspend > > to fail, which shouldn't be happening since they do support idling. > > Yes with just WFI, there will be no active cpuidle driver. > > [...] > > > > Again, since s2idle is userspace driven, I don't understand what do you > > > mean by unbootable kernel in the context of s2idle. > > > > Sorry, I meant "attempts to fix this bug have all led to an unbootable > > kernel." > > Again I assume you mean kernel hang or crash and nothing to do with boot. > Once you enter s2i state with your changes/fix, it hangs or is unresponsive > as it might have either failed to enter or resume from the state. > > -- > Regards, > Sudeep Thanks, Kazuki
next prev parent reply other threads:[~2023-02-08 11:21 UTC|newest] Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-02-04 15:27 s2idle breaks on machines without cpuidle support Kazuki 2023-02-04 15:27 ` Kazuki 2023-02-06 10:12 ` Sudeep Holla 2023-02-06 10:12 ` Sudeep Holla 2023-02-07 19:48 ` Kazuki 2023-02-07 19:48 ` Kazuki 2023-02-08 10:35 ` Sudeep Holla 2023-02-08 10:35 ` Sudeep Holla 2023-02-08 11:20 ` Kazuki [this message] 2023-02-08 11:20 ` Kazuki 2023-02-08 14:16 ` Sudeep Holla 2023-02-08 14:16 ` Sudeep Holla 2023-02-08 14:43 ` Kazuki 2023-02-08 14:43 ` Kazuki 2023-02-08 15:03 ` Sudeep Holla 2023-02-08 15:03 ` Sudeep Holla 2023-02-08 15:19 ` Kazuki 2023-02-08 15:19 ` Kazuki 2023-02-08 15:34 ` Sudeep Holla 2023-02-08 15:34 ` Sudeep Holla 2023-02-08 15:42 ` Kazuki 2023-02-08 15:42 ` Kazuki 2023-02-08 14:52 ` Kazuki 2023-02-08 14:52 ` Kazuki 2023-02-08 15:42 ` Hector Martin 2023-02-08 15:42 ` Hector Martin 2023-02-08 16:18 ` Sudeep Holla 2023-02-08 16:18 ` Sudeep Holla 2023-02-08 16:45 ` Hector Martin 2023-02-08 16:45 ` Hector Martin 2023-09-07 19:11 ` Florian Fainelli 2023-09-07 19:11 ` Florian Fainelli
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20230208112031.sdfuluajjerf4wwp@kazuki-mac \ --to=kazukih0205@gmail.com \ --cc=daniel.lezcano@linaro.org \ --cc=len.brown@intel.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=lpieralisi@kernel.org \ --cc=marcan@marcan.st \ --cc=pavel@ucw.cz \ --cc=rafael@kernel.org \ --cc=sudeep.holla@arm.com \ --cc=sven@svenpeter.dev \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.