Message-ID: <49F843BC.7020902@garzik.org>
Date:	Wed, 29 Apr 2009 08:10:36 -0400
From:	Jeff Garzik <jeff@...zik.org>
To:	Ingo Molnar <mingo@...e.hu>
CC:	David Miller <davem@...emloft.net>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	rdreier@...co.com, hpa@...or.com, tglx@...utronix.de,
	h.mitake@...il.com, rpjday@...shcourse.ca,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86: Remove readq()/writeq() on 32-bit

Ingo Molnar wrote:
> (Linus Cc:-ed)
> 
> * David Miller <davem@...emloft.net> wrote:
> 
>> From: Roland Dreier <rdreier@...co.com>
>> Date: Tue, 28 Apr 2009 12:05:10 -0700
>>
>>> As discussed in <http://lkml.org/lkml/2009/4/19/164> and follow-ups,
>>> readq()/writeq() for 32-bit x86 are implemented as two readl()/writel()
>>> operations.  This is not atomic (in the sense that another MMIO
>>> operation from another CPU or thread can be done in the middle of the
>>> two read/writes), and may not access the two halves of the register in
>>> the correct order to work with hardware.
>>>
>>> Rather than silently providing a 32-bit fallback that leaves a
>>> possibility for strange driver bugs, it's better to provide readq()
>>> and writeq() only for 64-bit architectures, and have a compile failure
>>> on 32-bit architectures that forces driver authors to think about what
>>> the correct solution is.
>>>
>>> This essentially reverts 2c5643b1 ("x86: provide readq()/writeq() on
>>> 32-bit too") and follow-on commits.  If in the future someone wants to
>>> provide a generic solution for all 32-bit architectures, that's great,
>>> but there's not much point in providing (arguably broken)
>>> implementations for only one architecture, since any portable driver
>>> will have to implement fallbacks for other architectures anyway.
>>>
>>> Signed-off-by: Roland Dreier <rolandd@...co.com>
>> Acked-by: David S. Miller <davem@...emloft.net>
> [...]
>>> We never seemed to reach closure on this.  I would strongly 
>>> suggest merging something like this, and then if someone has a 
>>> grand plan to unify all fallbacks, we can add that when it shows 
>>> up.  As it stands, the x86-32 situation is not progress towards 
>>> that grand unified plans, and does nothing that I can tell 
>>> beyond setting a trap for drivers.
> 
> I still have no particularly strong opinion on this - other the 
> reluctance i expressed already in the previous threads. My arguments 
> are not reflected (and not addressed) in the changelog AFAICS, so 
> let me repeat them here:

I do.


> What caused 2c5643b1 was that right now we have ugly per driver
> #defines and inlines for readq/writeq. See:
> 
>   git grep 'define.*readq' drivers/
> 
> for a rough estimation of what the current practices are. The 32-bit 
> wrapper we added 6 months ago is the obvious implementation on x86 
> and that it matches existing wrappers.

This is the key...


> So my (slight) preference would be to keep the default generic 
> implementation and not make any atomicity guarantees - we never made 
> any.

Agreed.

This removal patch is completely pointless, because it moves us 
backwards to the point where we had a bunch of drivers defining it.

Why is that any better?

"Forcing driver writers to think" translates in the real world to each 
hardware vendor putting the common "#define readq" into their driver.

At least the networking drivers I messed with (until 11/2008) were 
always fine with a non-atomic readq.

The x86 kernel 32-bit implementation of readq/writeq is the code that 
every hardware vendor otherwise would re-create, when doing a Linux driver.

	Jeff



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists