[RFC PATCH 0/3] Fix errors on DT overlay removal with devlinks

[RFC PATCH 0/3] Fix errors on DT overlay removal with devlinks

[Michael Auchter]

After updating to v5.9, I've started seeing errors in the kernel log
when using device tree overlays. Specifically, the problem seems to
happen when removing a device tree overlay that contains two devices
with some dependency between them (e.g., a device that provides a clock
and a device that consumes that clock). Removing such an overlay results
in:

  OF: ERROR: memory leak, expected refcount 1 instead of 2, of_node_get()/of_node_put() unbalanced - destroy
  OF: ERROR: memory leak, expected refcount 1 instead of 2, of_node_get()/of_node_put() unbalanced - destroy

followed by hitting some REFCOUNT_WARNs in refcount.c

In the first patch, I've included a unittest that can be used to
reproduce this when built with CONFIG_OF_UNITTEST [1].

I believe the issue is caused by the cleanup performed when releasing
the devlink device that's created to represent the dependency between
devices. The devlink device has references to the consumer and supplier
devices, which it drops in device_link_free; the devlink device's
release callback calls device_link_free via call_srcu.

When the overlay is being removed, all devices are removed, and
eventually the release callback for the devlink device run, and
schedules cleanup using call_srcu. Before device_link_free can and call
put_device on the consumer/supplier, the rest of the overlay removal
process runs, resulting in the error traces above.

Patches 2 and 3 are an attempt at fixing this: call srcu_barrier to wait
for any pending device_link_free's to execute before continuing on with
the removal process.

These patches resolve the issue, but probably not in the best way. In
particular, it seems strange to need to leak details of devlinks into
the device tree overlay code. So, I'd be curious to get some feedback or
hear any other ideas for how to resolve this issue.

Thanks,
 Michael

1. Note that this isn't a very good unit test: it will report a "pass"
   even if it fails with the aforementioned errors, as these errors
   aren't propogated.

Michael Auchter (3):
  of: unittest: add test of overlay with devlinks
  driver core: add device_links_barrier
  of: dynamic: add device links barrier before detach

 drivers/base/core.c                     | 10 ++++++++++
 drivers/of/dynamic.c                    |  3 +++
 drivers/of/unittest-data/Makefile       |  1 +
 drivers/of/unittest-data/overlay_16.dts | 26 +++++++++++++++++++++++++
 drivers/of/unittest.c                   | 16 +++++++++++++++
 include/linux/device.h                  |  1 +
 6 files changed, 57 insertions(+)
 create mode 100644 drivers/of/unittest-data/overlay_16.dts

-- 
2.25.4

[RFC PATCH 1/3] of: unittest: add test of overlay with devlinks

[Michael Auchter]

This adds a unittest to test applying/reverting an overlay that contains
a link between two devices.

Signed-off-by: Michael Auchter <michael.auchter@ni.com>
---
 drivers/of/unittest-data/Makefile       |  1 +
 drivers/of/unittest-data/overlay_16.dts | 26 +++++++++++++++++++++++++
 drivers/of/unittest.c                   | 16 +++++++++++++++
 3 files changed, 43 insertions(+)
 create mode 100644 drivers/of/unittest-data/overlay_16.dts

diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
index 009f4045c8e4..91a1ebbc451b 100644
--- a/drivers/of/unittest-data/Makefile
+++ b/drivers/of/unittest-data/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_OF_OVERLAY) += overlay.dtb.o \
 			    overlay_12.dtb.o \
 			    overlay_13.dtb.o \
 			    overlay_15.dtb.o \
+			    overlay_16.dtb.o \
 			    overlay_bad_add_dup_node.dtb.o \
 			    overlay_bad_add_dup_prop.dtb.o \
 			    overlay_bad_phandle.dtb.o \
diff --git a/drivers/of/unittest-data/overlay_16.dts b/drivers/of/unittest-data/overlay_16.dts
new file mode 100644
index 000000000000..eda206e2ebca
--- /dev/null
+++ b/drivers/of/unittest-data/overlay_16.dts
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+/* overlay_16 - device links */
+
+&unittest_test_bus {
+
+	/* suppress DTC warning */
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	unittest16: test-unittest16 {
+		#clock-cells = <0>;
+		compatible = "unittest";
+		status = "okay";
+		reg = <16>;
+	};
+
+	test-unittest161 {
+		compatible = "unittest";
+		status = "okay";
+		reg = <161>;
+		clocks = <&unittest16>;
+	};
+};
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 5579584758b7..d94dafb3746f 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -2369,6 +2369,18 @@ static void __init of_unittest_overlay_11(void)
 	unittest(ret == 0, "overlay test %d failed; overlay apply\n", 11);
 }
 
