From: Lukas Wunner <lukas@wunner.de>
To: Guilherme Giacomo Simoes <trintaeoitogc@gmail.com>
Cc: scott@spiteful.org, bhelgaas@google.com,
ilpo.jarvinen@linux.intel.com, wsa+renesas@sang-engineering.com,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
Nam Cao <namcao@linutronix.de>
Subject: Re: [PATCH] PCI: hotplug: check the return of hotplug bridge
Date: Sat, 17 Aug 2024 07:20:51 +0200 [thread overview]
Message-ID: <ZsAzM8K9PnN5jxR9@wunner.de> (raw)
In-Reply-To: <20240817032228.6844-1-trintaeoitogc@gmail.com>
[shorten subject, cc += Nam Cao, start of thread:
https://lore.kernel.org/all/20240817032228.6844-1-trintaeoitogc@gmail.com/
]
On Sat, Aug 17, 2024 at 12:22:27AM -0300, Guilherme Giacomo Simoes wrote:
> Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@gmail.com>
Hm, the body of the commit message ended up in the subject
and the patch was submitted twice.
> --- a/drivers/pci/hotplug/shpchp_pci.c
> +++ b/drivers/pci/hotplug/shpchp_pci.c
> @@ -48,8 +48,11 @@ int shpchp_configure_device(struct slot *p_slot)
> }
>
> for_each_pci_bridge(dev, parent) {
> - if (PCI_SLOT(dev->devfn) == p_slot->device)
> - pci_hp_add_bridge(dev);
> + if (PCI_SLOT(dev->devfn) == p_slot->device) {
> + ret = pci_hp_add_bridge(dev);
> + if (ret)
> + goto out;
> + }
> }
>
> pci_assign_unassigned_bridge_resources(bridge);
Nam Cao worked on this back in May:
v1:
https://lore.kernel.org/all/cover.1714762038.git.namcao@linutronix.de/
v2:
https://lore.kernel.org/all/cover.1714838173.git.namcao@linutronix.de/
v3:
https://lore.kernel.org/all/cover.1715609848.git.namcao@linutronix.de/
Note that there was discussion on v2 after v3 had been submitted,
i.e. the last messages in the discussion are in the v2 thread.
Nam Cao's patches didn't get applied, I think we hadn't reached
consensus or were waiting for a v4.
Nam Cao's v2 uses the exact same approach that you're proposing
and they subsequently found a way to crash the kernel despite the
newly introduced error handling:
https://lore.kernel.org/all/20240506083701.NZNifFGn@linutronix.de/
So I'm afraid your patch may not work in every scenario.
Thanks,
Lukas
next prev parent reply other threads:[~2024-08-17 5:29 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-17 3:22 [PATCH] PCI: hotplug: check the return of hotplug bridge In some pci drivers, when the pci bridge is added if the process return an error, the drivers don't check this and continue your execute normally. Then, this patch change this drivers for check return of pci_hp_add_bridge(), and if has an error, then the drivers call goto lable , free your mutex and return the error for your caller Guilherme Giacomo Simoes
2024-08-17 5:20 ` Lukas Wunner [this message]
-- strict thread matches above, loose matches on Subject: below --
2024-08-17 13:24 [PATCH] PCI: hotplug: check the return of hotplug bridge Guilherme Giacomo Simoes
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=ZsAzM8K9PnN5jxR9@wunner.de \
--to=lukas@wunner.de \
--cc=bhelgaas@google.com \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=namcao@linutronix.de \
--cc=scott@spiteful.org \
--cc=trintaeoitogc@gmail.com \
--cc=wsa+renesas@sang-engineering.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox