Re: [PATCH] don't udelay() in sn_mmiob

[Jack Steiner <steiner@sgi.com>]

On Thu, May 27, 2004 at 05:06:31PM -0400, Jesse Barnes wrote:
> On Thursday, May 27, 2004 4:46 pm, David Mosberger wrote:
> > Wouldn't you want at least a cpu_relax() in that loop?
> 
> That'll cause the CPU to switch to the other thread if it's SMT so it won't 
> spin waiting for the access to complete?  If so, then yes.  Here's an updated 
> patch.

I didn't think the code would actually spin.

sn_mmiob() is something like:

    1:	ld.acq r8=...
    	;;
	cmp.eq p8,p0=r8,r9
   (p8)	br 1b

The "load" is an uncached load. Won't the pipeline stall on the cmp
waiting for data to arrive. I would not have expected that a cpu_relax()
would have been needed here. 

Data takes a few hundred nsec to be loaded from the chipset.


If we were spinning on a CACHED memory address, I agree that cpu_relax() 
is need.



> 
> sn_mmiob is a lightweight way to ensure PCI write ordering, intended to be 
> used as an alternative to doing a PIO read. Unfortunately, with the udelay() 
> in there, it ends up being slower than a PCI read on small configurations, so 
> remove it in favor of a simple cpu_relax() so that we're HT-friendly.
> 
> Thanks,
> Jesse



-- 
Thanks

Jack Steiner (steiner@sgi.com)          651-683-5302
Principal Engineer                      SGI - Silicon Graphics, Inc.