From: "Kuppuswamy, Sathyanarayanan" <sathyanarayanan.kuppuswamy@linux.intel.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
ashok.raj@intel.com
Subject: Re: [PATCH v18 03/11] PCI/DPC: Fix DPC recovery issue in non hotplug case
Date: Sat, 28 Mar 2020 15:40:23 -0700 [thread overview]
Message-ID: <621ef46b-3556-e667-3982-16f3c908a793@linux.intel.com> (raw)
In-Reply-To: <20200328222111.GA136384@google.com>
On 3/28/20 3:21 PM, Bjorn Helgaas wrote:
>>> OK, thanks. I'm still uncomfortable with this issue, so I think I'm
>>> going to apply this series but omit this patch. Here's why:
>>>
>>> 1) The fact that resets may cause hotplug events isn't specific to
>>> DPC, so I don't think dpc_reset_link() is the right place. For
>>> instance, aer_root_reset() ultimately does a secondary bus reset.
>> Agree. Reset is common for pci_channel_io_frozen errors. I did not
>> look into aer_root_reset() implementation. So if state
>> is pci_channel_io_frozen then we can assume the slot has been
>> reseted.
>> The
>>> pci_slot_reset() -> pciehp_reset_slot() path goes to some trouble to
>>> ignore the resulting hotplug event, but the pci_bus_reset() path does
>>> not.
>>>
>>> 2) I'm not convinced that "hotplug_is_native()" is the correct test.
>>> Even if we're using ACPI hotplug (acpiphp), that will detach the
>>> drivers and remove the devices, won't it?
>> Yes, agreed. It does not handle ACPI hotplug case. In case of
>> ACPI hotplug, native_pcie_hotplug = 0. May be we need a new helper
>> function. hotplug_is_enabled() ?
> I'm not proposing the patch below to be applied. I only included it
> as an idea of where the hotplug testing should be.
>
> I'm proposing to merge the pci/edr branch as-is, without these two
> patches:
>
> PCI: move {pciehp,shpchp}_is_native() definitions to pci.c
> PCI/DPC: Fix DPC recovery issue in non hotplug case
>
> accepting that we still have some issues in the non-hotplug case that
> we can fix later.
Ok. I am fine with it. Thanks for working on it.
>
next prev parent reply other threads:[~2020-03-28 22:40 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-24 0:25 [PATCH v18 00/11] Add Error Disconnect Recover (EDR) support sathyanarayanan.kuppuswamy
2020-03-24 0:25 ` [PATCH v18 01/11] PCI/ERR: Update error status after reset_link() sathyanarayanan.kuppuswamy
2020-03-24 0:25 ` [PATCH v18 02/11] PCI: move {pciehp,shpchp}_is_native() definitions to pci.c sathyanarayanan.kuppuswamy
2020-03-24 0:26 ` [PATCH v18 03/11] PCI/DPC: Fix DPC recovery issue in non hotplug case sathyanarayanan.kuppuswamy
2020-03-24 23:49 ` Bjorn Helgaas
2020-03-25 1:17 ` Kuppuswamy, Sathyanarayanan
2020-03-28 17:10 ` Bjorn Helgaas
2020-03-28 22:04 ` Kuppuswamy, Sathyanarayanan
2020-03-28 22:21 ` Bjorn Helgaas
2020-03-28 22:40 ` Kuppuswamy, Sathyanarayanan [this message]
2020-03-24 0:26 ` [PATCH v18 04/11] PCI/DPC: Move DPC data into struct pci_dev sathyanarayanan.kuppuswamy
2020-03-24 0:26 ` [PATCH v18 05/11] PCI/ERR: Remove service dependency in pcie_do_recovery() sathyanarayanan.kuppuswamy
2020-03-28 21:12 ` Kuppuswamy, Sathyanarayanan
2020-03-28 21:32 ` Bjorn Helgaas
2020-03-28 21:55 ` Kuppuswamy, Sathyanarayanan
2020-03-28 22:16 ` Bjorn Helgaas
2020-03-24 0:26 ` [PATCH v18 06/11] PCI/ERR: Return status of pcie_do_recovery() sathyanarayanan.kuppuswamy
2020-03-24 0:26 ` [PATCH v18 07/11] PCI/DPC: Cache DPC capabilities in pci_init_capabilities() sathyanarayanan.kuppuswamy
2020-03-24 0:26 ` [PATCH v18 08/11] PCI/AER: Add pci_aer_raw_clear_status() to unconditionally clear Error Status sathyanarayanan.kuppuswamy
2020-03-24 0:26 ` [PATCH v18 09/11] PCI/DPC: Expose dpc_process_error(), dpc_reset_link() for use by EDR sathyanarayanan.kuppuswamy
2020-03-24 0:26 ` [PATCH v18 10/11] PCI/DPC: Add Error Disconnect Recover (EDR) support sathyanarayanan.kuppuswamy
2020-03-24 21:37 ` Bjorn Helgaas
2020-03-25 1:00 ` Kuppuswamy, Sathyanarayanan
2020-03-26 22:36 ` Bjorn Helgaas
2024-04-11 18:07 ` Bjorn Helgaas
2024-04-11 19:16 ` Kuppuswamy Sathyanarayanan
2020-03-24 0:26 ` [PATCH v18 11/11] PCI/AER: Rationalize error status register clearing sathyanarayanan.kuppuswamy
2020-03-31 15:28 ` [PATCH v18 00/11] Add Error Disconnect Recover (EDR) support Bjorn Helgaas
2020-03-31 16:28 ` Kuppuswamy, Sathyanarayanan
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=621ef46b-3556-e667-3982-16f3c908a793@linux.intel.com \
--to=sathyanarayanan.kuppuswamy@linux.intel.com \
--cc=ashok.raj@intel.com \
--cc=helgaas@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@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).