Messages in this thread |  | | | Subject | Re: readq/writeq on 32bit machines | | From | (Eric W. Biederman) | | Date | 18 May 2004 17:01:14 -0600 |
| |
Roland Dreier <roland@topspin.com> 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.
Eric - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |