genpd: domains can suspend asynchronously
Hi,
I reported this previously in the midst of another tangentially-related thread, so it could easily have gotten lost. I thought I'd repeat it here.
---
I noticed the genpd code has comments like this scattered all over:
* This function is only called in "noirq" and "syscore" stages of system power
* transitions, so it need not acquire locks (all of the "noirq" callbacks are
* executed sequentially, so it is guaranteed that it will never run twice in
* parallel).
Isn't that no longer true, now that noirq suspend can be asynchronous? Maybe we should grep for the phrase "need not acquire locks" throughout the kernel, in order to find low-hanging fruit for race conditions :)
---
Anyway, I was considering hacking my way through the subsystem here sometime, but there's no guarantee. And in case someone else wanted to fix this up, I figured I'd least put the report out there.
NB: I noticed there are more drivers enabling asynchronous suspend these days. Some USB, SDIO, and MMC host controllers seem ripe for triggering bugs here.
Brian
Quoted 10 lines · Brian Norris
Hi,
I reported this previously in the midst of another tangentially-related thread, so it could easily have gotten lost. I thought I'd repeat it here.
---
I noticed the genpd code has comments like this scattered all over:
Quoted 8 lines
* This function is only called in "noirq" and "syscore" stages of system power
* transitions, so it need not acquire locks (all of the "noirq" callbacks are
* executed sequentially, so it is guaranteed that it will never run twice in
* parallel).Isn't that no longer true, now that noirq suspend can be asynchronous? Maybe we should grep for the phrase "need not acquire locks" throughout the kernel, in order to find low-hanging fruit for race conditions :)
Thanks for the reminder. I have been starring at the comment for several times, but never come around to send a patch. I will fix it asap.
Moreover, genpd should also respect driver's ->suspend|resume_noirq() callbacks(), which is another related change that should be done. Let's me fix this as well.
Quoted 6 lines
---
Anyway, I was considering hacking my way through the subsystem here sometime, but there's no guarantee. And in case someone else wanted to fix this up, I figured I'd least put the report out there.
I can deal with, and allow me to add your reported-by/suggested-by tag.
Quoted 6 lines
NB: I noticed there are more drivers enabling asynchronous suspend these days. Some USB, SDIO, and MMC host controllers seem ripe for triggering bugs here.
Brian
Kind regards
Uffe