| Message ID | 11E08D716F0541429B7042699DD5C1A16B97BD8A@FMSMSX103.amr.corp.intel.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Fri, May 17, 2013 at 07:05:33PM +0000, Brandt, Todd E wrote: > Updates the drivers/base/power subsystem to allow any devices which > have registred as asynchronous and who have not registered "complete" > callbacks to be non-blocking. i.e system resume can finish and return > control to the user while these devices continue resuming. > > Changelog: > v2: > - Updated patch submission. Incorporates comments from Tejun Heo. > Fixed comment format, removed camelcase. No functional changes. > > Signed-off-by: Todd Brandt <todd.e.brandt@intel.com> > --- > drivers/base/power/main.c | 20 ++++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) > > diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c > index 2b7f77d..1b16379 100644 > --- a/drivers/base/power/main.c > +++ b/drivers/base/power/main.c > @@ -713,7 +713,6 @@ void dpm_resume(pm_message_t state) > put_device(dev); > } > mutex_unlock(&dpm_list_mtx); > - async_synchronize_full(); > dpm_show_time(starttime, state, NULL); > } > > @@ -726,11 +725,14 @@ static void device_complete(struct device *dev, pm_message_t state) > { > void (*callback)(struct device *) = NULL; > char *info = NULL; > + bool hascb = false; "hascb"? Please spell out what this is. > > if (dev->power.syscore) > return; > > - device_lock(dev); > + docomplete: > + if (hascb) > + device_lock(dev); > > if (dev->pm_domain) { > info = "completing power domain "; > @@ -751,13 +753,21 @@ static void device_complete(struct device *dev, pm_message_t state) > callback = dev->driver->pm->complete; > } > > + /* > + * if a callback exists, lock the device and call it > + * otherwise don't even lock/unlock the device > + */ > if (callback) { > + if (!hascb) { > + hascb = true; > + goto docomplete; You want to jump backwards? This isn't the scheduler, please, you can do better here. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, will do, thanks for the feedback. I used the goto to make the code addition smaller, but I obviously sacrificed code clarity.
On Fri, 17 May 2013, Brandt, Todd E wrote: > Updates the drivers/base/power subsystem to allow any devices which > have registred as asynchronous and who have not registered "complete" > callbacks to be non-blocking. i.e system resume can finish and return > control to the user while these devices continue resuming. This does not sound like a good idea. The presence or absence of a "complete" callback has nothing to do with whether or not system resume can finish before the device is ready. It is more closely related to whether the device's driver can register hot-plugged children below the device or needs to perform other actions after the children have been resumed. The whole approach seems wrong -- it violates the implicit agreement that the kernel will not try to perform I/O through a device that isn't at full power. A better approach would be to modify the SCSI disk driver to carry out the spin-up operation asynchronously with respect to the resume callback. Then the disk could be reported as back to full power while the spin-up is taking place. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks for the feedback, I've taken things back to the drawing board in lieu of these issues and will be resubmitting once I have a patch that fits better into the formal power management architecture. Would you like to be CC'ed? Todd Brandt Linux Kernel Developer OTC, Hillsboro OR https://opensource.intel.com/linux-wiki/ToddBrandt
On Mon, 3 Jun 2013, Brandt, Todd E wrote: > Thanks for the feedback, I've taken things back to the drawing board > in lieu of these issues and will be resubmitting once I have a patch > that fits better into the formal power management architecture. Would > you like to be CC'ed? No need, since I am subscribed to the linux-pm mailing list. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c index 2b7f77d..1b16379 100644 --- a/drivers/base/power/main.c +++ b/drivers/base/power/main.c @@ -713,7 +713,6 @@ void dpm_resume(pm_message_t state) put_device(dev); } mutex_unlock(&dpm_list_mtx); - async_synchronize_full(); dpm_show_time(starttime, state, NULL); } @@ -726,11 +725,14 @@ static void device_complete(struct device *dev, pm_message_t state) { void (*callback)(struct device *) = NULL; char *info = NULL; + bool hascb = false; if (dev->power.syscore) return; - device_lock(dev); + docomplete: + if (hascb) + device_lock(dev); if (dev->pm_domain) { info = "completing power domain "; @@ -751,13 +753,21 @@ static void device_complete(struct device *dev, pm_message_t state) callback = dev->driver->pm->complete; } + /* + * if a callback exists, lock the device and call it + * otherwise don't even lock/unlock the device + */ if (callback) { + if (!hascb) { + hascb = true; + goto docomplete; + } + pm_dev_dbg(dev, state, info); callback(dev); + device_unlock(dev); } - device_unlock(dev); - pm_runtime_put_sync(dev); } @@ -1180,6 +1190,8 @@ int dpm_suspend(pm_message_t state) might_sleep(); mutex_lock(&dpm_list_mtx); + /* wait for any processes still resuming */ + async_synchronize_full(); pm_transition = state; async_error = 0; while (!list_empty(&dpm_prepared_list)) {
Updates the drivers/base/power subsystem to allow any devices which have registred as asynchronous and who have not registered "complete" callbacks to be non-blocking. i.e system resume can finish and return control to the user while these devices continue resuming. Changelog: v2: - Updated patch submission. Incorporates comments from Tejun Heo. Fixed comment format, removed camelcase. No functional changes. Signed-off-by: Todd Brandt <todd.e.brandt@intel.com> --- drivers/base/power/main.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html