Re: 2.6.6-mm5

[ebiederm@xmission.com (Eric W. Biederman)]

Andrew Morton <akpm@osdl.org> writes:

> http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.6/2.6.6-mm5/
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.6/2.6.6-mm5/
> 
> 
> add-i386-readq.patch
>   add i386 readq()/writeq()

> static inline u64 readq(void *addr)
> {
> 	return readl(addr) | (((u64)readl(addr + 4)) << 32);
> }
> 
> static inline void writeq(u64 v, void *addr)
> {
> 	u32 v32;
> 
> 	v32 = v;
> 	writel(v32, addr);
> 	v32 = v >> 32;
> 	writel(v32, addr + 4);
> }
> 
> #endif

The implementation is broken and it will break drivers that actually
expect writeq and readq to be 64bit reads and writes.

In particular an interrupt can come in during the middle of a read
or a write and widely separate the two different halves.

Unless the driver has a lock protecting access to the card already
this race could be nasty and quite difficult to debug.  And I have
looked and we do have some cases in the kernel that do not have
a lock protecting them.

I attempted to suggest some alternative implementations earlier
in the original thread that brought this up but it looks like
you missed that.

Eric