netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stanislaw Gruszka <sgruszka@redhat.com>
To: David Miller <davem@davemloft.net>
Cc: Larry.Finger@lwfinger.net, linville@tuxdriver.com,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Ivo van Doorn <IvDoorn@gmail.com>,
	Gertjan van Wingerde <gwingerde@gmail.com>,
	Helmut Schaa <helmut.schaa@googlemail.com>,
	hedayaty@gmail.com
Subject: Re: pull request: wireless 2011-11-22
Date: Wed, 23 Nov 2011 09:03:54 +0100	[thread overview]
Message-ID: <20111123080353.GA3752@redhat.com> (raw)
In-Reply-To: <20111122.164057.1411098663938026383.davem@davemloft.net>

On Tue, Nov 22, 2011 at 04:40:57PM -0500, David Miller wrote:
> >>> It just means that we won't notice spurious interrupts if the device
> >>> sharing the line with rt2800pci generates one.
> >>>
> >>> This change is wrong.

Ehh, I had a feeling that I was doing something wrong ... 

> >> BTW, look at it this way, if what you say is true John then what's the
> >> point
> >> in returning any specific value at all?
> >>
> >> Everyone can just return IRQ_HANDLED and everything would just work.
> >>
> >> But you know that's not the case, and that it's important that this
> >> value
> >> is returned accurately.
> > 
> > I was trying to find the thread that reported the improvement in
> > performance with this change, but failed. Is it possible that their
> > change just papered over an interrupt storm from some other device
> > that shared that interrupt?
> 
> It doesn't fix a performance problem, it fixes a problem wherein the
> IRQ line is disabled by the generic IRQ code because all handlers
> return IRQ_NONE.

According to Amir, who reports this problem patch fix the performance
(https://bugzilla.redhat.com/show_bug.cgi?id=658451),
from very bad to expected on wifi link. It can be explained, if we
disable interrupt line, driver still operate, but read TX/RX statuses
from hardware when internal driver watchdog timeout. Hence we operate,
but very slow. If we prevent to disable interrupt by IRQ controller
driver, device operate normally.

The main problem here, that we have hardware (or firmware) that
generate interrupt with empty interrupt status register, so we can
not detect if interrupt is really generated by Ralink device.

Perhaps exist good fix for that problem, i.e. we can write to some
register to stop hardware generating spurious interrupts, or exist
other way than reading status register to find out if Ralink device
generated interrupt. But that require detailed hardware knowledge,
and I'm not sure if we ever can get such informations. Also this 
could be a hardware bug, device just generate spurious interrupts
and we can not do anything about it. I looked at driver from Ralink
site, and it do exactly that, it return IRQ_HANDLED if status register
is empty.

Ok, I'm thinking about other fix now ...

Stanislaw

  reply	other threads:[~2011-11-23  8:01 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-22 19:35 pull request: wireless 2011-11-22 John W. Linville
2011-11-22 20:14 ` David Miller
2011-11-22 20:56   ` John W. Linville
2011-11-22 21:05     ` David Miller
2011-11-22 21:13       ` David Miller
2011-11-22 21:26         ` John W. Linville
2011-11-22 21:30         ` Larry Finger
2011-11-22 21:40           ` David Miller
2011-11-23  8:03             ` Stanislaw Gruszka [this message]
2011-11-23  8:57               ` David Miller
2011-11-22 21:44           ` John W. Linville
2011-11-22 21:56 ` pull request: wireless 2011-11-22 #2 John W. Linville
2011-11-22 22:37   ` David Miller
2011-11-22 22:41     ` John W. Linville
2011-11-22 23:17       ` David Miller

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=20111123080353.GA3752@redhat.com \
    --to=sgruszka@redhat.com \
    --cc=IvDoorn@gmail.com \
    --cc=Larry.Finger@lwfinger.net \
    --cc=davem@davemloft.net \
    --cc=gwingerde@gmail.com \
    --cc=hedayaty@gmail.com \
    --cc=helmut.schaa@googlemail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.com \
    --cc=netdev@vger.kernel.org \
    /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).