Re: MMIO and gcc re-ordering issue

2008-06-01 Thread Pavel Machek
Hi!

   Though it's my understanding that at least ia64 does require the
   explicit barriers anyway, so we are still in a dodgy situation here
   where it's not clear what drivers should do and we end up with
   possibly excessive barriers on powerpc where I end up with both
   the wmb/rmb/mb that were added for ia64 -and- the ones I have in
   readl/writel to make them look synchronous... Not nice.
 
 ia64 is a disaster with a slightly different ordering problem -- the
 mmiowb() issue.  I know Ben knows far too much about this, but for big
 SGI boxes, you sometimes need mmiowb() to avoid problems with driver
 code that does totally sane stuff like
 
   spin_lock(mmio_lock);
   writel(val1, reg1);
   writel(val2, reg2);
   spin_unlock(mmio_lock);
 
 If that snippet is called on two CPUs at the same time, then the device
 might see a sequence like
 
   CPU1 -- write reg1
   CPU2 -- write reg1
   CPU1 -- write reg2
   CPU2 -- write reg2
 
 in spite of the fact that everything is totally ordered on the CPUs by
 the spin lock.
 
 The reason this is such a disaster is because the code looks right,
 makes sense, and works fine on 99.99% of all systems out there.  So I
 would bet that 99% of our drivers have missing mmiowb() bugs -- no one
 has plugged the hardware into an Altix box and cared enough to stress
 test it.

Yes, ia64 is a disaster, and needs its spinlock implementation fixed
:-).
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: MMIO and gcc re-ordering issue

2008-06-01 Thread Pavel Machek
Hi!

 The only way to guarantee ordering in the above setup, 
 is to either
 make writel() fully ordered or adding the mmiowb()'s 
 inbetween the two
 writel's. On Altix you have to go and read from the 
 PCI brige to
 ensure all writes to it have been flushed, which is 
 also what mmiowb()
 is doing. If writel() was to guarantee this ordering, 
 it would make
 every writel() call extremely expensive :-(
 
 Interesting. I've always been taught by ia64 people 
 that mmiowb() was
 intended to be used solely between writel() and 
 spin_unlock().
 
 I think in the above case, you really should make 
 writel() ordered.
 Anything else is asking for trouble, for the exact same 
 reasons that I
 made it fully ordered on powerpc at least vs. previous 
 stores. I only
 kept it relaxed vs. subsequent cacheable stores (ie, 
 spin_unlock), for
 which I use the trick mentioned before.
 
 Hmmm I hope I didn't mess up the description of this and 
 added to the
 confusion.
 
 The net result of that would be to kill performance 
 completely, I really
 don't like that idea Having each writel() go out and 
 poll the PCI
 bridge is going to make every driver used on Altix slow 
 as a dog.
 
 In addition it's still not going to solve the problem 
 for userland
 mapped stuff such as infinibug.
 
 Yes, this has some cost (can be fairly significant on 
 powerpc too) but
 I think it's a very basic assumption from drivers that 
 consecutive
 writel's, especially issued by the same CPU, will get 
 to the device
 in order.
 
 In this case the cost is more than just significant, 
 it's pretty
 crippling.
 
 If this is a performance problem, then provide relaxed 
 variants and
 use them in selected drivers.
 
 We'd have to make major changes to drivers like e1000, 
 tg3, mptsas, the
 qla2/3/4xxx and a bunch of others to get performance 
 back. I really
 think the code maintenance issue there will get far 
 worse than what we
 have today :(

Still better than changing semantics of writel for _all_ drivers.

If you are really sure driver does not depend on writel order, it is
just a sed/// command, so I don't see any big code maintenance
issues...
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 1/4] [POWERPC] 85xx: add board support for the TQM8548 modules

2008-06-01 Thread David Gibson
On Fri, May 30, 2008 at 08:49:45AM +0200, Wolfgang Grandegger wrote:
 This patch adds support for the TQM8548 modules from TQ-Components
 GmbH (http://www.tqc.de).

[snip]
 index 000..d09250a
 --- /dev/null
 +++ b/arch/powerpc/boot/dts/tqm8548.dts
 @@ -0,0 +1,370 @@
 +/*
 + * TQM8548 Device Tree Source
 + *
 + * Copyright 2006 Freescale Semiconductor Inc.
 + * Copyright 2008 Wolfgang Grandegger [EMAIL PROTECTED]
 + *
 + * This program is free software; you can redistribute  it and/or modify it
 + * under  the terms of  the GNU General  Public License as published by the
 + * Free Software Foundation;  either version 2 of the  License, or (at your
 + * option) any later version.
 + */
 +
 +/dts-v1/;

[snip]
 + [EMAIL PROTECTED] {
 + #address-cells = 1;
 + #size-cells = 0;
 + cell-index = 0;
[...]
 + enet0: [EMAIL PROTECTED] {
 + cell-index = 0;
[...]
 + serial0: [EMAIL PROTECTED] {
 + cell-index = 0;
[...]
 + pci0: [EMAIL PROTECTED] {
 + cell-index = 0;

You have a whole lot of 'cell-index' properties through both these
trees, and they all look wrong.  cell-index is a hack, which should be
avoided wherever practical - it should only be used when the index is
used to offset into some global register block, never simply to
differentiate (use reg for that) or name the devices (use aliases for
that).

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ftrace: powerpc clean ups

2008-06-01 Thread Michael Ellerman
On Thu, 2008-05-22 at 14:31 -0400, Steven Rostedt wrote:
 This patch cleans up the ftrace code in PowerPC based on the comments from
 Michael Ellerman.

Hi Steven,

Thanks for that.

I posted some patches last week, also in my git tree[1], that should
allow you to use create_branch() in your code (and also facilitate some
other things I wanted to do). I also added a #define for NOP also.

If/when my patches go into powerpc-next I'll do a patch against
linux-next for ftrace to use them.

cheers

[1]: http://git.kernel.org/?p=linux/kernel/git/mpe/linux-2.6.git;a=summary

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person


signature.asc
Description: This is a digitally signed message part
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev