From: ebiederm@xmission.com (Eric W. Biederman)
To: Rajat Jain <rajatjain@juniper.net>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
"linux-hotplug@vger.kernel.org" <linux-hotplug@vger.kernel.org>,
Greg KH <gregkh@linuxfoundation.org>,
Guenter Roeck <groeck@juniper.net>,
"tom.l.nguyen@intel.com" <tom.l.nguyen@intel.com>,
"kristen.c.accardi@intel.com" <kristen.c.accardi@intel.com>,
"rajesh.shah@intel.com" <rajesh.shah@intel.com>,
"rajatjain.linux@gmail.com" <rajatjain.linux@gmail.com>
Subject: Re: Enhancing pciehp
Date: Sat, 19 Oct 2013 01:22:37 +0000 [thread overview]
Message-ID: <87eh7ikqaq.fsf@xmission.com> (raw)
In-Reply-To: <c713940b329e4c6584e173cb49aaa043@BLUPR05MB118.namprd05.prod.outlook.com> (Rajat Jain's message of "Sat, 19 Oct 2013 01:07:23 +0000")
Rajat Jain <rajatjain@juniper.net> writes:
> Hello,
>
> I'm working on enhancing the pciehp driver to allow link state changes to drive the hot-plug and un-plug. The idea was discussed here:
>
> http://www.spinics.net/lists/linux-pci/msg25733.html
>
> However, this is to invite ideas on how to handle a particular corner case I see. Let's say a PCIe link comes up and the pciehp driver calls pciehp_enable_slot() to start scanning and adding the devices on the slot. However, what to do if before that completes, the link goes down due to some reason? Can the pciehp driver call pciehp_disable_slot() before the call to pciehp_enable_slot() returns (with or without error)?
>
> Essentially even without link state to driver the hot-plug, this problem can also be thought of as a problem that exists today. For e.g. for the slots that support surprise removal, the pciehp will initiate hot-plug and unplug based on the presence detection (as soon as card is inserted or removed). Now if a user puts in a card very momentarily, long enough for the hot-add process to get started (pciehp_enable_slot()) but removes it before it can be completed, the same problem can be envisioned. As the code exists today, I believe it would call pciehp_disable_slot() even before pciehp_enable_slot() returns. Is that the designed / correct behavior?
>
> Should I be following the same for link state change driven hot-plug?
The way I handled this problem is made the enable and disable all run
through the same single threaded work queue.
The code does need to be single threaded on a per hotplug slot basis
(aka enable and disable concurrently on the same slot is nonsense).
I think there is some locking in the device tree that you might be able
to depend on to help single thread this. At the time I was working on
this I did not see any locking or anything else that would serialize the
access so I went for the very cheap solution of an interrupt handler
that queued work in a single threaded work queue.
The code to be correct and robuse must handle the case of where what it
thought was going on add/remove is time late information and is actually
incorrect at the time of the hotplug scan.
This all gets even more interesting when you are plugging in a tree of
hotplug devices with hotplug slots of their own.
From an implementation point of view I have observed devices in the wild
with imperfect PCIe busses that due to bus noise would occassionally
flap their presence bit for no particularly good reason. It is a
hardware but so you don't need to handle it beyond not letting that kind
of activity crash the kernel but it does happen.
Eric
next prev parent reply other threads:[~2013-10-19 1:22 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-07 18:58 Enhancing pciehp (was: pcielw An alternate pcie hotplug driver) Rajat Jain
2013-10-07 20:31 ` Bjorn Helgaas
2013-10-19 1:07 ` Rajat Jain
2013-10-19 1:22 ` Eric W. Biederman [this message]
2013-10-07 22:42 ` Enhancing pciehp Eric W. Biederman
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=87eh7ikqaq.fsf@xmission.com \
--to=ebiederm@xmission.com \
--cc=bhelgaas@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=groeck@juniper.net \
--cc=kristen.c.accardi@intel.com \
--cc=linux-hotplug@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=rajatjain.linux@gmail.com \
--cc=rajatjain@juniper.net \
--cc=rajesh.shah@intel.com \
--cc=tom.l.nguyen@intel.com \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).