archive mirror
 help / color / mirror / Atom feed
From: "Kuppuswamy, Sathyanarayanan"  <>
To: Ethan Zhao <>
Cc: Kuppuswamy Sathyanarayanan 
	Bjorn Helgaas <>,
	Sinan Kaya <>,
	linux-pci <>,
	Linux Kernel Mailing List <>,
	Ashok Raj <>
Subject: Re: [PATCH v6 2/2] PCI/ERR: Split the fatal and non-fatal error recovery handling
Date: Wed, 14 Oct 2020 20:04:17 -0700	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On 10/14/20 6:58 PM, Ethan Zhao wrote:
> On Thu, Oct 15, 2020 at 1:06 AM Kuppuswamy, Sathyanarayanan
> <> wrote:
>> On 10/14/20 8:07 AM, Ethan Zhao wrote:
>>> On Wed, Oct 14, 2020 at 5:00 PM Kuppuswamy Sathyanarayanan
>>> <> wrote:
>>>> Commit bdb5ac85777d ("PCI/ERR: Handle fatal error recovery")
>>>> merged fatal and non-fatal error recovery paths, and also made
>>>> recovery code depend on hotplug handler for "remove affected
>>>> device + rescan" support. But this change also complicated the
>>>> error recovery path and which in turn led to the following
>>>> issues.
>>>> 1. We depend on hotplug handler for removing the affected
>>>> devices/drivers on DLLSC LINK down event (on DPC event
>>>> trigger) and DPC handler for handling the error recovery. Since
>>>> both handlers operate on same set of affected devices, it leads
>>>> to race condition, which in turn leads to  NULL pointer
>>>> exceptions or error recovery failures.You can find more details
>>>> about this issue in following link.
>>>> 2. For non-hotplug capable devices fatal (DPC) error recovery
>>>> is currently broken. Current fatal error recovery implementation
>>>> relies on PCIe hotplug (pciehp) handler for detaching and
>>>> re-enumerating the affected devices/drivers. So when dealing with
>>>> non-hotplug capable devices, recovery code does not restore the state
>>>> of the affected devices correctly. You can find more details about
>>>> this issue in the following links.
>>>> In order to fix the above two issues, we should stop relying on hotplug
>>>     Yes, it doesn't rely on hotplug handler to remove and rescan the device,
>>> but it couldn't prevent hotplug drivers from doing another replicated
>>> removal/rescanning.
>>> it doesn't make sense to leave another useless removal/rescanning there.
>>> Maybe that's why these two paths were merged to one and made it rely on
>>> hotplug.
>> No, as per PCIe spec, hotplug and DPC has no functional dependency. Hence
>> depending on it to handle some of its recovery function is in-correct and
>> would lead to issues in non-hotplug capable platforms (which is true
>> currently).

>   pci_lock_rescan_remove() is global lock for PCIe, the mal-functional
>   device's port holds this lock, it prevents the whole system from doing
>   hot-plug operation.
It does not prevent the hotplug operation, but it might delay it. Since both
DPC and hotplug operates on same set of devices, it must be synchronized.
>   Though pciehp is not so hot/scalable and performance critical, but there
>   is per cpu thread to handle hot-plug operation. synchronize all threads
>   make them walk backwards for scalability.
DPC events does not happen in high frequency. So I don't think we should
worry about the performance here. Even hotplug handler will hold this lock
when adding/removing the devices. So adding/removing devices is a serialized

>>>> --
>>>> 2.17.1
>> --
>> Sathyanarayanan Kuppuswamy
>> Linux Kernel Developer

Sathyanarayanan Kuppuswamy
Linux Kernel Developer

  reply	other threads:[~2020-10-15  3:04 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-14  8:18 [PATCH v6 1/2] PCI/ERR: Call pci_bus_reset() before calling ->slot_reset() callback Kuppuswamy Sathyanarayanan
2020-10-14  8:18 ` [PATCH v6 2/2] PCI/ERR: Split the fatal and non-fatal error recovery handling Kuppuswamy Sathyanarayanan
2020-10-14 15:07   ` Ethan Zhao
2020-10-14 17:06     ` Kuppuswamy, Sathyanarayanan
2020-10-15  1:58       ` Ethan Zhao
2020-10-15  3:04         ` Kuppuswamy, Sathyanarayanan [this message]
2020-10-15  5:05           ` Ethan Zhao
2020-10-15  5:53             ` Kuppuswamy, Sathyanarayanan
2020-10-15 14:03               ` Ethan Zhao
2020-10-15  6:52   ` Christoph Hellwig
2020-10-15 13:55   ` Ethan Zhao
2020-10-15 20:12     ` 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:

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

  git send-email \ \ \ \ \ \ \ \ \ \

* 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).