linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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.
> 

  reply	other threads:[~2020-03-28 22:40 UTC|newest]

Thread overview: 27+ 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
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).