On Thursday, October 07, 2010, Alan Stern wrote:
> On Thu, 7 Oct 2010, Kevin Hilman wrote:
>
> > My confusion is not about the use of spinlocks, it's a question of what
> > is being busy-waited for, and the thread that is being waited for is
> > going to complete when interrupts are disabled.
> >
> > Sorry to be dense, but can you (re)summarize what you're proposing as I
> > think I'm getting mixed up with all the various options we've been
> > tossing around.
> >
> > If it can work, I'm certainly in favor of a busy-wait approach as it
> > really ensures that sync requests are handled quickly.
>
> Okay, here's the story in a nutshell. Allowing a subsystem's or
> driver's runtime-PM callbacks to run with interrupts disabled faces two
> obstacles:
>
> (1): We don't want two different CPUs to run callbacks for the
> same device at the same time. So if a callback is already
> running on one CPU (i.e., if the device's runtime status is
> either SUSPENDING or RESUMING) then another CPU can't be
> allowed to invoke a callback.
>
> Thus, you can't do a synchronous pm_runtime_resume_irq()
> if the device is in the middle of a suspend or resume
> operation. We're left with two choices: Fail the synchronous
> call and force the driver to defer matters to a workqueue
> (possibly masking an IRQ line in the meantime), or busy-wait
> until the concurrent operation finishes.
>
> If the PM core simply avoids releasing dev->power.lock before
> invoking the runtime_suspend or runtime_resume callback, the
> end result is almost the same as with busy-waiting.
This is slightly more complicated, because suspend is a bit different from
resume. I think it's generally acceptable to refuse to do a "fast path" suspend
if the device state is not RPM_ACTIVE at the moment, but refusing to do
a "fast path" resume may be more problematic (more on that later).
> (2): In general we can't resume a device if its parent is suspended.
> If the parent's runtime_resume routine needs to run with
> interrupts enabled then there's no way to resume the device
> while keeping interrupts disabled.
>
> Possible solutions involve, again, deferring matters to a
> workqueue, or else simply not allowing the situation to arise
> in the first place (forbid a device to have interrupt-disabled
> callbacks unless its parent does too or the parent doesn't use
> runtime PM at all).
>
> In general I'm against the solutions that require a workqueue.
OK
> Raphael appears to favor workqueues for (1) and be against them for (2).
The particular case we need to handle (I think) is that some devices need to be
resumed "atomically" as a part of bringing a CPU package out of a C-like-state
(Kevin, is that correct?). In that case not only we can't defer the resume (by
using a workqueue), but also we have to finish the resume within strict time
constraints (the time to leave the given C-like-state is one of the parameters
used by cpuidle governors to decide which state to choose).
On the other hand, busy waiting (by looping) in the case the state is
RPM_RESUMING is as though the callbacks were executed under a spinlock and we
happened to wait on it. I'd prefer to avoid that. Moreover, if the state is
RPM_SUSPENDING at the time a "fast path" resume is started, we are almost
guaranteed to violate the time constraints (by busy waiting for the suspend to
complete and carrying out the resume).
So, here's an idea:
First, let's introduce two flags in struct dev_pm_info, irq_safe and
fast_resume. The former has to be set so that the things above work.
Second, let's add a new flag to pass to __pm_runtime_{suspend|resume}(),
RPM_FAST_RESUME such that if it is passed to __pm_runtime_suspend(), it
causes fast_resume to be set for the given device (if the status is already
RPM_SUSPENDED, it just sets the flag, otherwise it suspends the device
normally and then sets the flag). Now, if fast_resume is set,
__pm_runtime_resume() has to be passed RPM_FAST_RESUME, or it will
fail (it also will fail if passed RPM_FAST_RESUME and fast_resume isn't
set). In that case the status has to be RPM_SUSPENDED or RPM_RESUMING
(otherwise fast_resume won't be set) and if it is RPM_RESUMING, this means
that the other resume has been called with RPM_FAST_RESUME too (it would
fail otherwise), so it's fine to busy loop until the status is RPM_SUSPENDED
(it's the caller's problem to avoid that). RPM_FAST_RESUME causes
__pm_runtime_resume() to avod turning interrupts on.
If necessary, there may be a flag for __pm_runtime_suspend() that will
carry out "atomic" suspend, but I'm not sure if that's necessary. Kevin?
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
