From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Doug Anderson <dianders@chromium.org>
Cc: "Peter Hurley" <peter@hurleysoftware.com>,
california.l.sullivan@intel.com, "Liakhovetski,
Guennadi" <guennadi.liakhovetski@intel.com>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
jslaby@suse.com, "Brian Norris" <briannorris@chromium.org>,
"open list:ARM/Rockchip SoC..."
<linux-rockchip@lists.infradead.org>,
陈渐飞 <jeffy.chen@rock-chips.com>, 高美才 <eric.gao@rock-chips.com>,
phillip.raffeck@fau.de, anton.wuerfel@fau.de,
yegorslists@googlemail.com, matwey@sai.msu.ru,
linux-serial@vger.kernel.org,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] serial: 8250: Avoid "too much work" from bogus rx timeout interrupt
Date: Mon, 19 Dec 2016 22:18:08 +0200 [thread overview]
Message-ID: <1482178688.9552.104.camel@linux.intel.com> (raw)
In-Reply-To: <CAD=FV=VbaNSsAoL8=FW-nEJS=538hRNND3JHaA=QjF9kg75e=A@mail.gmail.com>
On Mon, 2016-12-19 at 09:54 -0800, Doug Anderson wrote:
> Hi,
>
> Yes. Almost all Intel HW is using DesignWare IP for HS UARTs.
>
> OK, so possibly we could add this workaround in just the DesignWare
> code and then we could be more sure we're not breaking other UARTs?
> That would work for me. It seems like it would be easier to validate
> that there are no unintended side effects if we put this just in the
> DesignWare driver.
Yes, don't need to touch others.
> Yes, I could believe that in the DMA case that my patch might not be
> the right thing to do. I can easily just add a check for "!up->dma"
> if it makes the patch better.
At least, yes.
> > > 1. We'll get the interrupt
> > > 2. We won't do _anything_ to service the interrupt.
> > > 3. We'll return back to serial8250_interrupt(), where we'll keep
> > > looping until we get "too much work"
> > > 4. We'll break out, but the interrupt will still be active.
> > > 5. Go back to #1
> > >
> > > ...and since this interrupt will keep firing and firing and firing
> > > with no delay in-between, we'll effectively lock the CPU up.
> >
> > And the root cause of that is... ?
>
> I don't understand your question. Basically what I'm saying is that
> we got an interrupt and did absolutely nothing to handle it or clear
> it. Then we returned "handled". Is it a mystery that the interrupt
> will fire again and again and again?
> Specifically:
> * reading the LSR doesn't clear the interrupt
> * The DR / BI bits aren't set.
> * serial8250_modem_status() won't clear the interrupt (reads the MSR)
> * nothing to transmit
> * we'll return "handled" since the only time serial8250_handle_irq()
> returns 0 is if we have UART_IIR_NO_INT.
My question here a bit rhetorical, we better understand root cause,
better fix would be.
> > What I think is that the root cause of this is still unknown and
> > either
> > above looks like a hack.
>
> I postulated a root cause of receiving a partial character, but I'd
> need to figure out how to twiddle bits in just the right way to
> somehow try to do this in a programmatic way. I can certainly
> reproduce this in a black-box sort of way by just doing suspend/resume
> testing long enough.
Have you tried to disable C-states or set PM QoS?
Do you have same issue with and without DMA?
> Even if the root cause isn't know, though, it seems like the current
> behavior of locking up a CPU is non-ideal. It seems like there ought
> to be some sort of way to detect and handle this case.
Have you read links I sent? In one mail I mentioned Intel's
documentation that suggests not to use RDI interrupt when DMA. Which
sounds weird.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
next prev parent reply other threads:[~2016-12-19 20:18 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-19 1:14 [PATCH] serial: 8250: Avoid "too much work" from bogus rx timeout interrupt Douglas Anderson
2016-12-19 12:59 ` Andy Shevchenko
2016-12-19 17:12 ` Doug Anderson
2016-12-19 17:33 ` Andy Shevchenko
2016-12-19 17:35 ` Andy Shevchenko
2016-12-19 17:54 ` Doug Anderson
2016-12-19 20:18 ` Andy Shevchenko [this message]
2016-12-19 21:13 ` Doug Anderson
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=1482178688.9552.104.camel@linux.intel.com \
--to=andriy.shevchenko@linux.intel.com \
--cc=anton.wuerfel@fau.de \
--cc=briannorris@chromium.org \
--cc=california.l.sullivan@intel.com \
--cc=dianders@chromium.org \
--cc=eric.gao@rock-chips.com \
--cc=gregkh@linuxfoundation.org \
--cc=guennadi.liakhovetski@intel.com \
--cc=jeffy.chen@rock-chips.com \
--cc=jslaby@suse.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=linux-serial@vger.kernel.org \
--cc=matwey@sai.msu.ru \
--cc=peter@hurleysoftware.com \
--cc=phillip.raffeck@fau.de \
--cc=yegorslists@googlemail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.