linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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

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