From: Mika Westerberg <mika.westerberg@linux.intel.com>
To: Lukas Wunner <lukas@wunner.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>,
Keith Busch <keith.busch@intel.com>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Frederick Lawler <fred@fredlawl.com>,
"Gustavo A . R . Silva" <gustavo@embeddedor.com>,
Sinan Kaya <okaya@kernel.org>,
Kai-Heng Feng <kai.heng.feng@canonical.com>,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] PCI: pciehp: Prevent deadlock on disconnect
Date: Mon, 23 Sep 2019 11:12:37 +0300 [thread overview]
Message-ID: <20190923081237.GB2773@lahna.fi.intel.com> (raw)
In-Reply-To: <20190923053403.jdjw6ed3sub6iuou@wunner.de>
Hi Lukas,
On Mon, Sep 23, 2019 at 07:34:03AM +0200, Lukas Wunner wrote:
> On Mon, Aug 12, 2019 at 05:31:33PM +0300, Mika Westerberg wrote:
> > If there are more than one PCIe switch with hotplug downstream ports
> > hot-removing them leads to a following deadlock:
>
> For the record, I think my comments on v1 of this patch still apply:
>
> https://patchwork.ozlabs.org/patch/1117870/#2230798
Well, so do I ;-)
As I tried to explain in v1 discussion, I think what we do here in this
patch is correct thing to do regardless. I mean once the hardware is
gone the driver should not do any decisions based on what it thinks it
reads from the now missing hardware. This also makes the deadlock
problem go away on all the system I've been testing. Where previously I
was able to reproduce the deadlock 100% reliably I have not seen it
happen once with this one applied (and haven't got reports from our
internal testing either).
Regarding suggestion of unbinding PCI drivers without
pci_lock_rescan_remove() hold, I haven't looked it too closely but I
think we need to take that lock anyway because when we are unbinding a
hotplug driver it is supposed to remove the hierarchy below touching the
shared structures, possibly concurrently. Unfortunately there is no
documentation what data pci_lock_rescan_remove() actually protects so
first one needs to understand that. I think one way to clean up this is
to use finer grained locking (with documented lock ordering) for PCI bus
structures that can be accessed simultaneusly by different threads. But
that is not a simple task.
next prev parent reply other threads:[~2019-09-23 8:12 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-12 14:31 [PATCH v2 1/2] PCI: pciehp: Do not disable interrupt twice on suspend Mika Westerberg
2019-08-12 14:31 ` [PATCH v2 2/2] PCI: pciehp: Prevent deadlock on disconnect Mika Westerberg
2019-08-19 2:28 ` Sinan Kaya
2019-08-19 8:56 ` Mika Westerberg
2019-08-19 12:28 ` Sinan Kaya
2019-09-23 5:34 ` Lukas Wunner
2019-09-23 8:12 ` Mika Westerberg [this message]
2019-09-23 8:28 ` Lukas Wunner
2019-09-23 8:28 ` Mika Westerberg
2019-10-18 7:10 ` Kai-Heng Feng
2019-10-22 23:00 ` Bjorn Helgaas
2019-10-23 7:52 ` Mika Westerberg
2019-10-24 9:38 ` Mika Westerberg
2019-10-24 17:11 ` Bjorn Helgaas
2019-09-19 6:42 ` [PATCH v2 1/2] PCI: pciehp: Do not disable interrupt twice on suspend Mika Westerberg
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=20190923081237.GB2773@lahna.fi.intel.com \
--to=mika.westerberg@linux.intel.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=bhelgaas@google.com \
--cc=fred@fredlawl.com \
--cc=gustavo@embeddedor.com \
--cc=kai.heng.feng@canonical.com \
--cc=keith.busch@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lukas@wunner.de \
--cc=okaya@kernel.org \
--cc=rjw@rjwysocki.net \
/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).