+/* test insertion of an overlay that references another node */
+static void __init of_unittest_overlay_16(void)
+{
+	int ret;
+
+	/* device should disable */
+	ret = of_unittest_apply_revert_overlay_check(16, 16, 0, 1,
+			PDEV_OVERLAY);
+
+	unittest(ret == 0, "overlay test %d failed; overlay apply\n", 16);
+}
+
 #if IS_BUILTIN(CONFIG_I2C) && IS_ENABLED(CONFIG_OF_OVERLAY)
 
 struct unittest_i2c_bus_data {
@@ -2757,6 +2769,8 @@ static void __init of_unittest_overlay(void)
 	of_unittest_overlay_i2c_cleanup();
 #endif
 
+	of_unittest_overlay_16();
+
 	of_unittest_overlay_gpio();
 
 	of_unittest_destroy_tracked_overlays();
@@ -2812,6 +2826,7 @@ OVERLAY_INFO_EXTERN(overlay_11);
 OVERLAY_INFO_EXTERN(overlay_12);
 OVERLAY_INFO_EXTERN(overlay_13);
 OVERLAY_INFO_EXTERN(overlay_15);
+OVERLAY_INFO_EXTERN(overlay_16);
 OVERLAY_INFO_EXTERN(overlay_gpio_01);
 OVERLAY_INFO_EXTERN(overlay_gpio_02a);
 OVERLAY_INFO_EXTERN(overlay_gpio_02b);
@@ -2842,6 +2857,7 @@ static struct overlay_info overlays[] = {
 	OVERLAY_INFO(overlay_12, 0),
 	OVERLAY_INFO(overlay_13, 0),
 	OVERLAY_INFO(overlay_15, 0),
+	OVERLAY_INFO(overlay_16, 0),
 	OVERLAY_INFO(overlay_gpio_01, 0),
 	OVERLAY_INFO(overlay_gpio_02a, 0),
 	OVERLAY_INFO(overlay_gpio_02b, 0),
-- 
2.25.4

[RFC PATCH 2/3] driver core: add device_links_barrier

[Michael Auchter]

Add a barrier to wait for all device_links SRCU sections to complete.

Signed-off-by: Michael Auchter <michael.auchter@ni.com>
---
 drivers/base/core.c    | 10 ++++++++++
 include/linux/device.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index bb5806a2bd4c..ef4429c4b1f2 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -82,6 +82,11 @@ int device_links_read_lock_held(void)
 {
 	return srcu_read_lock_held(&device_links_srcu);
 }
+
+void device_links_barrier(void)
+{
+	srcu_barrier(&device_links_srcu);
+}
 #else /* !CONFIG_SRCU */
 static DECLARE_RWSEM(device_links_lock);
 
@@ -112,6 +117,11 @@ int device_links_read_lock_held(void)
 	return lockdep_is_held(&device_links_lock);
 }
 #endif
+
+void device_links_barrier(void)
+{
+}
 #endif /* !CONFIG_SRCU */
 
 /**
diff --git a/include/linux/device.h b/include/linux/device.h
index 9e6ea8931a52..8c47e0beb308 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -977,6 +977,7 @@ void device_link_del(struct device_link *link);
 void device_link_remove(void *consumer, struct device *supplier);
 void device_links_supplier_sync_state_pause(void);
 void device_links_supplier_sync_state_resume(void);
+void device_links_barrier(void);
 
 extern __printf(3, 4)
 int dev_err_probe(const struct device *dev, int err, const char *fmt, ...);
-- 
2.25.4

[RFC PATCH 3/3] of: dynamic: add device links barrier before detach

[Michael Auchter]

Signed-off-by: Michael Auchter <michael.auchter@ni.com>
---
 drivers/of/dynamic.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 7478bfc8d440..a4e2881524e9 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -14,6 +14,7 @@
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/proc_fs.h>
+#include <linux/device.h>
 
 #include "of_private.h"
 
@@ -688,6 +689,8 @@ void of_changeset_destroy(struct of_changeset *ocs)
 {
 	struct of_changeset_entry *ce, *cen;
 
+	device_links_barrier();
+
 	list_for_each_entry_safe_reverse(ce, cen, &ocs->entries, node)
 		__of_changeset_entry_destroy(ce);
 }
-- 
2.25.4

Re: [RFC PATCH 0/3] Fix errors on DT overlay removal with devlinks

[Frank Rowand]

Hi Michael,

On 10/14/20 2:36 PM, Michael Auchter wrote:
> After updating to v5.9, I've started seeing errors in the kernel log
> when using device tree overlays. Specifically, the problem seems to
> happen when removing a device tree overlay that contains two devices
> with some dependency between them (e.g., a device that provides a clock
> and a device that consumes that clock). Removing such an overlay results
> in:
> 
>   OF: ERROR: memory leak, expected refcount 1 instead of 2, of_node_get()/of_node_put() unbalanced - destroy
>   OF: ERROR: memory leak, expected refcount 1 instead of 2, of_node_get()/of_node_put() unbalanced - destroy
> 
> followed by hitting some REFCOUNT_WARNs in refcount.c
> 
> In the first patch, I've included a unittest that can be used to
> reproduce this when built with CONFIG_OF_UNITTEST [1].
> 
> I believe the issue is caused by the cleanup performed when releasing
> the devlink device that's created to represent the dependency between
> devices. The devlink device has references to the consumer and supplier
> devices, which it drops in device_link_free; the devlink device's
> release callback calls device_link_free via call_srcu.
> 
> When the overlay is being removed, all devices are removed, and
> eventually the release callback for the devlink device run, and
> schedules cleanup using call_srcu. Before device_link_free can and call
> put_device on the consumer/supplier, the rest of the overlay removal
> process runs, resulting in the error traces above.
> 
> Patches 2 and 3 are an attempt at fixing this: call srcu_barrier to wait
> for any pending device_link_free's to execute before continuing on with
> the removal process.
> 
> These patches resolve the issue, but probably not in the best way. In
> particular, it seems strange to need to leak details of devlinks into
> the device tree overlay code. So, I'd be curious to get some feedback or
> hear any other ideas for how to resolve this issue.

Thanks for finding the problem, analyzing it, creating a unittest, and
creating a fix.

I agree with your analysis that there are issues with the implementation
of the test and fix.  I'll dig into this to see if I can provide some
useful improvements.

-Frank

> 
> Thanks,
>  Michael
> 
> 1. Note that this isn't a very good unit test: it will report a "pass"
>    even if it fails with the aforementioned errors, as these errors
>    aren't propogated.
> 
> Michael Auchter (3):
>   of: unittest: add test of overlay with devlinks
>   driver core: add device_links_barrier
>   of: dynamic: add device links barrier before detach
> 
>  drivers/base/core.c                     | 10 ++++++++++
>  drivers/of/dynamic.c                    |  3 +++
>  drivers/of/unittest-data/Makefile       |  1 +
>  drivers/of/unittest-data/overlay_16.dts | 26 +++++++++++++++++++++++++
>  drivers/of/unittest.c                   | 16 +++++++++++++++
>  include/linux/device.h                  |  1 +
>  6 files changed, 57 insertions(+)
>  create mode 100644 drivers/of/unittest-data/overlay_16.dts
> 

Re: [RFC PATCH 0/3] Fix errors on DT overlay removal with devlinks

[Frank Rowand]

Hi Saravana,

Michael found an issue related to the removal of a devicetree node
which involves devlinks:

On 10/14/20 2:36 PM, Michael Auchter wrote:
> After updating to v5.9, I've started seeing errors in the kernel log
> when using device tree overlays. Specifically, the problem seems to
> happen when removing a device tree overlay that contains two devices
> with some dependency between them (e.g., a device that provides a clock
> and a device that consumes that clock). Removing such an overlay results
> in:
> 
>   OF: ERROR: memory leak, expected refcount 1 instead of 2, of_node_get()/of_node_put() unbalanced - destroy
>   OF: ERROR: memory leak, expected refcount 1 instead of 2, of_node_get()/of_node_put() unbalanced - destroy
> 
> followed by hitting some REFCOUNT_WARNs in refcount.c
> 
> In the first patch, I've included a unittest that can be used to
> reproduce this when built with CONFIG_OF_UNITTEST [1].
> 
> I believe the issue is caused by the cleanup performed when releasing
> the devlink device that's created to represent the dependency between
> devices. The devlink device has references to the consumer and supplier
> devices, which it drops in device_link_free; the devlink device's
> release callback calls device_link_free via call_srcu.
> 
> When the overlay is being removed, all devices are removed, and
> eventually the release callback for the devlink device run, and
> schedules cleanup using call_srcu. Before device_link_free can and call
> put_device on the consumer/supplier, the rest of the overlay removal
> process runs, resulting in the error traces above.

When a devicetree node in an overlay is removed, the remove code expects
all previous users of the related device to have done the appropriate put
of the device and to have no later references.

As Michael described above, the devlink release callback defers the
put_device().  The cleanup via srcu was implemented in commit
843e600b8a2b01463c4d873a90b2c2ea8033f1f6 "driver core: Fix sleeping
in invalid context during device link deletion" to solve yet another
issue.


> 
> Patches 2 and 3 are an attempt at fixing this: call srcu_barrier to wait
> for any pending device_link_free's to execute before continuing on with
> the removal process.
> 
> These patches resolve the issue, but probably not in the best way. In
> particular, it seems strange to need to leak details of devlinks into
> the device tree overlay code. So, I'd be curious to get some feedback or
> hear any other ideas for how to resolve this issue.

I agree with Michael that adding an indirect call of srcu_barrier(&device_links_srcu)
into the devicetree overlay code is not an appropriate solution.

Is there some other way to fix the problem that 843e600b8a2b solves without
deferring the put_device() done by the devlink release callback?

-Frank

> 
> Thanks,
>  Michael
> 
> 1. Note that this isn't a very good unit test: it will report a "pass"
>    even if it fails with the aforementioned errors, as these errors
>    aren't propogated.
> 
> Michael Auchter (3):
>   of: unittest: add test of overlay with devlinks
>   driver core: add device_links_barrier
>   of: dynamic: add device links barrier before detach
> 
>  drivers/base/core.c                     | 10 ++++++++++
>  drivers/of/dynamic.c                    |  3 +++
>  drivers/of/unittest-data/Makefile       |  1 +
>  drivers/of/unittest-data/overlay_16.dts | 26 +++++++++++++++++++++++++
>  drivers/of/unittest.c                   | 16 +++++++++++++++
>  include/linux/device.h                  |  1 +
>  6 files changed, 57 insertions(+)
>  create mode 100644 drivers/of/unittest-data/overlay_16.dts
> 

Re: [RFC PATCH 0/3] Fix errors on DT overlay removal with devlinks

[Saravana Kannan]

On Wed, Oct 21, 2020 at 2:02 PM Frank Rowand <frowand.list@gmail.com> wrote:
>
> Hi Saravana,
>
> Michael found an issue related to the removal of a devicetree node
> which involves devlinks:
>
> On 10/14/20 2:36 PM, Michael Auchter wrote:
> > After updating to v5.9, I've started seeing errors in the kernel log
> > when using device tree overlays. Specifically, the problem seems to
> > happen when removing a device tree overlay that contains two devices
> > with some dependency between them (e.g., a device that provides a clock
> > and a device that consumes that clock). Removing such an overlay results
> > in:
> >
> >   OF: ERROR: memory leak, expected refcount 1 instead of 2, of_node_get()/of_node_put() unbalanced - destroy
> >   OF: ERROR: memory leak, expected refcount 1 instead of 2, of_node_get()/of_node_put() unbalanced - destroy
> >
> > followed by hitting some REFCOUNT_WARNs in refcount.c
> >
> > In the first patch, I've included a unittest that can be used to
> > reproduce this when built with CONFIG_OF_UNITTEST [1].
> >
> > I believe the issue is caused by the cleanup performed when releasing
> > the devlink device that's created to represent the dependency between
> > devices. The devlink device has references to the consumer and supplier
> > devices, which it drops in device_link_free; the devlink device's
> > release callback calls device_link_free via call_srcu.
> >
> > When the overlay is being removed, all devices are removed, and
> > eventually the release callback for the devlink device run, and
> > schedules cleanup using call_srcu. Before device_link_free can and call
> > put_device on the consumer/supplier, the rest of the overlay removal
> > process runs, resulting in the error traces above.
>
> When a devicetree node in an overlay is removed, the remove code expects
> all previous users of the related device to have done the appropriate put
> of the device and to have no later references.
>
> As Michael described above, the devlink release callback defers the
> put_device().  The cleanup via srcu was implemented in commit
> 843e600b8a2b01463c4d873a90b2c2ea8033f1f6 "driver core: Fix sleeping
> in invalid context during device link deletion" to solve yet another
> issue.
>
>
> >
> > Patches 2 and 3 are an attempt at fixing this: call srcu_barrier to wait
> > for any pending device_link_free's to execute before continuing on with
> > the removal process.
> >
> > These patches resolve the issue, but probably not in the best way. In
> > particular, it seems strange to need to leak details of devlinks into
> > the device tree overlay code. So, I'd be curious to get some feedback or
> > hear any other ideas for how to resolve this issue.
>
> I agree with Michael that adding an indirect call of srcu_barrier(&device_links_srcu)
> into the devicetree overlay code is not an appropriate solution.

I kind of see your point too. I wonder if the srcu_barrier() should
happen inside like so:
device_del() -> device_links_purge()->srcu_barrier()

I don't know what contention the use of SRCUs in device links was
trying to avoid, but I think the srcu_barrier() call path I suggested
above shouldn't be a problem. If that fixes the issue, the best way to
know if it's an issue is to send out a patch and see if Rafael has any
problem with it :)

> Is there some other way to fix the problem that 843e600b8a2b solves without
> deferring the put_device() done by the devlink release callback?

Ok I finally got some time to look into this closely.

Even if you revert 843e600b8a2b, you'll see that device_link_free()
(which drops the reference to the consumer and supplier devices) was
scheduled to run when the SRCU clean up occurs. So I think this issue
was present even before 843e600b8a2b, but commit 843e600b8a2b just
made it more likely to hit this scenario because it introduces some
delay in dropping the ref count of the supplier and consumer by going
through the device link device's release path. So, I think this issue
isn't related to 843e600b8a2b.

As to why 843e600b8a2b had to be written to call call_srcu() from the
device link device's release path, it's a mess of dependencies/delays:
1. The device link device is part of the struct device_link. So we
can't free device_link before the device_link.link_dev refcount goes
to 0.
2. But I can't assume device_link.link_dev's refcount will go to 0 as
soon as I call put_device() on it because of
CONFIG_DEBUG_KOBJECT_RELEASE which frees up the kobject after a random
delay.
3. The use of SRCU also means I can't free device_link until the SRCU
is cleaned up.

Because of (1), (2) and (3), when the device_link_del() (or any of the
other device link deletion APIs are called) I first have to do a
put_device(device_link.link_dev) to make sure the device memory is no
longer referenced, then trigger an SRCU clean up and then in the
scheduled SRCU cleanup I can free struct device_link. And obviously,
until struct device_link is ready to be freed up, I can't drop the
reference to the supplier and consumer devices (as that old copy of
device_link could be used by some code to refer to the supplier and
consumer devices).

Hope that helps explain the SRCU and device link device release dependencies.

Also, even if this patch series is applied as is, I wonder if the
current overlay code has a bug related to CONFIG_DEBUG_KOBJECT_RELEASE
delaying the actual freeing of the device. Something to look into?

-Saravana

Re: Re: [RFC PATCH 0/3] Fix errors on DT overlay removal with devlinks

[Michael Auchter]

Hey Saravana,

Thanks for taking the time to look into this!

On Mon, Oct 26, 2020 at 12:10:33PM -0700, Saravana Kannan wrote:
> On Wed, Oct 21, 2020 at 2:02 PM Frank Rowand <frowand.list@gmail.com> wrote:
> >
> > Hi Saravana,
> >
> > Michael found an issue related to the removal of a devicetree node
> > which involves devlinks:
> >
> > On 10/14/20 2:36 PM, Michael Auchter wrote:
> > > After updating to v5.9, I've started seeing errors in the kernel log
> > > when using device tree overlays. Specifically, the problem seems to
> > > happen when removing a device tree overlay that contains two devices
> > > with some dependency between them (e.g., a device that provides a clock
> > > and a device that consumes that clock). Removing such an overlay results
> > > in:
> > >
> > >   OF: ERROR: memory leak, expected refcount 1 instead of 2, of_node_get()/of_node_put() unbalanced - destroy
> > >   OF: ERROR: memory leak, expected refcount 1 instead of 2, of_node_get()/of_node_put() unbalanced - destroy
> > >
> > > followed by hitting some REFCOUNT_WARNs in refcount.c
> > >
> > > In the first patch, I've included a unittest that can be used to
> > > reproduce this when built with CONFIG_OF_UNITTEST [1].
> > >
> > > I believe the issue is caused by the cleanup performed when releasing
> > > the devlink device that's created to represent the dependency between
> > > devices. The devlink device has references to the consumer and supplier
> > > devices, which it drops in device_link_free; the devlink device's
> > > release callback calls device_link_free via call_srcu.
> > >
> > > When the overlay is being removed, all devices are removed, and
> > > eventually the release callback for the devlink device run, and
> > > schedules cleanup using call_srcu. Before device_link_free can and call
> > > put_device on the consumer/supplier, the rest of the overlay removal
> > > process runs, resulting in the error traces above.
> >
> > When a devicetree node in an overlay is removed, the remove code expects
> > all previous users of the related device to have done the appropriate put
> > of the device and to have no later references.
> >
> > As Michael described above, the devlink release callback defers the
> > put_device().  The cleanup via srcu was implemented in commit
> > 843e600b8a2b01463c4d873a90b2c2ea8033f1f6 "driver core: Fix sleeping
> > in invalid context during device link deletion" to solve yet another
> > issue.
> >
> >
> > >
> > > Patches 2 and 3 are an attempt at fixing this: call srcu_barrier to wait
> > > for any pending device_link_free's to execute before continuing on with
> > > the removal process.
> > >
> > > These patches resolve the issue, but probably not in the best way. In
> > > particular, it seems strange to need to leak details of devlinks into
> > > the device tree overlay code. So, I'd be curious to get some feedback or
> > > hear any other ideas for how to resolve this issue.
> >
> > I agree with Michael that adding an indirect call of srcu_barrier(&device_links_srcu)
> > into the devicetree overlay code is not an appropriate solution.
> 
> I kind of see your point too. I wonder if the srcu_barrier() should
> happen inside like so:
> device_del() -> device_links_purge()->srcu_barrier()
> 
> I don't know what contention the use of SRCUs in device links was
> trying to avoid, but I think the srcu_barrier() call path I suggested
> above shouldn't be a problem. If that fixes the issue, the best way to
> know if it's an issue is to send out a patch and see if Rafael has any
> problem with it :)

I was able to test this by adding the srcu_barrier() at the end of
device_links_purge(), and that does seem to have fixed the issue.

> > Is there some other way to fix the problem that 843e600b8a2b solves without
> > deferring the put_device() done by the devlink release callback?
> 
> Ok I finally got some time to look into this closely.
> 
> Even if you revert 843e600b8a2b, you'll see that device_link_free()
> (which drops the reference to the consumer and supplier devices) was
> scheduled to run when the SRCU clean up occurs. So I think this issue
> was present even before 843e600b8a2b, but commit 843e600b8a2b just
> made it more likely to hit this scenario because it introduces some
> delay in dropping the ref count of the supplier and consumer by going
> through the device link device's release path. So, I think this issue
> isn't related to 843e600b8a2b.
> 
> As to why 843e600b8a2b had to be written to call call_srcu() from the
> device link device's release path, it's a mess of dependencies/delays:
> 1. The device link device is part of the struct device_link. So we
> can't free device_link before the device_link.link_dev refcount goes
> to 0.
> 2. But I can't assume device_link.link_dev's refcount will go to 0 as
> soon as I call put_device() on it because of
> CONFIG_DEBUG_KOBJECT_RELEASE which frees up the kobject after a random
> delay.
> 3. The use of SRCU also means I can't free device_link until the SRCU
> is cleaned up.
> 
> Because of (1), (2) and (3), when the device_link_del() (or any of the
> other device link deletion APIs are called) I first have to do a
> put_device(device_link.link_dev) to make sure the device memory is no
> longer referenced, then trigger an SRCU clean up and then in the
> scheduled SRCU cleanup I can free struct device_link. And obviously,
> until struct device_link is ready to be freed up, I can't drop the
> reference to the supplier and consumer devices (as that old copy of
> device_link could be used by some code to refer to the supplier and
> consumer devices).
> 
> Hope that helps explain the SRCU and device link device release dependencies.
> 
> Also, even if this patch series is applied as is, I wonder if the
> current overlay code has a bug related to CONFIG_DEBUG_KOBJECT_RELEASE
> delaying the actual freeing of the device. Something to look into?

I also tried enabling CONFIG_DEBUG_KOBJECT_RELEASE... with or without
the addition of srcu_barrier() to device_links_purge(), I can't boot
successfully when CONFIG_OF_UNITTEST=y &&
CONFIG_DEBUG_KOBJECT_RELEASE=y: there are a ton of errors that result
from this combo.

Disabling the unittests and booting with CONFIG_DEBUG_KOBJECT_RELEASE=y,
I _do_ still see the errors mentioned in my original message when
removing an overlay. So yeah, it does seem like there are some latent
issues here...

Cheers,
 Michael

Re: Re: [RFC PATCH 0/3] Fix errors on DT overlay removal with devlinks

[Saravana Kannan]

On Wed, Oct 28, 2020 at 9:25 AM Michael Auchter <michael.auchter@ni.com> wrote:
>
> Hey Saravana,
>
> Thanks for taking the time to look into this!
>
> On Mon, Oct 26, 2020 at 12:10:33PM -0700, Saravana Kannan wrote:
> > On Wed, Oct 21, 2020 at 2:02 PM Frank Rowand <frowand.list@gmail.com> wrote:
> > >
> > > Hi Saravana,
> > >
> > > Michael found an issue related to the removal of a devicetree node
> > > which involves devlinks:
> > >
> > > On 10/14/20 2:36 PM, Michael Auchter wrote:
> > > > After updating to v5.9, I've started seeing errors in the kernel log
> > > > when using device tree overlays. Specifically, the problem seems to
> > > > happen when removing a device tree overlay that contains two devices
> > > > with some dependency between them (e.g., a device that provides a clock
> > > > and a device that consumes that clock). Removing such an overlay results
> > > > in:
> > > >
> > > >   OF: ERROR: memory leak, expected refcount 1 instead of 2, of_node_get()/of_node_put() unbalanced - destroy
> > > >   OF: ERROR: memory leak, expected refcount 1 instead of 2, of_node_get()/of_node_put() unbalanced - destroy
> > > >
> > > > followed by hitting some REFCOUNT_WARNs in refcount.c
> > > >
> > > > In the first patch, I've included a unittest that can be used to
> > > > reproduce this when built with CONFIG_OF_UNITTEST [1].
> > > >
> > > > I believe the issue is caused by the cleanup performed when releasing
> > > > the devlink device that's created to represent the dependency between
> > > > devices. The devlink device has references to the consumer and supplier
> > > > devices, which it drops in device_link_free; the devlink device's
> > > > release callback calls device_link_free via call_srcu.
> > > >
> > > > When the overlay is being removed, all devices are removed, and
> > > > eventually the release callback for the devlink device run, and
> > > > schedules cleanup using call_srcu. Before device_link_free can and call
> > > > put_device on the consumer/supplier, the rest of the overlay removal
> > > > process runs, resulting in the error traces above.
> > >
> > > When a devicetree node in an overlay is removed, the remove code expects
> > > all previous users of the related device to have done the appropriate put
> > > of the device and to have no later references.
> > >
> > > As Michael described above, the devlink release callback defers the
> > > put_device().  The cleanup via srcu was implemented in commit
> > > 843e600b8a2b01463c4d873a90b2c2ea8033f1f6 "driver core: Fix sleeping
> > > in invalid context during device link deletion" to solve yet another
> > > issue.
> > >
> > >
> > > >
> > > > Patches 2 and 3 are an attempt at fixing this: call srcu_barrier to wait
> > > > for any pending device_link_free's to execute before continuing on with
> > > > the removal process.
> > > >
> > > > These patches resolve the issue, but probably not in the best way. In
> > > > particular, it seems strange to need to leak details of devlinks into
> > > > the device tree overlay code. So, I'd be curious to get some feedback or
> > > > hear any other ideas for how to resolve this issue.
> > >
> > > I agree with Michael that adding an indirect call of srcu_barrier(&device_links_srcu)
> > > into the devicetree overlay code is not an appropriate solution.
> >
> > I kind of see your point too. I wonder if the srcu_barrier() should
> > happen inside like so:
> > device_del() -> device_links_purge()->srcu_barrier()
> >
> > I don't know what contention the use of SRCUs in device links was
> > trying to avoid, but I think the srcu_barrier() call path I suggested
> > above shouldn't be a problem. If that fixes the issue, the best way to
> > know if it's an issue is to send out a patch and see if Rafael has any
> > problem with it :)
>
> I was able to test this by adding the srcu_barrier() at the end of
> device_links_purge(), and that does seem to have fixed the issue.

Thanks for testing my suggestion. If you send out a patch for that,
I'd appreciated a Suggested-by: tag.

> > > Is there some other way to fix the problem that 843e600b8a2b solves without
> > > deferring the put_device() done by the devlink release callback?
> >
> > Ok I finally got some time to look into this closely.
> >
> > Even if you revert 843e600b8a2b, you'll see that device_link_free()
> > (which drops the reference to the consumer and supplier devices) was
> > scheduled to run when the SRCU clean up occurs. So I think this issue
> > was present even before 843e600b8a2b, but commit 843e600b8a2b just
> > made it more likely to hit this scenario because it introduces some
> > delay in dropping the ref count of the supplier and consumer by going
> > through the device link device's release path. So, I think this issue
> > isn't related to 843e600b8a2b.
> >
> > As to why 843e600b8a2b had to be written to call call_srcu() from the
> > device link device's release path, it's a mess of dependencies/delays:
> > 1. The device link device is part of the struct device_link. So we
> > can't free device_link before the device_link.link_dev refcount goes
> > to 0.
> > 2. But I can't assume device_link.link_dev's refcount will go to 0 as
> > soon as I call put_device() on it because of
> > CONFIG_DEBUG_KOBJECT_RELEASE which frees up the kobject after a random
> > delay.
> > 3. The use of SRCU also means I can't free device_link until the SRCU
> > is cleaned up.
> >
> > Because of (1), (2) and (3), when the device_link_del() (or any of the
> > other device link deletion APIs are called) I first have to do a
> > put_device(device_link.link_dev) to make sure the device memory is no
> > longer referenced, then trigger an SRCU clean up and then in the
> > scheduled SRCU cleanup I can free struct device_link. And obviously,
> > until struct device_link is ready to be freed up, I can't drop the
> > reference to the supplier and consumer devices (as that old copy of
> > device_link could be used by some code to refer to the supplier and
> > consumer devices).
> >
> > Hope that helps explain the SRCU and device link device release dependencies.
> >
> > Also, even if this patch series is applied as is, I wonder if the
> > current overlay code has a bug related to CONFIG_DEBUG_KOBJECT_RELEASE
> > delaying the actual freeing of the device. Something to look into?
>
> I also tried enabling CONFIG_DEBUG_KOBJECT_RELEASE... with or without
> the addition of srcu_barrier() to device_links_purge(), I can't boot
> successfully when CONFIG_OF_UNITTEST=y &&
> CONFIG_DEBUG_KOBJECT_RELEASE=y: there are a ton of errors that result
> from this combo.
>
> Disabling the unittests and booting with CONFIG_DEBUG_KOBJECT_RELEASE=y,
> I _do_ still see the errors mentioned in my original message when
> removing an overlay. So yeah, it does seem like there are some latent
> issues here...

Thanks for confirming my suspicion. I assume you see these errors even
with the srcu_barrier() call?

I'll leave this to Frank then :)

-Saravana

Re: [RFC PATCH 0/3] Fix errors on DT overlay removal with devlinks

[Frank Rowand]

On 10/28/20 11:25 AM, Michael Auchter wrote:
> Hey Saravana,
> 
> Thanks for taking the time to look into this!
> 
> On Mon, Oct 26, 2020 at 12:10:33PM -0700, Saravana Kannan wrote:
>> On Wed, Oct 21, 2020 at 2:02 PM Frank Rowand <frowand.list@gmail.com> wrote:
>>>
>>> Hi Saravana,
>>>
>>> Michael found an issue related to the removal of a devicetree node
>>> which involves devlinks:
>>>
>>> On 10/14/20 2:36 PM, Michael Auchter wrote:
>>>> After updating to v5.9, I've started seeing errors in the kernel log
>>>> when using device tree overlays. Specifically, the problem seems to
>>>> happen when removing a device tree overlay that contains two devices
>>>> with some dependency between them (e.g., a device that provides a clock
>>>> and a device that consumes that clock). Removing such an overlay results
>>>> in:
>>>>
>>>>   OF: ERROR: memory leak, expected refcount 1 instead of 2, of_node_get()/of_node_put() unbalanced - destroy
>>>>   OF: ERROR: memory leak, expected refcount 1 instead of 2, of_node_get()/of_node_put() unbalanced - destroy
>>>>
>>>> followed by hitting some REFCOUNT_WARNs in refcount.c
>>>>
>>>> In the first patch, I've included a unittest that can be used to
>>>> reproduce this when built with CONFIG_OF_UNITTEST [1].
>>>>
>>>> I believe the issue is caused by the cleanup performed when releasing
>>>> the devlink device that's created to represent the dependency between
>>>> devices. The devlink device has references to the consumer and supplier
>>>> devices, which it drops in device_link_free; the devlink device's
>>>> release callback calls device_link_free via call_srcu.
>>>>
>>>> When the overlay is being removed, all devices are removed, and
>>>> eventually the release callback for the devlink device run, and
>>>> schedules cleanup using call_srcu. Before device_link_free can and call
>>>> put_device on the consumer/supplier, the rest of the overlay removal
>>>> process runs, resulting in the error traces above.
>>>
>>> When a devicetree node in an overlay is removed, the remove code expects
>>> all previous users of the related device to have done the appropriate put
>>> of the device and to have no later references.
>>>
>>> As Michael described above, the devlink release callback defers the
>>> put_device().  The cleanup via srcu was implemented in commit
>>> 843e600b8a2b01463c4d873a90b2c2ea8033f1f6 "driver core: Fix sleeping
>>> in invalid context during device link deletion" to solve yet another
>>> issue.
>>>
>>>
>>>>
>>>> Patches 2 and 3 are an attempt at fixing this: call srcu_barrier to wait
>>>> for any pending device_link_free's to execute before continuing on with
>>>> the removal process.
>>>>
>>>> These patches resolve the issue, but probably not in the best way. In
>>>> particular, it seems strange to need to leak details of devlinks into
>>>> the device tree overlay code. So, I'd be curious to get some feedback or
>>>> hear any other ideas for how to resolve this issue.
>>>
>>> I agree with Michael that adding an indirect call of srcu_barrier(&device_links_srcu)
>>> into the devicetree overlay code is not an appropriate solution.
>>
>> I kind of see your point too. I wonder if the srcu_barrier() should
>> happen inside like so:
>> device_del() -> device_links_purge()->srcu_barrier()
>>
>> I don't know what contention the use of SRCUs in device links was
>> trying to avoid, but I think the srcu_barrier() call path I suggested
>> above shouldn't be a problem. If that fixes the issue, the best way to
>> know if it's an issue is to send out a patch and see if Rafael has any
>> problem with it :)
> 
> I was able to test this by adding the srcu_barrier() at the end of
> device_links_purge(), and that does seem to have fixed the issue.
> 
>>> Is there some other way to fix the problem that 843e600b8a2b solves without
>>> deferring the put_device() done by the devlink release callback?
>>
>> Ok I finally got some time to look into this closely.
>>
>> Even if you revert 843e600b8a2b, you'll see that device_link_free()
>> (which drops the reference to the consumer and supplier devices) was
>> scheduled to run when the SRCU clean up occurs. So I think this issue
>> was present even before 843e600b8a2b, but commit 843e600b8a2b just
>> made it more likely to hit this scenario because it introduces some
>> delay in dropping the ref count of the supplier and consumer by going
>> through the device link device's release path. So, I think this issue
>> isn't related to 843e600b8a2b.
>>
>> As to why 843e600b8a2b had to be written to call call_srcu() from the
>> device link device's release path, it's a mess of dependencies/delays:
>> 1. The device link device is part of the struct device_link. So we
>> can't free device_link before the device_link.link_dev refcount goes
>> to 0.
>> 2. But I can't assume device_link.link_dev's refcount will go to 0 as
>> soon as I call put_device() on it because of
>> CONFIG_DEBUG_KOBJECT_RELEASE which frees up the kobject after a random
>> delay.
>> 3. The use of SRCU also means I can't free device_link until the SRCU
>> is cleaned up.
>>
>> Because of (1), (2) and (3), when the device_link_del() (or any of the
>> other device link deletion APIs are called) I first have to do a
>> put_device(device_link.link_dev) to make sure the device memory is no
>> longer referenced, then trigger an SRCU clean up and then in the
>> scheduled SRCU cleanup I can free struct device_link. And obviously,
>> until struct device_link is ready to be freed up, I can't drop the
>> reference to the supplier and consumer devices (as that old copy of
>> device_link could be used by some code to refer to the supplier and
>> consumer devices).
>>
>> Hope that helps explain the SRCU and device link device release dependencies.
>>
>> Also, even if this patch series is applied as is, I wonder if the
>> current overlay code has a bug related to CONFIG_DEBUG_KOBJECT_RELEASE
>> delaying the actual freeing of the device. Something to look into?
> 
> I also tried enabling CONFIG_DEBUG_KOBJECT_RELEASE... with or without
> the addition of srcu_barrier() to device_links_purge(), I can't boot
> successfully when CONFIG_OF_UNITTEST=y &&
> CONFIG_DEBUG_KOBJECT_RELEASE=y: there are a ton of errors that result
> from this combo.

I'll add looking checking out booting with CONFIG_DEBUG_KOBJECT_RELEASE
enabled with CONFIG_OF_UNITTEST enabled to my todo list.

> 
> Disabling the unittests and booting with CONFIG_DEBUG_KOBJECT_RELEASE=y,
> I _do_ still see the errors mentioned in my original message when
> removing an overlay. So yeah, it does seem like there are some latent

Just to make sure I understand clearly, you are still seeing the
messages:

   OF: ERROR: memory leak, expected refcount 1 instead of 2, of_node_get()/of_node_put() unbalanced - destroy
   OF: ERROR: memory leak, expected refcount 1 instead of 2, of_node_get()/of_node_put() unbalanced - destroy

when the overlay is removed?  And if I apply patch 1/3 (the new unittest)
and also add srcu_barrier() at the end of device_links_purge()
then I will see these messages?

Can you add a reply to this email with the patch to add srcu_barrier() at
the end of device_links_purge() so that I am testing the same thing that
you are testing?  (If it makes life easier for you, you can just cut
and paste the patch into the reply, even if it results in a white
space damaged patch -- I'm assuming a one-line patch.)

-Frank

> issues here...
> 
> Cheers,
>  Michael
> 

Re: Re: [RFC PATCH 0/3] Fix errors on DT overlay removal with devlinks

[Michael Auchter]

On Thu, Oct 29, 2020 at 03:54:21PM -0500, Frank Rowand wrote:
> On 10/28/20 11:25 AM, Michael Auchter wrote:
> > Hey Saravana,
> > 
> > Thanks for taking the time to look into this!
> > 
> > On Mon, Oct 26, 2020 at 12:10:33PM -0700, Saravana Kannan wrote:
> >> On Wed, Oct 21, 2020 at 2:02 PM Frank Rowand <frowand.list@gmail.com> wrote:
> >>>
> >>> Hi Saravana,
> >>>
> >>> Michael found an issue related to the removal of a devicetree node
> >>> which involves devlinks:
> >>>
> >>> On 10/14/20 2:36 PM, Michael Auchter wrote:
> >>>> After updating to v5.9, I've started seeing errors in the kernel log
> >>>> when using device tree overlays. Specifically, the problem seems to
> >>>> happen when removing a device tree overlay that contains two devices
> >>>> with some dependency between them (e.g., a device that provides a clock
> >>>> and a device that consumes that clock). Removing such an overlay results
> >>>> in:
> >>>>
> >>>>   OF: ERROR: memory leak, expected refcount 1 instead of 2, of_node_get()/of_node_put() unbalanced - destroy
> >>>>   OF: ERROR: memory leak, expected refcount 1 instead of 2, of_node_get()/of_node_put() unbalanced - destroy
> >>>>
> >>>> followed by hitting some REFCOUNT_WARNs in refcount.c
> >>>>
> >>>> In the first patch, I've included a unittest that can be used to
> >>>> reproduce this when built with CONFIG_OF_UNITTEST [1].
> >>>>
> >>>> I believe the issue is caused by the cleanup performed when releasing
> >>>> the devlink device that's created to represent the dependency between
> >>>> devices. The devlink device has references to the consumer and supplier
> >>>> devices, which it drops in device_link_free; the devlink device's
> >>>> release callback calls device_link_free via call_srcu.
> >>>>
> >>>> When the overlay is being removed, all devices are removed, and
> >>>> eventually the release callback for the devlink device run, and
> >>>> schedules cleanup using call_srcu. Before device_link_free can and call
> >>>> put_device on the consumer/supplier, the rest of the overlay removal
> >>>> process runs, resulting in the error traces above.
> >>>
> >>> When a devicetree node in an overlay is removed, the remove code expects
> >>> all previous users of the related device to have done the appropriate put
> >>> of the device and to have no later references.
> >>>
> >>> As Michael described above, the devlink release callback defers the
> >>> put_device().  The cleanup via srcu was implemented in commit
> >>> 843e600b8a2b01463c4d873a90b2c2ea8033f1f6 "driver core: Fix sleeping
> >>> in invalid context during device link deletion" to solve yet another
> >>> issue.
> >>>
> >>>
> >>>>
> >>>> Patches 2 and 3 are an attempt at fixing this: call srcu_barrier to wait
> >>>> for any pending device_link_free's to execute before continuing on with
> >>>> the removal process.
> >>>>
> >>>> These patches resolve the issue, but probably not in the best way. In
> >>>> particular, it seems strange to need to leak details of devlinks into
> >>>> the device tree overlay code. So, I'd be curious to get some feedback or
> >>>> hear any other ideas for how to resolve this issue.
> >>>
> >>> I agree with Michael that adding an indirect call of srcu_barrier(&device_links_srcu)
> >>> into the devicetree overlay code is not an appropriate solution.
> >>
> >> I kind of see your point too. I wonder if the srcu_barrier() should
> >> happen inside like so:
> >> device_del() -> device_links_purge()->srcu_barrier()
> >>
> >> I don't know what contention the use of SRCUs in device links was
> >> trying to avoid, but I think the srcu_barrier() call path I suggested
> >> above shouldn't be a problem. If that fixes the issue, the best way to
> >> know if it's an issue is to send out a patch and see if Rafael has any
> >> problem with it :)
> > 
> > I was able to test this by adding the srcu_barrier() at the end of
> > device_links_purge(), and that does seem to have fixed the issue.
> > 
> >>> Is there some other way to fix the problem that 843e600b8a2b solves without
> >>> deferring the put_device() done by the devlink release callback?
> >>
> >> Ok I finally got some time to look into this closely.
> >>
> >> Even if you revert 843e600b8a2b, you'll see that device_link_free()
> >> (which drops the reference to the consumer and supplier devices) was
> >> scheduled to run when the SRCU clean up occurs. So I think this issue
> >> was present even before 843e600b8a2b, but commit 843e600b8a2b just
> >> made it more likely to hit this scenario because it introduces some
> >> delay in dropping the ref count of the supplier and consumer by going
> >> through the device link device's release path. So, I think this issue
> >> isn't related to 843e600b8a2b.
> >>
> >> As to why 843e600b8a2b had to be written to call call_srcu() from the
> >> device link device's release path, it's a mess of dependencies/delays:
> >> 1. The device link device is part of the struct device_link. So we
> >> can't free device_link before the device_link.link_dev refcount goes
> >> to 0.
> >> 2. But I can't assume device_link.link_dev's refcount will go to 0 as
> >> soon as I call put_device() on it because of
> >> CONFIG_DEBUG_KOBJECT_RELEASE which frees up the kobject after a random
> >> delay.
> >> 3. The use of SRCU also means I can't free device_link until the SRCU
> >> is cleaned up.
> >>
> >> Because of (1), (2) and (3), when the device_link_del() (or any of the
> >> other device link deletion APIs are called) I first have to do a
> >> put_device(device_link.link_dev) to make sure the device memory is no
> >> longer referenced, then trigger an SRCU clean up and then in the
> >> scheduled SRCU cleanup I can free struct device_link. And obviously,
> >> until struct device_link is ready to be freed up, I can't drop the
> >> reference to the supplier and consumer devices (as that old copy of
> >> device_link could be used by some code to refer to the supplier and
> >> consumer devices).
> >>
> >> Hope that helps explain the SRCU and device link device release dependencies.
> >>
> >> Also, even if this patch series is applied as is, I wonder if the
> >> current overlay code has a bug related to CONFIG_DEBUG_KOBJECT_RELEASE
> >> delaying the actual freeing of the device. Something to look into?
> > 
> > I also tried enabling CONFIG_DEBUG_KOBJECT_RELEASE... with or without
> > the addition of srcu_barrier() to device_links_purge(), I can't boot
> > successfully when CONFIG_OF_UNITTEST=y &&
> > CONFIG_DEBUG_KOBJECT_RELEASE=y: there are a ton of errors that result
> > from this combo.
> 
> I'll add looking checking out booting with CONFIG_DEBUG_KOBJECT_RELEASE
> enabled with CONFIG_OF_UNITTEST enabled to my todo list.
> 
> > 
> > Disabling the unittests and booting with CONFIG_DEBUG_KOBJECT_RELEASE=y,
> > I _do_ still see the errors mentioned in my original message when
> > removing an overlay. So yeah, it does seem like there are some latent
> 
> Just to make sure I understand clearly, you are still seeing the
> messages:
> 
>    OF: ERROR: memory leak, expected refcount 1 instead of 2, of_node_get()/of_node_put() unbalanced - destroy
>    OF: ERROR: memory leak, expected refcount 1 instead of 2, of_node_get()/of_node_put() unbalanced - destroy
> 
> when the overlay is removed?  And if I apply patch 1/3 (the new unittest)
> and also add srcu_barrier() at the end of device_links_purge()
> then I will see these messages?

Yes, that's correct: with CONFIG_DEBUG_KOBJECT_RELEASE=y + the new
unittest, I see those errors when booting, regardless of whether the
srcu_barrier is present in device_links_purge().

> Can you add a reply to this email with the patch to add srcu_barrier() at
> the end of device_links_purge() so that I am testing the same thing that
> you are testing?  (If it makes life easier for you, you can just cut
> and paste the patch into the reply, even if it results in a white
> space damaged patch -- I'm assuming a one-line patch.)

Sure thing:

From 87d9e27d1c10ec35a82db0c6d2b4d87ba22bbda5 Mon Sep 17 00:00:00 2001
From: Michael Auchter <michael.auchter@ni.com>
Date: Wed, 28 Oct 2020 15:43:04 -0500
Subject: [PATCH] driver core: wait for srcu callbacks in device_links_purge

Suggested-by: Saravana Kannan <saravanak@google.com>
Signed-off-by: Michael Auchter <michael.auchter@ni.com>
---
 drivers/base/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index f90e9f77bf8c..e131cc2e7083 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1399,6 +1399,8 @@ static void device_links_purge(struct device *dev)
 	}
 
 	device_links_write_unlock();
+
+	srcu_barrier(&device_links_srcu);
 }
 
 static u32 fw_devlink_flags = DL_FLAG_SYNC_STATE_ONLY;
-- 
2.25.4