I've been investigating a bug in 2.1.55 whereby my # of queued commands
(as reported by /proc/scsi/aha152x/0 becomes -1, and my drive light
stays on continuously. I've found the problem, and I'll be submitting
a patch as soon as I clean things up, but I came across the following
code in drivers/scsi/aha152x.c, starting at line 1274:
sti(); /* Hi Eric, guess what ;-) */
/* sleep until the abortion is complete */
while(!HOSTDATA(shpnt)->abortion_complete)
barrier();
HOSTDATA(shpnt)->aborting=0;
I started to get worried with the while() loop. Here we have a busy loop
checking a member of a structure (abortion_complete), until the interrupt
handler changes that member to nonzero. Aside from the problem that this
might violate the non-reentrancy assumption in scsi.c (in scsi_abort(),
that's the cleanup I have to do for my patch), I'm worried about the while
loop ever completing: none of these pointers or structures are declared
volatile. In fact, the 'volatile' keyword never appears in aha152x.c or
aha152x.h.
It happens that gcc generates the right assembly (on my 586):
#APP
sti
#NO_APP
cmpl $0,96(%esi)
jne .L1751
.align 4
.L1752:
#APP
#NO_APP
cmpl $0,96(%esi) <= Always rereads from memory
je .L1752
.L1751:
movl $0,92(%esi)
Now I realize that gcc is reading this from memory simply because of
the definition of barrier():
#define barrier() __asm__("": : :"memory")
A haven't found any cases in the scsi code where reliance on barrier()
causes the wrong code to be generated, but I can't help but be nervous
about it. It seems excessively fragile to me.
I wonder how many other places in the kernel are written the same way?
- --
Jon Paul Nollmann ne' Darren Senn sinster@darkwater.com
POBox 64132, Sunnyvale, CA, USA 94088-4132
Unsolicited commercial email will be archived at $1/byte/day.
Wasurenaide -- doko e itte mo soko ni iru yo.
-----BEGIN PGP SIGNATURE-----
Version: 2.6.2
iQCVAwUBNCcu3MyyNwVEgVztAQF/rwP7B+GzLr6SsIHccD/c6I3hgNIeaP/CVEDz
GmA1BbpmLoO9C1bIgRsH0fCax/NLE2CQEJBAsT0raqjihZJO3ovBZVlxkIZ7qfqq
j8jfInKqNK9u4TeA02f1EDNkVz567IuLZTehc+y2KWYNFOmUYZXu/R5NhvKRlfBU
rAV21UY8T2k=
=Qu+q
-----END PGP SIGNATURE-----