Date: Mon, 5 Jan 2004 08:13:14 -0800 (PST) From: Don Lewis <truckman@FreeBSD.org> To: silby@silby.com Cc: ryans@gamersimpact.com Subject: Re: 5.2-RC oerrs and collisions on dc0 Message-ID: <200401051613.i05GDE7E011556@gw.catspoiler.org> In-Reply-To: <53342.158.6.15.27.1073314708.squirrel@webmail.pair.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 5 Jan, silby@silby.com wrote:
>> I just took a closer look at the busdma diff, and this change to
>> dc_txeof() looks very suspicious:
>>
>> - if (!(cur_tx->dc_ctl & DC_TXCTL_LASTFRAG) ||
>> + if (!(cur_tx->dc_ctl & DC_TXCTL_FIRSTFRAG) ||
>> cur_tx->dc_ctl & DC_TXCTL_SETUP) {
>
> I'm current checking e-mail via a webmail interface and I haven't had time
> to check over your later posts, but I thought I'd note that the change
> above _is_ busdma related; one subtle change in the busdma code was that
> the mbuf is now linked to the first fragment in the chain, whereas before
> it was linked to the last fragment. So, the change does make sense on the
> surface, although I wouldn't be surprised if it broke something subtle.
Hmn, in that case skipping the "if" block if either DC_TXCTL_LASTFRAG is
set or if DC_TXCTL_FIRSTFRAG is set would be appropriate. Then we would
need to execute the code fragment:
if (txstat & DC_TXSTAT_ERRSUM) {
ifp->if_oerrors++;
if (txstat & DC_TXSTAT_EXCESSCOLL)
ifp->if_collisions++;
if (txstat & DC_TXSTAT_LATECOLL)
ifp->if_collisions++;
if (!(txstat & DC_TXSTAT_UNDERRUN)) {
dc_init(sc);
return;
}
}
ifp->if_collisions += (txstat & DC_TXSTAT_COLLCNT) >> 3;
if the DC_TXCTL_LASTFRAG is set and execute the code fragment
ifp->if_opackets++;
if (sc->dc_cdata.dc_tx_chain[idx] != NULL) {
m_freem(sc->dc_cdata.dc_tx_chain[idx]);
sc->dc_cdata.dc_tx_chain[idx] = NULL;
}
if the DC_TXCTL_FIRSTFRAGMENT flag is set.
I think this would introduce another subtle bug. I'm pretty sure that
the chip clears the DC_TXSTAT_OWN bit after it does the DMA for each
descriptor, so freeing the mbuf chain as soon as we see the
DC_TXSTAT_OWN bit go away on the first descriptor may free the mbuf
chain before the chip has copied the entire frame. I think it would be
better to revert the change to dc_txeof() and to hang the mbuf chain off
the last descriptor.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200401051613.i05GDE7E011556>