Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 25 Sep 2001 20:36:09 +1000 (EST)
From:      Bruce Evans <bde@zeta.org.au>
To:        Wilko Bulte <wkb@freebie.xs4all.nl>
Cc:        John Baldwin <jhb@FreeBSD.ORG>, Garrett Wollman <wollman@khavrinen.lcs.mit.edu>, <current@FreeBSD.ORG>
Subject:   Re: Seen this lock order reversal?
Message-ID:  <20010925195231.A29474-100000@delplex.bde.org>
In-Reply-To: <20010924213556.A16239@freebie.xs4all.nl>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 24 Sep 2001, Wilko Bulte wrote:

> I did notice that the default Alpha beep is of a much higher frequency
> than the x86 one. Any relation? (long shot... I suppose)

This bug is well known (including by your mailbox).  From mail sent to
your mailbox:

% From bde@zeta.org.au Mon Jun 18 17:40:56 2001 +1000
% Date: Mon, 18 Jun 2001 17:40:51 +1000 (EST)
% From: Bruce Evans <bde@zeta.org.au>
% X-Sender: bde@besplex.bde.org
% To: Bill Fumerola <billf@mu.org>
% cc: wilko@FreeBSD.org, bde@FreeBSD.org
% Subject: Re: alpha/17637: misconfigured syscons bell causes panic on alpha
% In-Reply-To: <20010618001741.B2194@elvis.mu.org>
% Message-ID: <Pine.BSF.4.21.0106181725130.7056-100000@besplex.bde.org>
% MIME-Version: 1.0
% Content-Type: TEXT/PLAIN; charset=X-UNKNOWN
% Status: O
% X-Status:
% X-Keywords:
% X-UID: 2948
%
% On Mon, 18 Jun 2001, Bill Fumerola wrote:
%
% > On Sun, Jun 17, 2001 at 01:14:59PM -0700, wilko@FreeBSD.org wrote:
% >
% > > Problem was fixed long ago, so let us also close the PR..
% > >
% > > ----------------------------
% > > revision 1.15
% > > date: 2000/03/30 22:39:48;  author: billf;  state: Exp;  lines: +5 -3
% > > Avoid dividing by zero when beeping with a zero pitch. This was bad.
% > >
% > > PR:             alpha/17637
% > > Submitted by:   Bosko Milekic <bmilekic@dsuper.net>
% > > Reported by:    Dennis Lindroos <lindroos@nls.fi>
% >
% > As I recall, bde had issues with my fix. I don't remember them, though.
%
% From my saved mail (only the initial reply):
%
% | Patches in PRs are rarely suitable for committing verbatim, and this one
% | was no exception :-).  It has:
% |
% | 1) Style bugs on every line (2 "if (foo) bar;"'s and 2 gratuitous new
% |    empty lines).
% | 2) Internally inconsistent code.  The `pitch' arg is now loaded into the
% |    timer counter unchanged in the pitch == 0 case.
% | 3) Doesn't fix the real bug, which is that `pitch' isn't actually the
% |    pitch; it is a raw i8254 maximum count which must be loaded directly
% |    into the i8254 counter, as it is in the i386 version of sysbeep().
% |    Callers must convert from pitches in Hz to "pitch"es in i8254 counts,
% |    although this interface is Broken As Designed (AFAIK it is for bug
% |    for bug compatibility with a BAD SCO interface).  The conversion
% |    is impossible to do correctly for alphas, since the necessary
% |    conversion factor (timer_freq) is not exported from clock.c alphas.
% |    The necessary conversion factor is not exported from the kernel
% |    for any arch, so kbdcontrol(1) hard-codes it as 1193182.  Fortunately
% |    (?), this is almost correct for alphas.  Note that 1193182 is close
% |    to 10^6, so popular pitches near 1000 Hz have the magic property
% |    of not being changed much by the conversion.  This limits complaints
% |    about bizarre beep frequencies.  We use a default of BELL_PITCH =
% |    800; this becomes 1193182 / 800 = 1491 on alphas.
% |
% | Perhaps users shouldn't be allowed to program bizarre i8254 counts of 0
% | and >= 65536.  Counts >= 65536 are truncated modulo 65536.  At least the
% | KDMKTONE ioctl does the truncation silently.
% |
% | The sysbeep() interface also confuses:
% | dev/mlx/mlx.c: hard-coded i8254 count of 500.
% | i386/isa/pcvt/*: hard-coded timer_freq of 1193182 and strange differences
% |                  in default pitches 1493 and 1500.
% | i386/i386/trap.c: hard-coded timer_freq of TIMER_FREQ.
% | pccard/pccard_beep.c: hard-coded i8254 counts of 1600, 1200, 3200 are named
% |                       as pitches.
% <End of old mail>
%
% Even kbdcontrol.c is confused about this:
%
% | void
% | set_bell_values(char *opt)
% | {
% | 	int bell, duration, pitch;
% |
% | 	bell = 0;
% | 	if (!strncmp(opt, "quiet.", 6)) {
% | 		bell = 2;
% | 		opt += 6;
% | 	}
% | 	if (!strcmp(opt, "visual"))
% | 		bell |= 1;
% | 	else if (!strcmp(opt, "normal"))
% | 		duration = 5, pitch = 800;
% | 	else if (!strcmp(opt, "off"))
% | 		duration = 0, pitch = 0;
% | 	else {
% | 		char		*v1;
% |
% | 		bell = 0;
% | 		duration = strtol(opt, &v1, 0);
% | 		if ((duration < 0) || (*v1 != '.'))
% | 			goto badopt;
% | 		opt = ++v1;
% | 		pitch = strtol(opt, &v1, 0);
% | 		if ((pitch < 0) || (*opt == '\0') || (*v1 != '\0')) {
% | badopt:
% | 			warnx("argument to -b must be duration.pitch or [quiet.]visual|normal|off");
% | 			return;
% | 		}
%
% `pitch' is now actually in Hz.
%
% | 		if (pitch != 0)
% | 			pitch = 1193182 / pitch;	/* in Hz */
%
% Bogus comment.  `pitch' is NOT in Hz.  It is now a dimensionless number.
%
% | 		duration /= 10;	/* in 10 m sec */
% | 	}
% |
% | 	ioctl(0, CONS_BELLTYPE, &bell);
% | 	if ((bell & ~2) == 0)
% | 		fprintf(stderr, "[=%d;%dB", pitch, duration);
% | }
%
% Bruce
%
%
% From bde@zeta.org.au Tue Jun 19 16:43:28 2001 +1000
% Date: Tue, 19 Jun 2001 16:43:24 +1000 (EST)
% From: Bruce Evans <bde@zeta.org.au>
% X-Sender: bde@besplex.bde.org
% To: Wilko Bulte <wkb@freebie.xs4all.nl>
% cc: Bill Fumerola <billf@mu.org>, wilko@FreeBSD.ORG, bde@FreeBSD.ORG
% Subject: Re: alpha/17637: misconfigured syscons bell causes panic on alpha
% In-Reply-To: <20010618235810.C2135@freebie.xs4all.nl>
% Message-ID: <Pine.BSF.4.21.0106191629430.14720-100000@besplex.bde.org>
% MIME-Version: 1.0
% Content-Type: TEXT/PLAIN; charset=US-ASCII
% Status: O
% X-Status:
% X-Keywords:
% X-UID: 2957
%
% On Mon, 18 Jun 2001, Wilko Bulte wrote:
%
% > On Mon, Jun 18, 2001 at 05:40:51PM +1000, Bruce Evans wrote:
% > > On Mon, 18 Jun 2001, Bill Fumerola wrote:
% > >
% > > > On Sun, Jun 17, 2001 at 01:14:59PM -0700, wilko@FreeBSD.org wrote:
% > > >
% > > > > Problem was fixed long ago, so let us also close the PR..
% > > > >
% > > > > ----------------------------
% > > > > revision 1.15
% > > > > date: 2000/03/30 22:39:48;  author: billf;  state: Exp;  lines: +5 -3
% > > > > Avoid dividing by zero when beeping with a zero pitch. This was bad.
% > > > >
% > > > > PR:             alpha/17637
% > > > > Submitted by:   Bosko Milekic <bmilekic@dsuper.net>
% > > > > Reported by:    Dennis Lindroos <lindroos@nls.fi>
% > > >
% > > > As I recall, bde had issues with my fix. I don't remember them, though.
% > >
% > > >From my saved mail (only the initial reply):
% > >
% > > | Patches in PRs are rarely suitable for committing verbatim, and this one
% > > | was no exception :-).  It has:
% >
% > But it appears to be in the RELENG_4 source code more or less verbatim.
%
% Yes.  The bogus patch was MFC.  It was also MFa (to ia64) together with
% the broken alpha sysbeep() to give inverted pitches on ia64's too.
%
% > So closing the PR was still valid. Or?
%
% No.
%
% Bruce

Related (cosmetic) fixes for kbdcontrol(1):

diff -c2 kbdcontrol.c~ kbdcontrol.c
*** kbdcontrol.c~	Mon Jul 16 02:18:36 2001
--- kbdcontrol.c	Tue Aug 28 22:27:39 2001
***************
*** 882,886 ****
  set_bell_values(char *opt)
  {
! 	int bell, duration, pitch;

  	bell = 0;
--- 882,886 ----
  set_bell_values(char *opt)
  {
! 	int bell, duration, period, pitch;

  	bell = 0;
***************
*** 889,898 ****
  		opt += 6;
  	}
! 	if (!strcmp(opt, "visual"))
  		bell |= 1;
! 	else if (!strcmp(opt, "normal"))
! 		duration = 5, pitch = 800;
  	else if (!strcmp(opt, "off"))
! 		duration = 0, pitch = 0;
  	else {
  		char		*v1;
--- 889,903 ----
  		opt += 6;
  	}
! 	if (!strcmp(opt, "visual")) {
  		bell |= 1;
! 		duration = 0, period = 0;	/* XXX */
! 	} else if (!strcmp(opt, "normal"))
! 		/*
! 		 * XXX the historical normal "pitch" of 800 Hz is actually
! 		 * a period of 800 IBM-PC-i8254 cycles or 670 usec.
! 		 */
! 		duration = 5, period = 800;
  	else if (!strcmp(opt, "off"))
! 		duration = 0, period = 0;	/* XXX */
  	else {
  		char		*v1;
***************
*** 910,914 ****
  		}
  		if (pitch != 0)
! 			pitch = 1193182 / pitch;	/* in Hz */
  		duration /= 10;	/* in 10 m sec */
  	}
--- 915,931 ----
  		}
  		if (pitch != 0)
! 			/*
! 			 * XXX the boundary cases are broken.  The kernel
! 			 * silently truncates large periods modulo 0x10000
! 			 * (18.2 msec).  I'm not sure what a period of 0
! 			 * does.
! 			 */
! 			period = 1193182 / pitch;	/* in i8254 cycles */
! 		else
! 			/*
! 			 * A pitch of 0 should give a period of "infinity"
! 			 * (actually 65535).  But be bug for bug compatible.
! 			 */
! 			period = 0;
  		duration /= 10;	/* in 10 m sec */
  	}
***************
*** 916,920 ****
  	ioctl(0, CONS_BELLTYPE, &bell);
  	if ((bell & ~2) == 0)
! 		fprintf(stderr, "[=%d;%dB", pitch, duration);
  }

--- 933,937 ----
  	ioctl(0, CONS_BELLTYPE, &bell);
  	if ((bell & ~2) == 0)
! 		fprintf(stderr, "[=%d;%dB", period, duration);
  }

Some of the my original mail seems to be inverted.  My current understanding
is:
- the default pitch for sysbeep() is 800 <somethings> on both alphas and
  i386's.  The alpha sysbeep() bogusly (not bug for bug compatibly)
  inverts 800 (in Hz) to get a period (in i8254 timer cycles), so the
  default pitch is actually a pitch on alphas; it's only on i386's that
  800 (cylces) becomes 1491 (Hz).
- non-default pitches (set by kbdcontrol(1)) are converted to backwards-
  compatible (broken as designed) units in kbdcontrol(1), so the breakage
  is reversed: explicitly requesting 800 Hz gives 800 Hz on i386's and
  1491 Hz on alphas.
- I don't quite understand why you say that the default beep pitch is
  higher on alphas.  It's the non-default beep pitch that is higher.
- places that have pre-inverted "pitches" of about 1491 (cycles) (pcvt,
  pccard, etc.) have too-high beep frequencies on alphas.

Bruce


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010925195231.A29474-100000>