From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Segher Boessenkool <segher@kernel.crashing.org>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>,
Alan Cox <alan@lxorguk.ukuu.org.uk>,
David Miller <davem@davemloft.net>,
jeff@garzik.org, paulus@samba.org, torvalds@osdl.org,
linux-kernel@vger.kernel.org, akpm@osdl.org
Subject: Re: Opinion on ordering of writel vs. stores to RAM
Date: Mon, 11 Sep 2006 11:35:54 +1000 [thread overview]
Message-ID: <1157938555.31071.300.camel@localhost.localdomain> (raw)
In-Reply-To: <2486D031-097B-45C6-AC47-D8745844C5A3@kernel.crashing.org>
> What exactly will "weak" mean? If it's weak enough to please all
> architectures and busses, it'll be so weak that you'll need 2**N
> (with a big N) different barriers.
Which is why I proposed a precise semantic: weak MMIO vs. main meory but
strong between MMIOs on the same CPU. Which, in PowerPC language
translates into no barrier in __writel and eieio,load,twi,isync on
__readl (maybe even a second eieio, I have think about it)
> > - flags argument to ioremap
>
> ioremap is a bad name anyway, if we'll change the API, change the
> name as well (and it's a bad idea to keep the same name but make it
> mean something different, anyway).
We already have a new API with a nicer name, pci_iomap, to use with the
new "ioread/iowrite" accessors that Linus defined a while ago. Problem:
- It's a unifed PIO/MMIO interface, thus there is some added overhead
to calls compared to a simple MMIO-only writel/readl. Though your
proposal would add a similar overhead to writel/readl, so maybe we can
consolidate all of these ...
- It doesn't have a flags arguemnt. But then, there are few enough
users that this can be easily fixed (at least more easily than ioremap)
- It has few users :) Thus lots of drivers will need heavy conversion.
But that's a good point to remember about the existence of this "other"
MMIO/PIO access API. Because adding a whole lot of __* version of those
calls in addition to the MMIO only readX/writeX will be a mess and tend
to favor your option of just having a flag passed at map time for the
sake of simplicity.
> > Oh, and all MMIO accessors are *documented* with strongly defined
> > semantics. :)
>
> Not sure what this means? Document them in all-caps? :-)
>
> > If we go this route though, can I request that we don't introduce any
> > performance regressions in drivers currently using mmiowb()? I.e.
> > they'll be converted over to the new accessor routines when they
> > become
> > available along with the new barrier macros?
>
> In my proposal at least, those drivers won't lose any performance
> (except for a conditional on their I/O cookie, which is a trivial
> performance loss compared to the cost of the I/O /an sich/); they
> won't need any changes either, except for some renaming. Drivers
> _not_ using it might get a performance loss though, because they
> will be forced to run with every-I/O-ordered semantics; they will
> suddenly start to work *correctly* though.
As I wrote in my "vote" email, I have no strong preference at this
point, though I must admit that the fact that your proposal makes the
driver conversion trivial and ends up, imho, easier for driver writers
(and possibly more flexible for archs) makes it appealing. The "only"
problem is the possibly added overhead of a test in readl/writel. I
don't know wether that's relevant enough to justify not going your way
though. Only platforms like ia64 and powerpc will need that test (thus
no performance regression on x86), it might be worth a bit of
benchmarking here to see wether the added test has any real impact on
performances.
Note that nothing prevents us from adding _both_ proposals. It may sound
a bit weird but at the end of the day, if writel/readl are defined as
being strongly ordered, as per Option A, adding a test to make them
optionally -not- strongly ordered based on a cooke in the address won't
hurt much.
In which case, we end up with those 3 cases:
- slow case: normal ioremap, readl/writel strongly ordered
- faster case : "special" ioremap, readl/writel faster
- even faster case : __readl/__writel (save a test from the above)
I mean that having both options at once is possible. Wether it's
something we want to do is a different matter as it might be considered
as adding confusion.
Ben.
next prev parent reply other threads:[~2006-09-11 1:36 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-09-09 2:03 Paul Mackerras
2006-09-09 2:42 ` Linus Torvalds
2006-09-09 3:02 ` Paul Mackerras
2006-09-09 3:54 ` Linus Torvalds
2006-09-09 7:24 ` Benjamin Herrenschmidt
2006-09-09 9:34 ` David Miller
2006-09-09 9:55 ` Jeff Garzik
2006-09-09 10:08 ` David Miller
2006-09-10 17:18 ` Jesse Barnes
2006-09-10 19:35 ` Alan Cox
2006-09-10 21:25 ` Benjamin Herrenschmidt
2006-09-10 22:23 ` Alan Cox
2006-09-10 22:18 ` Benjamin Herrenschmidt
2006-09-11 13:19 ` Jes Sorensen
2006-09-10 23:35 ` Segher Boessenkool
2006-09-11 0:12 ` Benjamin Herrenschmidt
2006-09-11 0:34 ` Jesse Barnes
2006-09-11 1:04 ` Benjamin Herrenschmidt
2006-09-11 1:13 ` Segher Boessenkool
2006-09-11 1:35 ` Benjamin Herrenschmidt [this message]
2006-09-11 9:02 ` Alan Cox
2006-09-11 9:23 ` Benjamin Herrenschmidt
2006-09-11 0:25 ` Jesse Barnes
2006-09-11 0:54 ` Segher Boessenkool
2006-09-11 1:10 ` Benjamin Herrenschmidt
2006-09-11 1:48 ` Segher Boessenkool
2006-09-11 3:53 ` Benjamin Herrenschmidt
2006-09-11 18:12 ` Jesse Barnes
2006-09-11 1:00 ` Benjamin Herrenschmidt
2006-09-11 18:08 ` Jesse Barnes
2006-09-11 21:32 ` Benjamin Herrenschmidt
2006-09-10 20:01 ` Segher Boessenkool
2006-09-11 13:21 ` David Miller
2006-09-11 14:17 ` Segher Boessenkool
2006-09-12 0:32 ` David Miller
2006-09-12 0:49 ` Benjamin Herrenschmidt
2006-09-12 16:47 ` Segher Boessenkool
2006-09-12 0:54 ` Roland Dreier
2006-09-09 11:16 ` Paul Mackerras
2006-09-09 7:23 ` Benjamin Herrenschmidt
2006-09-09 9:38 ` David Miller
2006-09-09 15:09 ` Alan Cox
2006-09-10 17:19 ` Jesse Barnes
2006-09-10 17:35 ` Michael Buesch
2006-09-10 17:49 ` Linus Torvalds
2006-09-10 18:02 ` Michael Buesch
2006-09-09 15:08 ` Alan Cox
2006-09-09 18:34 ` Auke Kok
2006-09-09 19:10 ` Patrick McFarland
2006-09-09 15:06 ` Alan Cox
2006-09-11 5:03 Michael Chan
2006-09-11 5:21 ` Benjamin Herrenschmidt
2006-09-12 4:30 Albert Cahalan
2006-09-12 5:30 ` Benjamin Herrenschmidt
2006-09-12 6:04 ` Albert Cahalan
2006-09-12 6:12 ` Benjamin Herrenschmidt
2006-09-12 7:09 ` Albert Cahalan
2006-09-12 7:17 ` Benjamin Herrenschmidt
2006-09-12 7:21 ` Albert Cahalan
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=1157938555.31071.300.camel@localhost.localdomain \
--to=benh@kernel.crashing.org \
--cc=akpm@osdl.org \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=davem@davemloft.net \
--cc=jbarnes@virtuousgeek.org \
--cc=jeff@garzik.org \
--cc=linux-kernel@vger.kernel.org \
--cc=paulus@samba.org \
--cc=segher@kernel.crashing.org \
--cc=torvalds@osdl.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
Powered by JetHome