From mboxrd@z Thu Jan 1 00:00:00 1970 From: Colin Cross Subject: Re: [PATCH] PM: Prevent waiting forever on asynchronous resume after abort Date: Thu, 2 Sep 2010 15:45:19 -0700 Message-ID: References: Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-pm-bounces@lists.linux-foundation.org Errors-To: linux-pm-bounces@lists.linux-foundation.org To: Alan Stern Cc: Randy Dunlap , Len Brown , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, linux-pm@lists.linux-foundation.org, Andrew Morton List-Id: linux-pm@vger.kernel.org On Thu, Sep 2, 2010 at 2:34 PM, Alan Stern wrot= e: > On Thu, 2 Sep 2010, Colin Cross wrote: > >> That would work, but I still don't see why it's better. =A0With either >> of your changes, the power.completion variable is storing state, and >> not just used for notification. =A0However, 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. =A0Setting >> 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. =A0Instead of using a completion I > suppose we could have used a new "transition_complete" variable > together with a waitqueue. =A0Would you prefer that? =A0It'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. 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.