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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 2 Sep 2010 13:24:28 -0700
From:	Colin Cross <ccross@...roid.com>
To:	"Rafael J. Wysocki" <rjw@...k.pl>
Cc:	Alan Stern <stern@...land.harvard.edu>,
	linux-kernel@...r.kernel.org, linux-pm@...ts.linux-foundation.org,
	Pavel Machek <pavel@....cz>, Len Brown <len.brown@...el.com>,
	Greg Kroah-Hartman <gregkh@...e.de>,
	Randy Dunlap <randy.dunlap@...cle.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] PM: Prevent waiting forever on asynchronous resume after abort

On Thu, Sep 2, 2010 at 12:46 PM, Rafael J. Wysocki <rjw@...k.pl> wrote:
> On Thursday, September 02, 2010, Alan Stern wrote:
>> On Wed, 1 Sep 2010, Colin Cross wrote:
>>
>> > Only wait on a parent device during resume if the parent device is
>> > suspended.
>> >
>> > Consider three drivers, A, B, and C.  The parent of A is C, and C
>> > has async_suspend set.  On boot, C->power.completion is initialized
>> > to 0.
>> >
>> > During the first suspend:
>> > suspend_devices_and_enter(...)
>> >  dpm_resume(...)
>> >   device_suspend(A)
>> >   device_suspend(B) returns error, aborts suspend
>> >  dpm_resume_end(...)
>> >    dpm_resume(...)
>> >     device_resume(A)
>> >      dpm_wait(A->parent == C)
>> >       wait_for_completion(C->power.completion)
>> >
>> > The wait_for_completion will never complete, because
>> > complete_all(C->power.completion) will only be called from
>> > device_suspend(C) or device_resume(C), neither of which is called
>> > if suspend is aborted before C.
>>
>> This would work okay if C->power.completion had been initialized to the
>> completed state during boot, right?
>>
>> > After a successful suspend->resume cycle, where B doesn't abort
>> > suspend, C->power.completion is left in the completed state by the
>> > call to device_resume(C), and the same call path will work if B
>> > aborts suspend.
>> >
>> > Signed-off-by: Colin Cross <ccross@...roid.com>
>> > ---
>> >  drivers/base/power/main.c |    3 ++-
>> >  1 files changed, 2 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
>> > index cb784a0..e159910 100644
>> > --- a/drivers/base/power/main.c
>> > +++ b/drivers/base/power/main.c
>> > @@ -526,7 +526,8 @@ static int device_resume(struct device *dev, pm_message_t state, bool async)
>> >     TRACE_DEVICE(dev);
>> >     TRACE_RESUME(0);
>> >
>> > -   dpm_wait(dev->parent, async);
>> > +   if (dev->parent && dev->parent->power.status >= DPM_OFF)
>> > +           dpm_wait(dev->parent, async);
>> >     device_lock(dev);
>> >
>> >     dev->power.status = DPM_RESUMING;
>>
>> I think it would be better to change device_pm_init() and add a
>> complete_all().
>
> I agree.
That would work, and was my first solution, but it increases the
reliance on the completion variable being left completed between state
transitions, which is undocumented and unnecessary.  It seems more
straightforward to me to only wait on the parent if the parent is
suspended.

> Who's writing the patch?
I'll write it if you still don't like this one.


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