From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Dominique Martinet <dominique.martinet@atmark-techno.com>
Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org,
Sean V Kelley <sean.v.kelley@intel.com>,
Bjorn Helgaas <bhelgaas@google.com>,
Sasha Levin <sashal@kernel.org>,
Jonathan Cameron <Jonathan.Cameron@huawei.com>,
linux-pci@vger.kernel.org, Keith Busch <kbusch@kernel.org>,
Hinko Kocevar <hinko.kocevar@ess.eu>,
Dan Williams <dan.j.williams@intel.com>,
"Kuppuswamy,
Sathyanarayanan" <sathyanarayanan.kuppuswamy@intel.com>
Subject: Re: [PATCH 5.10 480/545] PCI/ERR: Add pci_walk_bridge() to pcie_do_recovery()
Date: Tue, 23 Aug 2022 09:11:12 +0200 [thread overview]
Message-ID: <YwR9kFH3UqhkV4d0@kroah.com> (raw)
In-Reply-To: <YwL/brvUP1aiwo93@atmark-techno.com>
On Mon, Aug 22, 2022 at 01:00:46PM +0900, Dominique Martinet wrote:
> Greg Kroah-Hartman wrote on Fri, Aug 19, 2022 at 05:44:10PM +0200:
> > From: Sean V Kelley <sean.v.kelley@intel.com>
> >
> > [ Upstream commit 05e9ae19ab83881a0f33025bd1288e41e552a34b ]
> >
> > Consolidate subordinate bus checks with pci_walk_bus() into
> > pci_walk_bridge() for walking below potentially AER affected bridges.
> >
> > Link: https://lore.kernel.org/r/20201121001036.8560-10-sean.v.kelley@intel.com
> > Tested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> # non-native/no RCEC
> > Signed-off-by: Sean V Kelley <sean.v.kelley@intel.com>
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > Signed-off-by: Sasha Levin <sashal@kernel.org>
> > ---
> > drivers/pci/pcie/err.c | 30 +++++++++++++++++++++++-------
> > 1 file changed, 23 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
> > index 931e75f2549d..8b53aecdb43d 100644
> > --- a/drivers/pci/pcie/err.c
> > +++ b/drivers/pci/pcie/err.c
> > [...]
> > @@ -165,23 +182,22 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
> > else
> > bridge = pci_upstream_bridge(dev);
> >
> > - bus = bridge->subordinate;
> > pci_dbg(bridge, "broadcast error_detected message\n");
> > if (state == pci_channel_io_frozen) {
> > - pci_walk_bus(bus, report_frozen_detected, &status);
> > + pci_walk_bridge(bridge, report_frozen_detected, &status);
> > status = reset_subordinates(bridge);
> > if (status != PCI_ERS_RESULT_RECOVERED) {
> > pci_warn(bridge, "subordinate device reset failed\n");
> > goto failed;
> > }
>
> A local conflict merging this made me notice a later commit:
> -----
> commit 387c72cdd7fb6bef650fb078d0f6ae9682abf631
> Author: Keith Busch <kbusch@kernel.org>
> Date: Mon Jan 4 15:02:58 2021 -0800
>
> PCI/ERR: Retain status from error notification
>
> Overwriting the frozen detected status with the result of the link reset
> loses the NEED_RESET result that drivers are depending on for error
> handling to report the .slot_reset() callback. Retain this status so
> that subsequent error handling has the correct flow.
>
> Link: https://lore.kernel.org/r/20210104230300.1277180-4-kbusch@kernel.org
> Reported-by: Hinko Kocevar <hinko.kocevar@ess.eu>
> Tested-by: Hedi Berriche <hedi.berriche@hpe.com>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Acked-by: Sean V Kelley <sean.v.kelley@intel.com>
> Acked-by: Hedi Berriche <hedi.berriche@hpe.com>
>
> diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
> index a84f0bf4c1e2..b576aa890c76 100644
> --- a/drivers/pci/pcie/err.c
> +++ b/drivers/pci/pcie/err.c
> @@ -198,8 +198,7 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
> pci_dbg(bridge, "broadcast error_detected message\n");
> if (state == pci_channel_io_frozen) {
> pci_walk_bridge(bridge, report_frozen_detected, &status);
> - status = reset_subordinates(bridge);
> - if (status != PCI_ERS_RESULT_RECOVERED) {
> + if (reset_subordinates(bridge) != PCI_ERS_RESULT_RECOVERED) {
> pci_warn(bridge, "subordinate device reset failed\n");
> goto failed;
> }
> -----
>
> Since this (commit I reply to) has been picked up, I think it'd make
> sense to also include this (commit I just listed) in a later 5.10 tag.
> It cherry-picks without error but would you like me to resend?
> (I have added in Cc all involved people to this mail)
>
> Digging through the mails the patch came with seem to imply approval for
> stable merges; but it didn't make sense until pci_walk_bridge() had been
> added just now. Now it's here we probably want both:
> https://lore.kernel.org/all/d9ee4151-b28d-a52a-b5be-190a75e0e49b@intel.com/
>
>
> (I noticed because the NXP kernel we are provided includes a different
> "fix" for what I believe to be the same issue, previously discussed here:
> https://lore.kernel.org/linux-pci/12115.1588207324@famine/
>
> I haven't actually encountered any of the problems discribed, so this is
> purely theorical for me; it just looks a bit weird.)
I've queued up the commit you referenced above now, thanks!
greg k-h
prev parent reply other threads:[~2022-08-23 7:11 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20220819153850.911668266@linuxfoundation.org>
2022-08-22 4:00 ` [PATCH 5.10 480/545] PCI/ERR: Add pci_walk_bridge() to pcie_do_recovery() Dominique Martinet
2022-08-23 7:11 ` Greg Kroah-Hartman [this message]
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=YwR9kFH3UqhkV4d0@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=Jonathan.Cameron@huawei.com \
--cc=bhelgaas@google.com \
--cc=dan.j.williams@intel.com \
--cc=dominique.martinet@atmark-techno.com \
--cc=hinko.kocevar@ess.eu \
--cc=kbusch@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=sashal@kernel.org \
--cc=sathyanarayanan.kuppuswamy@intel.com \
--cc=sean.v.kelley@intel.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).