During driver unbinding, __device_links_no_driver() can raise the following warning: --- 8< --- WARNING: CPU: 0 PID: 166 at drivers/base/core.c:1426 __device_links_no_driver+0xac/0xb4 ... Call trace: __device_links_no_driver+0xac/0xb4 device_links_driver_cleanup+0xa8/0xf0 device_release_driver_internal+0x204/0x240 device_release_driver+0x18/0x24 bus_remove_device+0xcc/0x10c device_del+0x158/0x414 platform_device_del.part.0+0x1c/0x88 platform_device_unregister+0x24/0x40 of_platform_device_destroy+0xfc/0x10c device_for_each_child_reverse+0x64/0xb4 devm_of_platform_populate_release+0x4c/0x84 release_nodes+0x5c/0x90 devres_release_all+0x8c/0xdc device_unbind_cleanup+0x18/0x68 device_release_driver_internal+0x20c/0x240 device_links_unbind_consumers+0xe0/0x108 device_release_driver_internal+0xf0/0x240 driver_detach+0x50/0x9c bus_remove_driver+0x6c/0xbc driver_unregister+0x30/0x60 ... --- 8< ---
This warning is raised because, during device removal, we unlink a consumer while its supplier links.status is DL_DEV_UNBINDING. Even if the link is not a SYNC_STATE_ONLY, the warning should not appear in that case.
Filter out this warning in case of the supplier driver is unbinding.
Fixes: 8c3e315d4296 ("driver core: Update device link status correctly for SYNC_STATE_ONLY links") Cc: stable@vger.kernel.org Signed-off-by: Herve Codina herve.codina@bootlin.com --- drivers/base/core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c index 17f2568e0a79..f4b09691998e 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -1423,7 +1423,8 @@ static void __device_links_no_driver(struct device *dev) if (link->supplier->links.status == DL_DEV_DRIVER_BOUND) { WRITE_ONCE(link->status, DL_STATE_AVAILABLE); } else { - WARN_ON(!(link->flags & DL_FLAG_SYNC_STATE_ONLY)); + WARN_ON(!(link->flags & DL_FLAG_SYNC_STATE_ONLY) && + link->supplier->links.status != DL_DEV_UNBINDING); WRITE_ONCE(link->status, DL_STATE_DORMANT); } }
On Fri, Nov 10, 2023 at 12:02 AM Herve Codina herve.codina@bootlin.com wrote:
During driver unbinding, __device_links_no_driver() can raise the following warning: --- 8< --- WARNING: CPU: 0 PID: 166 at drivers/base/core.c:1426 __device_links_no_driver+0xac/0xb4 ... Call trace: __device_links_no_driver+0xac/0xb4 device_links_driver_cleanup+0xa8/0xf0 device_release_driver_internal+0x204/0x240 device_release_driver+0x18/0x24 bus_remove_device+0xcc/0x10c device_del+0x158/0x414 platform_device_del.part.0+0x1c/0x88 platform_device_unregister+0x24/0x40 of_platform_device_destroy+0xfc/0x10c device_for_each_child_reverse+0x64/0xb4 devm_of_platform_populate_release+0x4c/0x84 release_nodes+0x5c/0x90 devres_release_all+0x8c/0xdc device_unbind_cleanup+0x18/0x68 device_release_driver_internal+0x20c/0x240 device_links_unbind_consumers+0xe0/0x108 device_release_driver_internal+0xf0/0x240 driver_detach+0x50/0x9c bus_remove_driver+0x6c/0xbc driver_unregister+0x30/0x60 ... --- 8< ---
This warning is raised because, during device removal, we unlink a consumer while its supplier links.status is DL_DEV_UNBINDING. Even if the link is not a SYNC_STATE_ONLY, the warning should not appear in that case.
Filter out this warning in case of the supplier driver is unbinding.
Fixes: 8c3e315d4296 ("driver core: Update device link status correctly for SYNC_STATE_ONLY links")
Wrong Fixes tag. I just added the SYNC_STATE_ONLY exception. The issue has been there since before.
Cc: stable@vger.kernel.org Signed-off-by: Herve Codina herve.codina@bootlin.com
drivers/base/core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c index 17f2568e0a79..f4b09691998e 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -1423,7 +1423,8 @@ static void __device_links_no_driver(struct device *dev) if (link->supplier->links.status == DL_DEV_DRIVER_BOUND) { WRITE_ONCE(link->status, DL_STATE_AVAILABLE); } else {
WARN_ON(!(link->flags & DL_FLAG_SYNC_STATE_ONLY));
WARN_ON(!(link->flags & DL_FLAG_SYNC_STATE_ONLY) &&link->supplier->links.status != DL_DEV_UNBINDING);
Don't delete the warning please. Make it better so it doesn't warn when it shouldn't.
This combined with the other patches you sent make me think this is more of an issue in the device removal ordering than an actual issue with the warning. I'm not fully convinced the warning is incorrect yet.
-Saravana
WRITE_ONCE(link->status, DL_STATE_DORMANT); } }-- 2.41.0
Hi Saravana,
On Fri, 10 Nov 2023 17:42:26 -0800 Saravana Kannan saravanak@google.com wrote:
On Fri, Nov 10, 2023 at 12:02 AM Herve Codina herve.codina@bootlin.com wrote:
During driver unbinding, __device_links_no_driver() can raise the following warning: --- 8< --- WARNING: CPU: 0 PID: 166 at drivers/base/core.c:1426 __device_links_no_driver+0xac/0xb4 ... Call trace: __device_links_no_driver+0xac/0xb4 device_links_driver_cleanup+0xa8/0xf0 device_release_driver_internal+0x204/0x240 device_release_driver+0x18/0x24 bus_remove_device+0xcc/0x10c device_del+0x158/0x414 platform_device_del.part.0+0x1c/0x88 platform_device_unregister+0x24/0x40 of_platform_device_destroy+0xfc/0x10c device_for_each_child_reverse+0x64/0xb4 devm_of_platform_populate_release+0x4c/0x84 release_nodes+0x5c/0x90 devres_release_all+0x8c/0xdc device_unbind_cleanup+0x18/0x68 device_release_driver_internal+0x20c/0x240 device_links_unbind_consumers+0xe0/0x108 device_release_driver_internal+0xf0/0x240 driver_detach+0x50/0x9c bus_remove_driver+0x6c/0xbc driver_unregister+0x30/0x60 ... --- 8< ---
This warning is raised because, during device removal, we unlink a consumer while its supplier links.status is DL_DEV_UNBINDING. Even if the link is not a SYNC_STATE_ONLY, the warning should not appear in that case.
Filter out this warning in case of the supplier driver is unbinding.
Fixes: 8c3e315d4296 ("driver core: Update device link status correctly for SYNC_STATE_ONLY links")
Wrong Fixes tag. I just added the SYNC_STATE_ONLY exception. The issue has been there since before.
This commit adds the check if (link->supplier->links.status == DL_DEV_DRIVER_BOUND) to set the link.status to DL_STATE_CONSUMER_PROBE or DL_STATE_DORMANT.
Also this commit adds the warning on !(link->flags & DL_FLAG_SYNC_STATE_ONLY)
Cc: stable@vger.kernel.org Signed-off-by: Herve Codina herve.codina@bootlin.com
drivers/base/core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c index 17f2568e0a79..f4b09691998e 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -1423,7 +1423,8 @@ static void __device_links_no_driver(struct device *dev) if (link->supplier->links.status == DL_DEV_DRIVER_BOUND) { WRITE_ONCE(link->status, DL_STATE_AVAILABLE); } else {
WARN_ON(!(link->flags & DL_FLAG_SYNC_STATE_ONLY));
WARN_ON(!(link->flags & DL_FLAG_SYNC_STATE_ONLY) &&link->supplier->links.status != DL_DEV_UNBINDING);Don't delete the warning please. Make it better so it doesn't warn when it shouldn't.
This combined with the other patches you sent make me think this is more of an issue in the device removal ordering than an actual issue with the warning. I'm not fully convinced the warning is incorrect yet.
When link->supplier->links.status == DL_DEV_UNBINDING, what should be the link->status set ? DL_STATE_DORMANT seems correct in that case.
Removing or not the warning in that case depends on the answer to: Is DL_FLAG_SYNC_STATE_ONLY should be set in link->flags on all calls to __device_links_no_driver() with link->supplier->links.status set to DL_DEV_UNBINDING ?
I lack the knowledge to answer perfectly to this question. Can you help me on this point ?
Best regards, Hervé
linux-stable-mirror@lists.linaro.org
-
Herve Codina -
Saravana Kannan