-----Original Message-----
From: Julie Brandon <julie@merp.demon.co.uk>
To: linux-kernel@vger.rutgers.edu <linux-kernel@vger.rutgers.edu>
Date: Thursday, July 23, 1998 7:16 PM
Subject: Bug in 2.0.35; vt.c (bad problem with console beep semaphore code)
>Hiyas!
>
>[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
>
-
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