Re: [PATCH] PM: Prevent waiting forever on asynchronous resume after abort

[Rafael J. Wysocki]

On Friday, September 03, 2010, Colin Cross wrote:
> On Thu, Sep 2, 2010 at 2:34 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> > On Thu, 2 Sep 2010, Colin Cross wrote:
> >
> >> That would work, but I still don't see why it's better.  With either
> >> of your changes, the power.completion variable is storing state, and
> >> not just used for notification.  However, the exact meaning of that
> >> state is unclear, especially during the transition from an aborted
> >> suspend to resume, and the state is duplicating power.status.  Setting
> >> it to complete in dpm_prepare is especially confusing, because at that
> >> point nothing is completed, it hasn't even been started.
> >
> > The state being waited for varies from time to time and is only
> > partially related to power.status.  Instead of using a completion I
> > suppose we could have used a new "transition_complete" variable
> > together with a waitqueue.  Would you prefer that?  It's effectively
> > the same thing as a completion, but without the nice packaging already
> > provided by the kernel.
> No, that doesn't change anything.  What I'd prefer to see is a
> wait_for_condition on the desired state of the parent.  As is,
> power.completion means one thing during suspend (the device has
> started, but not finished, suspending), and a different thing during
> resume (the device has not finished resuming, and may not have started
> resuming).  That difference is exactly what caused the bug - the
> completion has to be set on init so that it is set before the device
> starts suspend.

Not really.  The bug is there, because my analysis of the suspend error code
path was wrong.  Sorry about that, but it has nothing to do with the "different
meaning" of the completions during suspend and resume.

The completions here are simply used to enforce a specific ordering of
operations, nothing more.  They have no meaning beyond that.

> I'll send the complete_all on init patch, as it's the only way to fix
> the problem given the current implementation of dpm_wait.

Great, thanks.

Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/