All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johan Hovold <johan@kernel.org>
To: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Johan Hovold <johan+linaro@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Manivannan Sadhasivam <mani@kernel.org>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: Re: [PATCH 1/3] PCI/pwrctrl: Fix device leak at registration
Date: Tue, 22 Jul 2025 14:00:50 +0200	[thread overview]
Message-ID: <aH99cmBkOwCOkIZk@hovoldconsulting.com> (raw)
In-Reply-To: <20250722110526.00002a60@huawei.com>

On Tue, Jul 22, 2025 at 11:05:26AM +0100, Jonathan Cameron wrote:
> On Mon, 21 Jul 2025 17:36:07 +0200
> Johan Hovold <johan+linaro@kernel.org> wrote:

> Perhaps time for 
> DEFINE_FREE(put_pdev, struct platform_device *, if (_T) put_device(&_T->dev));
> 
> then...
> 
> > ---
> >  drivers/pci/bus.c | 14 +++++++++-----
> >  1 file changed, 9 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> > index 69048869ef1c..0394a9c77b38 100644
> > --- a/drivers/pci/bus.c
> > +++ b/drivers/pci/bus.c
> > @@ -362,11 +362,15 @@ void pci_bus_add_device(struct pci_dev *dev)
> >  	 * before PCI client drivers.
> >  	 */
> >  	pdev = of_find_device_by_node(dn);
> > -	if (pdev && of_pci_supply_present(dn)) {
> > -		if (!device_link_add(&dev->dev, &pdev->dev,
> > -				     DL_FLAG_AUTOREMOVE_CONSUMER))
> > -			pci_err(dev, "failed to add device link to power control device %s\n",
> > -				pdev->name);
> 
> 	struct platform_device *pdev __free(put_pdev) =
> 		of_find_device_by_node(dn);
> > +	if (pdev) {
> > +		if (of_pci_supply_present(dn)) {
> > +			if (!device_link_add(&dev->dev, &pdev->dev,
> > +					     DL_FLAG_AUTOREMOVE_CONSUMER)) {
> > +				pci_err(dev, "failed to add device link to power control device %s\n",
> > +					pdev->name);
> > +			}
> > +		}
> > +		put_device(&pdev->dev);
> 
> and no need for any explicit put.
> 
> We already do this extensively in some subsystems (e.g. CXL) and it
> greatly simplifies code.

No, I'm no fan of those kind of changes which I find leads to less
readable code (e.g. with those in-code declarations).

Johan

  reply	other threads:[~2025-07-22 12:01 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-21 15:36 [PATCH 0/3] PCI/pwrctrl: Fix device and OF node leaks Johan Hovold
2025-07-21 15:36 ` [PATCH 1/3] PCI/pwrctrl: Fix device leak at registration Johan Hovold
2025-07-22 10:05   ` Jonathan Cameron
2025-07-22 12:00     ` Johan Hovold [this message]
2025-07-22 18:33   ` Markus Elfring
2025-07-21 15:36 ` [PATCH 2/3] PCI/pwrctrl: Fix device and OF node leak at bus scan Johan Hovold
2025-07-22 10:08   ` Jonathan Cameron
2025-07-22 12:04     ` Johan Hovold
2025-07-21 15:36 ` [PATCH 3/3] PCI/pwrctrl: Fix device leak at device stop Johan Hovold
2025-07-23 16:18 ` [PATCH 0/3] PCI/pwrctrl: Fix device and OF node leaks Manivannan Sadhasivam
2025-08-27  9:49 ` Johan Hovold
2025-08-27 19:35 ` Bjorn Helgaas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aH99cmBkOwCOkIZk@hovoldconsulting.com \
    --to=johan@kernel.org \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=bhelgaas@google.com \
    --cc=johan+linaro@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mani@kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.