From: Linus Torvalds <torvalds@linux-foundation.org>
To: "Luis R. Rodriguez" <mcgrof@gmail.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>,
linux-kernel <linux-kernel@vger.kernel.org>,
Jeff Garzik <jeff@garzik.org>,
"John W. Linville" <linville@tuxdriver.com>,
linux-wireless <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH] Clarify pci_iomap() usage for MMIO-only devices
Date: Tue, 18 Sep 2007 14:14:28 -0700 (PDT) [thread overview]
Message-ID: <alpine.LFD.0.999.0709181408390.16478@woody.linux-foundation.org> (raw)
In-Reply-To: <43e72e890709181402x7335d0acp306cf576fac6c070@mail.gmail.com>
On Tue, 18 Sep 2007, Luis R. Rodriguez wrote:
>
> ACK but do we really need to benchmark this if we *know* we are
> creating unnecessary branches? How about Jeff's suggestion of
> introducing pci_mmio_map() ?
Here's the counter-argument:
- the run-time expense of this is basically *zero*
- the expense of badly written drivers is absolutely *huge*
The fewer "clever" interfaces we have for no good reason, and the less
unnecessary stuff that really doesn't matter, the more we help the
*second* case.
The fact is, for device drivers, the *true* performance issues are nowhere
*near* the actual IO accessor functions, and are all at a much higher
level for all but a very very limited set of drivers. And I think you're
making the important issues *worse*.
Really. "pci_mmio_map()" may not be a bad interface, but you're simply
working on entirely the wrong problem. The problem is not an out-of-line
IO accessor function - the problem is that you're making it even more
complicated to write drivers.
The whole point of the "iomap()" interface was *never* about making the
driver interfaces faster. It was about making them *cleaner*. And you're
screwing that up!
The old situation with SATA drivers that had
if (iomem)
writel(..)
else
outl(..)
in the cases that needed it (and used hardcoded writel/outl in the cases
that didn't) was an example of code that "in theory" is faster. But
dammit, in practice that mattered not one whit, and what iomap() tries to
do is to attack the _real_ problem we had in that area.
Which had nothing what-so-ever to do with any branches.
Linus
next prev parent reply other threads:[~2007-09-18 21:15 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-09-17 20:22 [PATCH] Clarify pci_iomap() usage for MMIO-only devices Luis R. Rodriguez
2007-09-18 10:34 ` Alan Cox
2007-09-18 18:46 ` Luis R. Rodriguez
2007-09-18 19:03 ` Linus Torvalds
2007-09-18 19:07 ` Luis R. Rodriguez
2007-09-18 19:22 ` Linus Torvalds
2007-09-18 20:12 ` Luis R. Rodriguez
2007-09-18 20:30 ` Linus Torvalds
2007-09-18 21:02 ` Luis R. Rodriguez
2007-09-18 21:14 ` Linus Torvalds [this message]
2007-09-18 21:30 ` Jeff Garzik
2007-09-18 21:42 ` Linus Torvalds
2007-09-18 22:25 ` Linus Torvalds
2007-09-18 22:21 ` Benjamin Herrenschmidt
2007-09-18 22:36 ` Linus Torvalds
2007-09-18 20:21 ` Jeff Garzik
2007-09-18 22:20 ` Benjamin Herrenschmidt
2007-09-18 23:08 ` Jeff Garzik
2007-09-18 23:27 ` Linus Torvalds
2007-09-18 22:16 ` Benjamin Herrenschmidt
2007-09-18 22:25 ` Alan Cox
2007-09-19 15:07 ` Nick Kossifidis
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=alpine.LFD.0.999.0709181408390.16478@woody.linux-foundation.org \
--to=torvalds@linux-foundation.org \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=jeff@garzik.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=linville@tuxdriver.com \
--cc=mcgrof@gmail.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