Skip to contentSkip to conversation

bogus barriers in sym53c8xx_2?

DISCUSSION David Mosberger 3 participants 4 messages Aug 19, 2003 – Aug 21, 2003

lore
Share & tools
lore

David Mosberger

author
#0 lore raw

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.

	--david
#1 lore raw
Quoted 6 lines · David Mosberger

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")

Quoted 6 lines

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()?

Signature · 3 lines
"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
#2 lore raw
In reply to David Mosberger
Quoted 7 lines

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)

Quoted 8 lines

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

author
#3 lore raw
In reply to 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
Quoted 11 lines
Quoted 10 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line · 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:

Quoted 8 lines
Quoted 8 lines

- #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")