[This e-mail includes full details of a bug + a suggested patch, I'm
not on the list myself so if you want to contact me please feel
free to e-mail me. Note: the patch is at the very end of this
text.... search for START CUT to jump there now.]
Sorry if you folks already know this, but I've otherwise not managed
to find out any reference to this bug on the net anywhere, so I
though I better tell you folks....
In the latest stable kernel release, 2.0.35, there seems to be
a new bug in the console beeping code -- introduced by the new
fix in the console beep code! Ooops!!!!!
The problem is, the new semaphore mechanism has a small mistake which
prevents the console beep sound being turned off by KIOCSOUND if it
was initiated with KIOCSOUND (ticks of 0). This does not effect the
normal 'ASCII 7 bell' console beep (which calls the functions
designed to beep only for a set period -- using a timer to call the
routine to stop the sound), only sound initiated and stoped with
KIOCSOUND.
For an example, if you have "minicom", try firing that up and
connecting to something. The final "I'm connected" 'beep' just
stays on indefinitely! Oooooow!!!! (You can shut it up by
making a console beep.)
(If you don't have minicom and are desperate for a simple example,
e-mail me and I'll forward some example 'C' code you can try.)
Whoooops.
*8-)
The bug is a terribly simple one, and a simple one to correct
as well.
The following is the routine, _kd_mksound in vt.c that KIOCSOUND
calls (with a ticks of 0?).
It begins by a test & set of a semaphore to ensure that there's only
ever one invocation of this routine running at any one time.
Consider the case where it is called with a frequency of 0, which is
used to turn a currently sounding beep off (if previously turned on
with KIOCSOUND). It attempts to turn the sound off by calling
another routine, kd_nosound. *BUT* in this particular circumstance
the call to kd_nosound will *not* turn the sound off...
...cos it itself checks to see if that 'kd_mksound is running'
semaphore is set. Which of course it will be, always, during the
course of kd_mksound!
void
_kd_mksound(unsigned int hz, unsigned int ticks)
{
static struct timer_list sound_timer = { NULL, NULL, 0, 0,
kd_nosound };
unsigned int count = 0;
if (hz > 20 && hz < 32767)
count = 1193180 / hz;
/* ignore multiple simultaneous requests for sound */
if (!set_bit(0, &mksound_lock)) {
***** ^^^^^^^^^^^^^^^^^^^^^^^^^^
***** Semaphore test and test
/* set_bit in 2.0.x is same as test-and-set in 2.1.x */
del_timer(&sound_timer);
if (count) {
/* enable counter 2 */
outb_p(inb_p(0x61)|3, 0x61);
/* set command for counter 2, 2 byte write */
outb_p(0xB6, 0x43);
/* select desired HZ */
outb_p(count & 0xff, 0x42);
outb((count >> 8) & 0xff, 0x42);
if (ticks) {
sound_timer.expires = jiffies+ticks;
add_timer(&sound_timer);
}
} else
kd_nosound(0);
***** ^^^^^^^^^^^^^
***** If frequency is 0, call kd_nosound in
***** an attempt to stop the current beep
*****
***** *PROBLEM* kd_nosound checks the same semaphore
***** which is set above and not cleared until later in
***** this routine - kd_nosound won't stop the sound if
***** it is set ... which of course it will always be
***** at this point! Ooops.
mksound_lock = 0;
***** ^^^^^^^^^^^^^^^^
***** Clear semaphore
}
return;
}
This is the routine used to stop the beep. Note that
before stoping the sound, it checks the semaphore -- the
semaphore used to make sure the above routine is not
presently running.
static void
kd_nosound(unsigned long ignored)
{
/* if sound is being set up, don't turn it off */
if (!mksound_lock)
/* disable counter 2 */
outb(inb_p(0x61)&0xFC, 0x61);
return;
}
The fix??!?!?!?!
Well, in kd_mksound, you can't just clear the semaphore before
calling kd_nosound, as that risks a situation where one task starts a
sound right after the semaphore is cleared and before kd_nosound is
executed, and then kd_nosound will stop the sound immediately.
I don't think we can remove the semaphore check in kd_nosound either,
as that routine can/will be called when the sound timer expires after
setting up a timed beep. As such, we need to be sure that the sound
timer expiring won't prematurely end the sound just as a new sound is
being started up.
What I suggest is to change kd_mksound so that instead of calling
kd_nosound(0) when when frequency = 0 (i.e. when count = 0, as per
the else statement in kd_mksound) to instead just use the instruction
from kd_nosound that stops the sound. We don't have to worry about
that semaphore check at this point, as that is already done at the
start of kd_mksound, and the semaphore is not cleared until the end
of kd_mksound, so we're already guaranteed exclusive access at this
point anyway.
[Sorry if I've not made much sense.... the suggested patch
probably speaks louder and clearer than my words... *8-) ]
Here is my suggested patch-
####################### START CUT ############################
--- linux/drivers/char/vt.c.orig Mon Jul 13 21:47:30 1998
+++ linux/drivers/char/vt.c Fri Jul 24 00:25:19 1998
@@ -190,7 +190,8 @@
add_timer(&sound_timer);
}
} else
- kd_nosound(0);
+ /* disable counter 2 */
+ outb(inb_p(0x61)&0xFC, 0x61);
mksound_lock = 0;
}
####################### END CUT ############################
Caveat: It does seem that there's another potential problem where the
speaker can perhaps be left on indefinitely... but only on the rare
occasion where the system was running incredibly sluggish and
the length of a beep set very very short...
...what happens if a set-length beep is requested, and the sound
timer runs out before the code reaches the part that clears the
semaphore - the beep will never be turned off. I've no idea whether
this situation is realistic or not though? If so, and assuming I'm
guessing what cli() & sti() do correctly (unlikely) - perhaps we
could still do with an invocation of cli() just before 'add_timer' &
sti() just before return in kd_mksound (noting that cli() & sti()
were used in 2.0.34 in this routine but dropped in 2.0.35 -
presumably as the author now assumed that blocking interrupts was now
a no longer necessary hack)?
Thanks for reading this... and I really hope I've reported this bug
correctly and not bothered anyone. *8-X My *sincere* appologies if
I've messed up (I'm a linux-kernel-bug-report virgin...)!!!!
Take care y'all!!!
Ta-ra,
Julie (julie@merp.demon.co.uk)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.altern.org/andrebalsa/doc/lkml-faq.html