Date: Mon, 26 Jul 2004 17:15:24 -0500 (CDT) From: "Conrad J. Sabatier" <conrads@cox.net> To: Don Lewis <truckman@FreeBSD.org> Cc: freebsd-current@FreeBSD.org Subject: Re: Questionable code in sys/dev/sound/pcm/channel.c Message-ID: <XFMail.20040726171524.conrads@cox.net> In-Reply-To: <200407262155.i6QLtNuZ058373@gw.catspoiler.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 26-Jul-2004 Don Lewis wrote:
> On 26 Jul, Conrad J. Sabatier wrote:
>> I'm a little perplexed at the following bit of logic in chn_write()
>> (which is where the "interrupt timeout, channel dead" messages are
>> being generated).
>>
>> Within an else branch within the main while loop, we have:
>>
>> else {
>> timeout = (hz * sndbuf_getblksz(bs)) /
>> (sndbuf_getspd(bs) * sndbuf_getbps(bs));
>> if (timeout < 1)
>> timeout = 1;
>> timeout = 1;
>>
>> Why the formulaic calculation of timeout, if it's simply going to be
>> unconditionally set to 1 immediately afterwards anyway? What's
>> going on
>> here?
>
> Hmn, looks bogus to me. I think the intention is to round timeout up
> to 1 if the result of the formula is zero. The final assignment
> statement looks bogus to me. Maybe a too short timeout is the
> source of this problem.
>
> It looks like this assignment appeared in rev 1.65.
Hmm, your guess is as good as (or probably better than) mine. :-)
A little more in the way of comments certainly wouldn't hurt.
>> Also, at the end of the function:
>>
>> if (count <= 0) {
>> c->flags |= CHN_F_DEAD;
>> printf("%s: play interrupt timeout, channel dead\n",
>> c->name);
>> }
>>
>> return ret;
>> }
>>
>> Could it be that the conditional test is wrong here? Perhaps
>> we should be using (count < 0) instead?
>>
>> I don't know. I'm having no small difficulty understanding this
>> code, but these two items caught my attention.
>
> I ran into the same problem when I was looking at the code a few days
> ago.
>
> BTW, the trace output that was posted showed write() returning 0
> immediately before the failure occurred.
Are you referring to the truss output I posted a few days ago? The
thing of it is, though, that the original "channel dead" message had
already occurred in a previous run of madplay (which wasn't traced), so
it's really hard to say if there's any useful info to be obtained from
tracing a later run, after the pcm device was already "broken".
So far, I still haven't gotten the error with the new kernel I'm
testing. I wouldn't say absolutely that that single patch (of the
final conditional test) is "the fix", but it may help in the meantime.
--
Conrad J. Sabatier <conrads@cox.net> -- "In Unix veritas"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.20040726171524.conrads>