Messages in this thread | | | From | Kelly Rossmoyer <> | Date | Fri, 28 Jan 2022 23:52:52 -0800 | Subject | Re: [RFC] PM: suspend: Upstreaming wakeup reason capture support |
| |
On Tue, Jan 25, 2022 at 9:09 PM Zichar Zhang <zichar.zhang@linaro.org> wrote: > > hello John, Kelly!
Hello! Happy to see what the future may bring in this space, and looking forward to what we can all do to move this area forward.
> -- If these IRQs do hanpen, the code in this commit will catch > them as "unknown wakeup reason" and suggest user to check the > kernel log for more information.
I would argue that - at least in Android context, as that's what I know - the kernel log is not a substitute for wakeup reason capture. 1) It is common to find yourself troubleshooting battery life issues for which kernel log data is not unavailable. 2) Troubleshooting is not the only consideration. Since "why aren't we in suspend right now?" is such a key question for mobile device battery life, this is also about power attribution. And I think trying to build that upon live kernel log parsing would be both inefficient and brittle. (But my lack of knowledge is vast, so maybe that's a solvable problem?)
> >> * abort reasons, including: > >> * wakeup_source activity > >> * failure to freeze userspace > >> * failure to suspend devices > >> * failed syscore_suspend callback > > -- As mentioned before, if these "abort action" happened, you > can catch string "unknown wakeup reason", and check the kernel > log then.
I don't think the kernel log is a solution here. Suspend aborts can be a significant fraction of how suspend attempts end, making them a key contributor to battery drain. If the eventual set of patches doesn't solve for at least the most common kinds of suspend aborts, that leaves a lot of power observability off the table. And again, at least on Android, kernel log content is often not available for the interval when a series of suspend aborts contributed to power drain.
> The patch is not complete, these is the next steps: > 1. add interface to show time spend in suspend/resume work. > (after this, I think it could be works and replace the android patch)
I believe Android would experience a significant regression in capability with only those two patches implemented, so that would require a lot of careful consideration.
> 2. we need solve the "unmapped HW IRQs", "misconfigured IRQs" and > "threaded IRQs" problem. > (this is the hardest one)
FWIW, I'm already expecting unmapped HW IRQs and misconfigured IRQs not to make the cut. Those have helped solve real and recurring issues, but they're admittedly niche solutions to infrequent problems at the expense of messy code coupling, so... maybe not broadly beneficial for Linux.
Threaded wakeup IRQs are a different matter. As one example, there are architectures in which the RTC wakeup IRQ is threaded and knowing "this string of wakeups was due to the RTC" is a lot more useful than "this string of wakeups was due to an irqchip with tens of child IRQs".
> ... So it just report a "wakeup reason" from > interrupt subsystem. It just a coincidence that most hardware "wakeup > reason" is also the interupt signal. Even the "interrupt" and "wake up" > signal are separated from each other in GIC700. > So give them a chance to report the their "wakeup reason".)
Agreed. But I do hope we can find a way to bring them together so a single story is presented to userspace. Or, barring that, at least make it easy for userspace to figure that out after the fact.
Thanks so much for the work you are doing on this and taking the time to walk me through your thoughts. I'm happy to contribute in whatever manner adds value.
--
Kelly Rossmoyer | Software Engineer | krossmo@google.com | 858-239-4111
| |