linux-kernel - Re: [PATCH v3 04/28] driver core: Avoid warning when removing a device while its supplier is unbinding

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGETcx85vKLcFcWBR2G005Nup8UkmVZFLYgd4DuUBOKxHvJn8Q@mail.gmail.com>
Date: Wed, 2 Jul 2025 14:02:20 -0700
From: Saravana Kannan <saravanak@...gle.com>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: Herve Codina <herve.codina@...tlin.com>, Andrew Lunn <andrew@...n.ch>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.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>, 
	Bjorn Helgaas <bhelgaas@...gle.com>, Mark Brown <broonie@...nel.org>, Len Brown <lenb@...nel.org>, 
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, 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>, Davidlohr Bueso <dave@...olabs.net>, 
	Dave Jiang <dave.jiang@...el.com>, Alison Schofield <alison.schofield@...el.com>, 
	Vishal Verma <vishal.l.verma@...el.com>, Ira Weiny <ira.weiny@...el.com>, 
	Dan Williams <dan.j.williams@...el.com>, 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, 
	linux-cxl@...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 v3 04/28] driver core: Avoid warning when removing a
 device while its supplier is unbinding

On Wed, Jul 2, 2025 at 11:22 AM Rafael J. Wysocki <rafael@...nel.org> wrote:
>
> On Fri, Jun 13, 2025 at 3:48 PM Herve Codina <herve.codina@...tlin.com> 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.
> >
> > Signed-off-by: Herve Codina <herve.codina@...tlin.com>
>
> The change is OK, so
>
> Reviewed-by: Rafael J. Wysocki <rafael@...nel.org>

You can add mine too.

Reviewed-by: Saravana Kannan <saravanak@...gle.com>

-Saravana

>
> but it will clash with
>
> https://lore.kernel.org/linux-pm/2793309.mvXUDI8C0e@rjwysocki.net/
>
> that's been queued up by Greg AFAICS.
>
> > ---
> >  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 8fead097c404..ce367c44f642 100644
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -1494,7 +1494,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->supplier->links.status != DL_DEV_UNBINDING &&
> > +                               !(link->flags & DL_FLAG_SYNC_STATE_ONLY));
> >                         WRITE_ONCE(link->status, DL_STATE_DORMANT);
> >                 }
> >         }
> > --
> > 2.49.0
> >