Re: readq/writeq on 32bit machines

From: Eric W. Biederman
Date: Tue May 18 2004 - 18:05:27 EST

Roland Dreier <roland@xxxxxxxxxxx> writes:

> Thanks for posting this Eric... I sent a less detailed reply yesterday
> (pointing out that atomic writeq is needed sometimes) but it seems to
> have gotten eaten.
> Eric> This issue came last night on the openib list. The driver
> Eric> currently rolls it's own version of writeq and in the case
> Eric> where there is not an atomic 64bit write it needs to a
> Eric> spinlock to make certain things don't get out of order. The
> Eric> driver fails with the current 2 writel() version.
> Eric> Here is an SSE version, that should not be to intrusive.
> Eric> According to intel's docs a 64bit aligned 64bit write is
> Eric> atomic all of the way back to the Pentium. If
> Eric> kernel_fpu_begin/kernel_fpu_end are safe in interrupt
> Eric> context we can do an atomic/correct version of writeq for
> Eric> x86 processors that don't support sse as well. Although I
> Eric> don't know if we want to.
> static inline void __raw_writeq(u64 val, unsigned long dest)
> {
> unsigned long cr0;
> u64 xmmsave __attribute__((aligned(8));
> preempt_disable();
> cr0 = read_cr0();
> clts();
> asm volatile (
> "movlps %%xmm0,(%0); \n\t"
> "movlps (%2),%%xmm0; \n\t"
> "movlps %%xmm0,(%1); \n\t"
> "movlps (%0),%%xmm0; \n\t"
> : =m (&xmmsave), "=m" ((void *)dest)
> : "m" (&val)
> );
> write_cr0(cr0);
> preempt_enable();
> }
> This is pretty much what I wrote in the above-mentioned openib
> driver. However I'm worried about using
> u64 xmmsave __attribute__((aligned(8));
> for a stack variable. I don't think gcc respects the alignment
> attribute for stack variables (I've had a problem in the past using
> movdqa to a stack variable, even if I do __attribute__((aligned(16))).
> If we're sure gcc aligns xmmsave properly, stick a comment in and
> leave out the __attribute__; if not then I think we have to do

I picked that up out of xor.h where the raid code does something similar,
so if there is a problem it needs to be fixed there as well.

> u8 xmmsave[8 + 7];
> and then use ~7 & (xmmsave + 7).
> Eric> Thinking about this a little more we might be able to get
> Eric> away with.
> static inline void __raw_writeq(u64 val, unsigned long dest)
> {
> unsigned long flags;
> local_irq_save(flags);
> writel(val & 0xffffffff, addr);
> writel(val >> 32, addr + 4);
> irq_restore(flags);
> }
> I don't think this is good enough on SMP. In the openib case, it's
> entirely possible for one CPU to be ringing a (64-bit) work queue
> doorbell at the same time as another CPU is ringing a (64-bit)
> completion queue doorbell, and if the 32-bit halves of those doorbells
> get interleaved, the hardware gets confused. Maybe there's some magic
> aspect of the PC hardware that ensures this can't happen but I'd hate
> to count on it without some very good documentation.

Right. It does make the window incredibly small though. I am even
nervous that the version with a spinlock might break, if something really
needs an atomic guarantee.

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at