On Wed, Dec 6, 2023 at 7:09 PM Saravana Kannan <saravanak@xxxxxxxxxx> wrote: > > On Wed, Dec 6, 2023 at 9:15 AM Rob Herring <robh@xxxxxxxxxx> wrote: > > > > On Thu, Nov 30, 2023 at 06:41:07PM +0100, Herve Codina wrote: > > > Hi, > > > > +Saravana for comment > > I'll respond to this within a week -- very swamped at the moment. The > main thing I want to make sure is that we don't cause an indirect > deadlock with this wait(). I'll go back and look at why we added the > work queue and then check for device/devlink locking issues. > Sorry about the long delay, but I finally got back to this because Nuno nudged me to review a similar patch they sent. I'll leave some easy to address comments in the patches. -Saravana > -Saravana > > > > > Looks okay to me though. > > > > > > > > In the following sequence: > > > of_platform_depopulate(); /* Remove devices from a DT overlay node */ > > > of_overlay_remove(); /* Remove the DT overlay node itself */ > > > > > > Some warnings are raised by __of_changeset_entry_destroy() which was > > > called from of_overlay_remove(): > > > ERROR: memory leak, expected refcount 1 instead of 2 ... > > > > > > The issue is that, during the device devlink removals triggered from the > > > of_platform_depopulate(), jobs are put in a workqueue. > > > These jobs drop the reference to the devices. When a device is no more > > > referenced (refcount == 0), it is released and the reference to its > > > of_node is dropped by a call to of_node_put(). > > > These operations are fully correct except that, because of the > > > workqueue, they are done asynchronously with respect to function calls. > > > > > > In the sequence provided, the jobs are run too late, after the call to > > > __of_changeset_entry_destroy() and so a missing of_node_put() call is > > > detected by __of_changeset_entry_destroy(). > > > > > > This series fixes this issue introducing device_link_wait_removal() in > > > order to wait for the end of jobs execution (patch 1) and using this > > > function to synchronize the overlay removal with the end of jobs > > > execution (patch 2). > > > > > > Best regards, > > > Hervé > > > > > > Herve Codina (2): > > > driver core: Introduce device_link_wait_removal() > > > of: overlay: Synchronize of_overlay_remove() with the devlink removals > > > > > > drivers/base/core.c | 26 +++++++++++++++++++++++--- > > > drivers/of/overlay.c | 6 ++++++ > > > include/linux/device.h | 1 + > > > 3 files changed, 30 insertions(+), 3 deletions(-) > > > > > > -- > > > 2.42.0 > > >
