| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aBt5FvZ95S1Y_Mba@smile.fi.intel.com>
Date: Wed, 7 May 2025 18:15:34 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Herve Codina <herve.codina@...tlin.com>
Cc: Andrew Lunn <andrew@...n.ch>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Danilo Krummrich <dakr@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>, Andi Shyti <andi.shyti@...nel.org>,
Wolfram Sang <wsa+renesas@...g-engineering.com>,
Peter Rosin <peda@...ntia.se>,
Derek Kiernan <derek.kiernan@....com>,
Dragan Cvetic <dragan.cvetic@....com>,
Arnd Bergmann <arnd@...db.de>, Rob Herring <robh@...nel.org>,
Saravana Kannan <saravanak@...gle.com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Mark Brown <broonie@...nel.org>, Len Brown <lenb@...nel.org>,
Daniel Scally <djrscally@...il.com>,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
Sakari Ailus <sakari.ailus@...ux.intel.com>,
Wolfram Sang <wsa@...nel.org>,
Geert Uytterhoeven <geert+renesas@...der.be>,
linux-kernel@...r.kernel.org, imx@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, linux-clk@...r.kernel.org,
linux-i2c@...r.kernel.org, devicetree@...r.kernel.org,
linux-pci@...r.kernel.org, linux-spi@...r.kernel.org,
linux-acpi@...r.kernel.org,
Allan Nielsen <allan.nielsen@...rochip.com>,
Horatiu Vultur <horatiu.vultur@...rochip.com>,
Steen Hegelund <steen.hegelund@...rochip.com>,
Luca Ceresoli <luca.ceresoli@...tlin.com>,
Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH v2 04/26] driver core: Avoid warning when removing a
device while its supplier is unbinding
On Wed, May 07, 2025 at 09:12:46AM +0200, Herve Codina wrote:
> During driver removal, the following warning can appear:
> WARNING: CPU: 1 PID: 139 at drivers/base/core.c:1497 __device_links_no_driver+0xcc/0xfc
> ...
> Call trace:
> __device_links_no_driver+0xcc/0xfc (P)
> device_links_driver_cleanup+0xa8/0xf0
> device_release_driver_internal+0x208/0x23c
> device_links_unbind_consumers+0xe0/0x108
> device_release_driver_internal+0xec/0x23c
> device_links_unbind_consumers+0xe0/0x108
> device_release_driver_internal+0xec/0x23c
> device_links_unbind_consumers+0xe0/0x108
> device_release_driver_internal+0xec/0x23c
> driver_detach+0xa0/0x12c
> bus_remove_driver+0x6c/0xbc
> driver_unregister+0x30/0x60
> pci_unregister_driver+0x20/0x9c
> lan966x_pci_driver_exit+0x18/0xa90 [lan966x_pci]
>
> This warning is triggered when a consumer is removed because the links
> status of its supplier is not DL_DEV_DRIVER_BOUND and the link flag
> DL_FLAG_SYNC_STATE_ONLY is not set.
>
> The topology in terms of consumers/suppliers used was the following
> (consumer ---> supplier):
>
> i2c -----------> OIC ----> PCI device
> | ^
> | |
> +---> pinctrl ---+
>
> When the PCI device is removed, the OIC (interrupt controller) has to be
> removed. In order to remove the OIC, pinctrl and i2c need to be removed
> and to remove pinctrl, i2c need to be removed. The removal order is:
> 1) i2c
> 2) pinctrl
> 3) OIC
> 4) PCI device
>
> In details, the removal sequence is the following (with 0000:01:00.0 the
> PCI device):
> driver_detach: call device_release_driver_internal(0000:01:00.0)...
> device_links_busy(0000:01:00.0):
> links->status = DL_DEV_UNBINDING
> device_links_unbind_consumers(0000:01:00.0):
> 0000:01:00.0--oic link->status = DL_STATE_SUPPLIER_UNBIND
> call device_release_driver_internal(oic)...
> device_links_busy(oic):
> links->status = DL_DEV_UNBINDING
> device_links_unbind_consumers(oic):
> oic--pinctrl link->status = DL_STATE_SUPPLIER_UNBIND
> call device_release_driver_internal(pinctrl)...
> device_links_busy(pinctrl):
> links->status = DL_DEV_UNBINDING
> device_links_unbind_consumers(pinctrl):
> pinctrl--i2c link->status = DL_STATE_SUPPLIER_UNBIND
> call device_release_driver_internal(i2c)...
> device_links_busy(i2c): links->status = DL_DEV_UNBINDING
> __device_links_no_driver(i2c)...
> pinctrl--i2c link->status is DL_STATE_SUPPLIER_UNBIND
> oic--i2c link->status is DL_STATE_ACTIVE
> oic--i2c link->supplier->links.status is DL_DEV_UNBINDING
>
> The warning is triggered by the i2c removal because the OIC (supplier)
> links status is not DL_DEV_DRIVER_BOUND. Its links status is indeed set
> to DL_DEV_UNBINDING.
>
> It is perfectly legit to have the links status set to DL_DEV_UNBINDING
> in that case. Indeed we had started to unbind the OIC which triggered
> the consumer unbinding and didn't finish yet when the i2c is unbound.
>
> Avoid the warning when the supplier links status is set to
> DL_DEV_UNBINDING and thus support this removal sequence without any
> warnings.
...
> 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));
> + if (link->supplier->links.status != DL_DEV_UNBINDING)
> + WARN_ON(!(link->flags & DL_FLAG_SYNC_STATE_ONLY));
Why not
WARN_ON(link->supplier->links.status != DL_DEV_UNBINDING &&
!(link->flags & DL_FLAG_SYNC_STATE_ONLY));
> WRITE_ONCE(link->status, DL_STATE_DORMANT);
> }
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists