Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Tue, 7 Jul 2020 17:59:08 +0200 | Subject | Re: [PATCH v1 1/3] driver core: Don't do deferred probe in parallel with kernel_init thread |
| |
On Wed, Jul 1, 2020 at 9:43 PM Saravana Kannan <saravanak@google.com> wrote: > > The current deferred probe implementation can mess up suspend/resume > ordering if deferred probe thread is kicked off in parallel with the > main initcall thread (kernel_init thread) [1]. > > For example: > > Say device-B is a consumer of device-A. > > Initcall thread Deferred probe thread > =============== ===================== > 1. device-A is added. > 2. device-B is added. > 3. dpm_list is now [device-A, device-B]. > 4. driver-A defers probe of device-A. > 5. device-A is moved to > end of dpm_list > 6. dpm_list is now > [device-B, device-A] > 7. driver-B is registereed and probes device-B. > 8. dpm_list stays as [device-B, device-A]. > > The reverse order of dpm_list is used for suspend. So in this case > device-A would incorrectly get suspended before device-B. > > Commit 716a7a259690 ("driver core: fw_devlink: Add support for batching > fwnode parsing") kicked off the deferred probe thread early during boot > to run in parallel with the initcall thread and caused suspend/resume > regressions. This patch removes the parallel run of the deferred probe > thread to avoid the suspend/resume regressions. > > [1] - https://lore.kernel.org/lkml/CAGETcx8W96KAw-d_siTX4qHB_-7ddk0miYRDQeHE6E0_8qx-6Q@mail.gmail.com/ > > Fixes: 716a7a259690 ("driver core: fw_devlink: Add support for batching fwnode parsing") > Signed-off-by: Saravana Kannan <saravanak@google.com> > --- > drivers/base/base.h | 1 - > drivers/base/core.c | 1 - > drivers/base/dd.c | 5 ----- > 3 files changed, 7 deletions(-) > > diff --git a/drivers/base/base.h b/drivers/base/base.h > index 95c22c0f9036..40fb069a8a7e 100644 > --- a/drivers/base/base.h > +++ b/drivers/base/base.h > @@ -153,7 +153,6 @@ extern char *make_class_name(const char *name, struct kobject *kobj); > extern int devres_release_all(struct device *dev); > extern void device_block_probing(void); > extern void device_unblock_probing(void); > -extern void driver_deferred_probe_force_trigger(void); > > /* /sys/devices directory */ > extern struct kset *devices_kset; > diff --git a/drivers/base/core.c b/drivers/base/core.c > index 67d39a90b45c..35cc9896eb9e 100644 > --- a/drivers/base/core.c > +++ b/drivers/base/core.c > @@ -1323,7 +1323,6 @@ void fw_devlink_resume(void) > goto out; > > device_link_add_missing_supplier_links(); > - driver_deferred_probe_force_trigger();
So it looks like this was not needed after all.
What was the role of it then?
> out: > mutex_unlock(&defer_fw_devlink_lock); > } > diff --git a/drivers/base/dd.c b/drivers/base/dd.c > index 9a1d940342ac..48ca81cb8ebc 100644 > --- a/drivers/base/dd.c > +++ b/drivers/base/dd.c > @@ -164,11 +164,6 @@ static void driver_deferred_probe_trigger(void) > if (!driver_deferred_probe_enable) > return; > > - driver_deferred_probe_force_trigger(); > -} > - > -void driver_deferred_probe_force_trigger(void) > -{ > /* > * A successful probe means that all the devices in the pending list > * should be triggered to be reprobed. Move all the deferred devices > -- > 2.27.0.212.ge8ba1cc988-goog >
| |