Re: bogus barriers in sym53c8xx_2?

Re: bogus barriers in sym53c8xx_2?

[Matthew Wilcox]

On Tue, Aug 19, 2003 at 04:49:53PM -0700, David Mosberger wrote:
> In drivers/scsi/sym53c8xx_2/sym_misc.h we find:
> 
> #elif	defined	__ia64__
> #define __READ_BARRIER()	__asm__ volatile("mf.a; mf" : : : "memory")
> #define __WRITE_BARRIER()	__asm__ volatile("mf.a; mf" : : : "memory")
> 
> based on the comments and the other implementations, these barriers
> are bogus and the "mf.a" should be dropped.
> 
> Anyone know who wrote this code originally and why the mf.a was added?
> 
> mf.a is very slow and should be avoided except were truly needed.

I'm sure Gerard must have written it originally.  It's there
in the earliest version of the sym2 driver I can find --
sym-2.1.16a-for-linux-2.4.13.patch.gz.  A similar barrier is there in
the sym1 driver (drivers/scsi/sym53c8xx_defs.h).  It seems to have been
introduced around 2.4.3 (symbios driver version 1.6b -> 1.7.3a-20010304)

So you're looking for a patch which looks something like this:

- #define __READ_BARRIER()	__asm__ volatile("mf.a; mf" : : : "memory")
- #define __WRITE_BARRIER()	__asm__ volatile("mf.a; mf" : : : "memory")
+ #define __READ_BARRIER()	__asm__ volatile("mf" : : : "memory")
+ #define __WRITE_BARRIER()	__asm__ volatile("mf" : : : "memory")

Or really, might be better to just define them to rmb() and wmb()?

-- 
"It's not Hollywood.  War is real, war is primarily not about defeat or
victory, it is about death.  I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk

Re: bogus barriers in sym53c8xx_2?

[Anton Blanchard]

> I'm sure Gerard must have written it originally.  It's there
> in the earliest version of the sym2 driver I can find --
> sym-2.1.16a-for-linux-2.4.13.patch.gz.  A similar barrier is there in
> the sym1 driver (drivers/scsi/sym53c8xx_defs.h).  It seems to have been
> introduced around 2.4.3 (symbios driver version 1.6b -> 1.7.3a-20010304)
> 
> So you're looking for a patch which looks something like this:
> 
> - #define __READ_BARRIER()	__asm__ volatile("mf.a; mf" : : : "memory")
> - #define __WRITE_BARRIER()	__asm__ volatile("mf.a; mf" : : : "memory")
> + #define __READ_BARRIER()	__asm__ volatile("mf" : : : "memory")
> + #define __WRITE_BARRIER()	__asm__ volatile("mf" : : : "memory")
> 
> Or really, might be better to just define them to rmb() and wmb()?

I suspect so, the powerpc ones are overkill too:

#define __READ_BARRIER()        __asm__ volatile("eieio; sync" : : : "memory")
#define __WRITE_BARRIER()       __asm__ volatile("eieio; sync" : : : "memory")

Anton

Re: bogus barriers in sym53c8xx_2?

[David Mosberger]

I didn't see any followup.  Gerard, do you have any comments?
I'm inclined to drop the #ifdef __ia64 part.  Also, I think
Matthew's suggestion to use rmb()/wmb() for the generic case
is a good one (though it makes no difference for ia64).

	--david

>>>>> On Wed, 20 Aug 2003 13:43:18 +1000, Anton Blanchard <anton@samba.org> said:

  >> I'm sure Gerard must have written it originally.  It's there in
  >> the earliest version of the sym2 driver I can find --
  >> sym-2.1.16a-for-linux-2.4.13.patch.gz.  A similar barrier is
  >> there in the sym1 driver (drivers/scsi/sym53c8xx_defs.h).  It
  >> seems to have been introduced around 2.4.3 (symbios driver
  >> version 1.6b -> 1.7.3a-20010304)

  >> So you're looking for a patch which looks something like this:

  >> - #define __READ_BARRIER() __asm__ volatile("mf.a; mf" : : :
  >> "memory") - #define __WRITE_BARRIER() __asm__ volatile("mf.a; mf"
  >> : : : "memory") + #define __READ_BARRIER() __asm__ volatile("mf"
  >> : : : "memory") + #define __WRITE_BARRIER() __asm__ volatile("mf"
  >> : : : "memory")

  >> Or really, might be better to just define them to rmb() and
  >> wmb()?

  Anton> I suspect so, the powerpc ones are overkill too:

  Anton> #define __READ_BARRIER() __asm__ volatile("eieio; sync" : : :
  Anton> "memory") #define __WRITE_BARRIER() __asm__ volatile("eieio;
  Anton> sync" : : : "memory")