From: Rajat Jain <rajatxjain@gmail.com>
To: Bjorn Helgaas <bhelgaas@google.com>
Cc: Rajat Jain <rajatjain@juniper.net>,
Rajat Jain <rajatjain.linux@gmail.com>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
linux hotplug mailing <linux-hotplug@vger.kernel.org>,
Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>,
Yijing Wang <wangyijing@huawei.com>,
Greg KH <gregkh@linuxfoundation.org>,
Tom Nguyen <tom.l.nguyen@intel.com>,
Kristen Accardi <kristen.c.accardi@intel.com>,
Guenter Roeck <groeck@juniper.net>
Subject: Re: [PATCH] pciehp: Acknowledge the spurious "cmd completed" event.
Date: Fri, 21 Feb 2014 01:43:40 +0000 [thread overview]
Message-ID: <CAA93t1qib_07MZad=87t0cpOnLFaVKKf6F_-2CE=+Z3+dzBXcg@mail.gmail.com> (raw)
In-Reply-To: <CAErSpo4n19oYLzzkwQ_QeYq9v48DQPzLsMgCoihdT7OdsXGqtg@mail.gmail.com>
On Thu, Feb 20, 2014 at 2:20 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Feb 20, 2014 at 12:42 AM, Rajat Jain <rajatxjain@gmail.com> wrote:
>> Hello Bjorn,
>>
>>>>
>>>> On Mon, Nov 25, 2013 at 07:03:11PM +0000, Rajat Jain wrote:
>>>> > Hello,
>>>> >
>>>> > > > On a different note, I feel there is still a need to apply my
>>>> > > > original
>>>> > > patch. There is still an open problem in case of spurious interrupts
>>>> > > (or in any case where the condition "if (slot_status &
>>>> PCI_EXP_SLTSTA_CC)"
>>>> > > becomes true in pcie_write_cmd()). That is because once that
>>>> > > happens, we never clear that interrupt, and no further hotplug
>>>> > > interrupts shall be received unless we do that.
>>>> > >
>>>> > > I agree this is an issue and we should address it somehow. My
>>>> > > hesitation is just that I'd prefer to do some more aggressive
>>>> > > restructuring rather than apply a point fix. For example:
>>>> >
>>>> > OK, I'll attempt to fix it that way when I get time.
>>>> >
>>>> > >
>>>> > > - We currently look at PCI_EXP_SLTSTA_CC in pcie_isr(),
>>>> > > pcie_poll_cmd(), and pcie_write_cmd(). I think it would be better
>>>> > > to look at it only in pcie_isr().
>>>> > >
>>>> > > - I don't think pcie_poll_cmd() should exist at all; we should poll
>>>> > > by calling pcie_isr() instead.
>>>> > >
>>>> > > - We need pcie_write_cmd(), but I think the way it waits is
>>>> backwards.
>>>> > > Currently we issue the command, then wait for it to complete. I
>>>> > > think we should issue the command, note the current time, and return
>>>> > > without waiting. The *next* time we need to issue a command, we can
>>>> > > wait for completion of the previous one (or timeout) if necessary.
>>>> > >
>>>> > > But maybe we need the point fix in the interim, especially if
>>>> > > anybody can actually produce the scenario you mention.
>>>> >
>>>> > Ok.
>>>>
>>>> This patch is still in patchwork, but I've lost track of where we are.
>>>> Did you resolve this in the series that I just applied, or is it still
>>>> an outstanding issue?
>>>
>>> No, I did not solve it. It is still an outstanding issue. So far I am using your patch to overcome this:
>>>
>>> http://www.spinics.net/lists/hotplug/msg05830.html
>>>
>>> I'll just attempt to conclude the status on this issue so that you can make the decision on the course of action. IMHO there are 2 independent issues that we discussed in this thread:
>>>
>>> 1) PCIe compliant HW (that generates cmd completed interrupts at every write of Slot_ctrl register) being penalized with 1 second delay during the boot up. Your patch solves this.
>>>
>>> 2) If there is a genuine spurious interrupt, it does not get acknowledged. I had originally posted a patch for THIS problem.
>>> http://www.spinics.net/lists/hotplug/msg05815.html
>>>
>>> You had indicated that you would rather want a bigger restructuring of the driver to solve (2).
>>>
>>> My observation: MY problem (in my setup) is not seen if I use either of the patches (yours or mine).
>>>
>>> My opinion: I think my patch solves (2) but might not solve (1) for all corner cases. Also your patch solves (1) but may not solve (2) for all corner cases -Thus we should probably solve both of these problems individually.
>>>
>>
>> Just wondering if you decided on how to solve this problem?
>>
>> Are you planning this for 3.15?
>
> Sorry, I haven't had a chance to work on this, so I don't think *I*
> will get anything done for v3.15. To make forward progress, maybe we
> should merge your original patch? Would you mind posting it again so
> it gets into patchwork again?
Just sent it again, Thanks!
prev parent reply other threads:[~2014-02-21 1:43 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-05 22:33 [PATCH] pciehp: Acknowledge the spurious "cmd completed" event Rajat Jain
2013-11-06 0:25 ` Bjorn Helgaas
2013-11-06 2:38 ` Rajat Jain
2013-11-07 21:53 ` Rajat Jain
2013-11-08 1:23 ` Bjorn Helgaas
2013-11-08 17:30 ` Rajat Jain
2013-11-08 23:15 ` Bjorn Helgaas
2013-11-11 21:26 ` Rajat Jain
2013-11-23 0:51 ` Bjorn Helgaas
2013-11-23 1:59 ` Guenter Roeck
2013-11-23 14:56 ` Rajat Jain
2013-11-23 19:32 ` Bjorn Helgaas
2013-11-25 19:03 ` Rajat Jain
2014-02-12 0:34 ` Bjorn Helgaas
2014-02-12 1:08 ` Rajat Jain
2014-02-20 7:42 ` Rajat Jain
2014-02-20 22:20 ` Bjorn Helgaas
2014-02-21 1:43 ` Rajat Jain [this message]
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='CAA93t1qib_07MZad=87t0cpOnLFaVKKf6F_-2CE=+Z3+dzBXcg@mail.gmail.com' \
--to=rajatxjain@gmail.com \
--cc=bhelgaas@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=groeck@juniper.net \
--cc=kaneshige.kenji@jp.fujitsu.com \
--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=tom.l.nguyen@intel.com \
--cc=wangyijing@huawei.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).