Hi Saravana, Thank you for working on this !
On Wed, Nov 04, 2020 at 03:23:37PM -0800, Saravana Kannan wrote: > The current implementation of fw_devlink is very inefficient because it > tries to get away without creating fwnode links in the name of saving > memory usage. Past attempts to optimize runtime at the cost of memory > usage were blocked with request for data showing that the optimization > made significant improvement for real world scenarios. > > We have those scenarios now. There have been several reports of boot > time increase in the order of seconds in this thread [1]. Several OEMs > and SoC manufacturers have also privately reported significant > (350-400ms) increase in boot time due to all the parsing done by > fw_devlink. > > So this patch series refactors fw_devlink to be more efficient. The key > difference now is the addition of support for fwnode links -- just a few > simple APIs. This also allows most of the code to be moved out of > firmware specific (DT mostly) code into driver core. > > This brings the following benefits: > - Instead of parsing the device tree multiple times (complexity was > close to O(N^3) where N in the number of properties) during bootup, > fw_devlink parses each fwnode node/property only once and creates > fwnode links. The rest of the fw_devlink code then just looks at these > fwnode links to do rest of the work. > > - Makes it much easier to debug probe issue due to fw_devlink in the > future. fw_devlink=on blocks the probing of devices if they depend on > a device that hasn't been added yet. With this refactor, it'll be very > easy to tell what that device is because we now have a reference to > the fwnode of the device. > > - Much easier to add fw_devlink support to ACPI and other firmware > types. A refactor to move the common bits from DT specific code to > driver core was in my TODO list as a prerequisite to adding ACPI > support to fw_devlink. This series gets that done. > > Tomi/Laurent/Grygorii, > > If you can test this series, that'd be great! I gave it a try, rebasing my branch from v5.9 to v5.10-rc2 first. On v5.10-rc2 the kernel dies when booting due to a deadlock (reported by lockdep, so hopefully not too hard to debug). *sigh*. Fortunately, it dies after the fw_devlink initialization, so I can still report results. Before your series: [ 0.743065] cpuidle: using governor menu [ 13.350259] No ATAGs? With your series applied: [ 0.722670] cpuidle: using governor menu [ 1.135859] No ATAGs? That's a very clear improvement :-) Tested-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com> > [1] - > https://lore.kernel.org/linux-pm/CAGETcx-aiW251dhEXT1GNb9bi6YcX8W=jlbrro5cnpuejgl...@mail.gmail.com/ > > Saravana Kannan (18): > Revert "driver core: Avoid deferred probe due to fw_devlink_pause/resume()" > Revert "driver core: Rename dev_links_info.defer_sync to defer_hook" > Revert "driver core: Don't do deferred probe in parallel with kernel_init > thread" > Revert "driver core: Remove check in driver_deferred_probe_force_trigger()" > Revert "of: platform: Batch fwnode parsing when adding all top level > devices" > Revert "driver core: fw_devlink: Add support for batching fwnode parsing" > driver core: Add fwnode_init() > driver core: Add fwnode link support > driver core: Allow only unprobed consumers for SYNC_STATE_ONLY device links > device property: Add fwnode_is_ancestor_of() > driver core: Redefine the meaning of fwnode_operations.add_links() > driver core: Add fw_devlink_parse_fwtree() > driver core: Add fwnode_get_next_parent_dev() helper function > driver core: Use device's fwnode to check if it is waiting for suppliers > of: property: Update implementation of add_links() to create fwnode links > efi: Update implementation of add_links() to create fwnode links > driver core: Add helper functions to convert fwnode links to device links > driver core: Refactor fw_devlink feature > > drivers/acpi/property.c | 2 +- > drivers/acpi/scan.c | 2 +- > drivers/base/core.c | 584 +++++++++++++++++++++----------- > drivers/base/property.c | 27 ++ > drivers/base/swnode.c | 2 +- > drivers/firmware/efi/efi-init.c | 31 +- > drivers/of/dynamic.c | 1 + > drivers/of/platform.c | 2 - > drivers/of/property.c | 150 +++----- > include/linux/device.h | 10 +- > include/linux/fwnode.h | 66 ++-- > include/linux/of.h | 2 +- > include/linux/property.h | 2 + > kernel/irq/irqdomain.c | 2 +- > 14 files changed, 490 insertions(+), 393 deletions(-) -- Regards, Laurent Pinchart