Messages in this thread | | | From | Saravana Kannan <> | Date | Tue, 7 Jul 2020 14:54:21 -0700 | Subject | Re: [PATCH v1 1/3] driver core: Don't do deferred probe in parallel with kernel_init thread |
| |
On Tue, Jul 7, 2020 at 8:59 AM Rafael J. Wysocki <rafael@kernel.org> wrote: > > 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?
It's needed to avoid delaying all probe till late_initcall. Patch 3/3 has more details.
-Saravana
| |