All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alan Stern <stern@rowland.harvard.edu>
To: Colin Cross <ccross@android.com>
Cc: Randy Dunlap <randy.dunlap@oracle.com>,
	Len Brown <len.brown@intel.com>,
	Greg Kroah-Hartman <gregkh@suse.de>,
	linux-kernel@vger.kernel.org,
	linux-pm@lists.linux-foundation.org,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH] PM: Prevent waiting forever on asynchronous resume after abort
Date: Fri, 3 Sep 2010 10:04:14 -0400 (EDT)	[thread overview]
Message-ID: <Pine.LNX.4.44L0.1009030953060.1548-100000__21873.9396463663$1283522793$gmane$org@iolanthe.rowland.org> (raw)
In-Reply-To: <AANLkTindhyovNphyCfnSuqN8KogZ0APdWyL7T=vYHHPF@mail.gmail.com>

On Thu, 2 Sep 2010, Colin Cross wrote:

> You're right, wait_event would be much worse.
> 
> I think there's another race condition during suspend.  If an
> asynchronous device calls device_pm_wait_for_dev on a device that
> hasn't had device_suspend called on it yet, power.completion will
> still be set from initialization or the last time it completed resume,
> and it won't wait.

That can't happen in a properly-designed system.  It would mean the 
async device didn't suspend because it was waiting for a device which 
was registered before it -- and that would deadlock even if you used 
synchronous suspend.

> Assuming that problem is fixed somehow, there's also a deadlock
> possibility.  Consider 3 devices.  A, B, and C, registered in that
> order.  A is async, and the suspend handler calls
> device_pm_wait_for_dev(C).  B's suspend handler returns an error.  A's
> suspend handler is now stuck waiting on C->power.completion, but
> device_suspend(C) will never be called.

Why not?  The normal suspend order is last-to-first, so C will be 
suspended before B.

> There are also an unhandled edge condition - what is the expected
> behavior for a call to device_pm_wait_for_dev on a device if the
> suspend handler for that device returns an error?  Currently, the
> calling device will continue as if the target device had suspended.

It looks like __device_suspend needs to set async_error.  Which means 
async_suspend doesn't need to set it.  This is indeed a bug.

> What about splitting power.completion into two flags,
> power.suspend_complete and power.resume_complete?
> power.resume_complete is initialized to 1, because the devices start
> resumed.  Clear power.suspend_complete for all devices at the
> beginning of dpm_suspend, and clear power.resume_complete for any
> device that is suspended at the beginning of dpm_resume.  The
> semantics of each flag is then always clear.  Any time between the
> beginning and end of dpm_suspend, waiting on any device's
> power.suspend_complete will block until that device is in suspend.
> Any time between the beginning and end of dpm_resume, waiting on
> power.resume_complete will block IFF the device is suspended.

How are you going to wait for these things?  With wait_event?  Didn't 
you say above that it would be worse than using completions?

> A solution to the 2nd and 3rd problems would still be needed - a way
> to abort drivers that call device_pm_wait_for_dev when suspend is
> aborted, and a return value to tell them the device being waited on is
> not suspended.

No solutions are needed.  See above.

Alan Stern

  parent reply	other threads:[~2010-09-03 14:04 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-02  2:54 [PATCH] PM: Prevent waiting forever on asynchronous resume after abort Colin Cross
2010-09-02 13:50 ` Alan Stern
2010-09-02 13:50 ` Alan Stern
2010-09-02 19:46   ` Rafael J. Wysocki
2010-09-02 19:46   ` Rafael J. Wysocki
2010-09-02 20:24     ` Colin Cross
2010-09-02 20:30       ` Rafael J. Wysocki
2010-09-02 20:30       ` Rafael J. Wysocki
2010-09-02 20:45       ` Alan Stern
2010-09-02 20:45       ` Alan Stern
2010-09-02 21:01         ` Colin Cross
2010-09-02 21:01         ` Colin Cross
2010-09-02 21:06           ` Rafael J. Wysocki
2010-09-02 21:06             ` Rafael J. Wysocki
2010-09-02 21:34           ` Alan Stern
2010-09-02 22:45             ` Colin Cross
2010-09-02 23:09               ` Rafael J. Wysocki
2010-09-03  0:14                 ` Colin Cross
2010-09-03  0:14                 ` Colin Cross
2010-09-03  0:35                   ` Rafael J. Wysocki
2010-09-03  0:35                   ` Rafael J. Wysocki
2010-09-03  1:54                     ` Colin Cross
2010-09-03  1:54                     ` Colin Cross
2010-09-03  2:42                       ` Alan Stern
2010-09-03  4:30                         ` Colin Cross
2010-09-03 14:04                           ` Alan Stern
2010-09-03 16:48                             ` Colin Cross
2010-09-03 17:31                               ` Alan Stern
2010-09-03 17:31                               ` Alan Stern
2010-09-16 20:36                                 ` [PATCH] PM: Fix potential issue with failing asynchronous suspend (was: Re: [PATCH] PM: Prevent waiting ...) Rafael J. Wysocki
2010-09-16 20:36                                 ` Rafael J. Wysocki
2010-09-16 21:00                                   ` Alan Stern
2010-09-16 21:24                                     ` Rafael J. Wysocki
2010-09-16 21:24                                     ` Rafael J. Wysocki
2010-09-16 21:00                                   ` Alan Stern
2010-09-03 16:48                             ` [PATCH] PM: Prevent waiting forever on asynchronous resume after abort Colin Cross
2010-09-03 14:04                           ` Alan Stern [this message]
2010-09-03  4:30                         ` Colin Cross
2010-09-03  2:42                       ` Alan Stern
2010-09-02 23:09               ` Rafael J. Wysocki
2010-09-02 22:45             ` Colin Cross
2010-09-02 21:34           ` Alan Stern
2010-09-02 21:05       ` Rafael J. Wysocki
2010-09-02 21:05         ` Rafael J. Wysocki
2010-09-02 21:31         ` Colin Cross
2010-09-02 21:40           ` Rafael J. Wysocki
2010-09-02 22:59             ` [PATCH v2] " Colin Cross
2010-09-02 23:25               ` Rafael J. Wysocki
2010-09-02 23:25                 ` Rafael J. Wysocki
2010-09-02 22:59             ` Colin Cross
2010-09-02 21:40           ` [PATCH] " Rafael J. Wysocki
2010-09-02 21:31         ` Colin Cross
2010-09-02 20:24     ` Colin Cross
2010-09-02 20:27     ` Alan Stern
2010-09-02 20:27     ` Alan Stern
  -- strict thread matches above, loose matches on Subject: below --
2010-09-02  2:54 Colin Cross

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='Pine.LNX.4.44L0.1009030953060.1548-100000__21873.9396463663$1283522793$gmane$org@iolanthe.rowland.org' \
    --to=stern@rowland.harvard.edu \
    --cc=akpm@linux-foundation.org \
    --cc=ccross@android.com \
    --cc=gregkh@suse.de \
    --cc=len.brown@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@lists.linux-foundation.org \
    --cc=randy.dunlap@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.