Date: Mon, 05 Jul 1999 21:53:19 -0700 From: David Greenman <dg@root.com> To: wpaul@FreeBSD.ORG (Bill Paul) Cc: committers@FreeBSD.ORG, cvs-all@FreeBSD.ORG Subject: Re: cvs commit: src/sys/pci if_ti.c if_tireg.h Message-ID: <199907060453.VAA09190@implode.root.com> In-Reply-To: Your message of "Mon, 05 Jul 1999 19:06:43 PDT." <19990706020643.B10D214C94@hub.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
>> >wpaul 1999/07/05 13:19:43 PDT >> > >> > Modified files: >> > sys/pci if_ti.c if_tireg.h >> > Log: >> > Remove ti_refill_rx_rings() and associated stuff; replace dirty RX buffers >> > in ti_rxeof() instead. This doesn't really seem to provide much in the >> > way of a performance boost, and I'm pretty sure it can cause mbuf leakage >> > in some extreme cases. >> >> Just to clarify - the old method may have mbuf leakage problems, or do mean >> that the new method has problems? > >action: Bill stares blankly at dg. > >The name of the game is to fix things. I would never in a million years >commit code to deliberately leak mbufs. Yes, well, that's why I asked. If one follows the logic in the above commit message, one may easily come to the wrong conclusion about what "this" refers to. It normally refers to your action, not the target of your action. > ti_refill_rx_rings() could sometimes >leak mbufs, and while testing with the SysKonnect card I decided that it >doesn't really help all that much anyway, so I killed it. > >The problem with ti_refill_rx_rings() is that in some cases, ti_rxeof() >will leave the current receive buffer in place instead of consuming it >(e.g. if the received frame was bad). ti_refill_rx_rings() didn't check >this condition and would clobber the mbuf pointer with a pointer to >a new mbuf in its place. > >It also doesn't allow for recovery in the face of allocation failures. >If we can't allocate a new buffer, we should leave the old one in place >and tank the packet. But by the time we get to refill routine, we may >have had to toss several packets and we can no longer fill in the >empty descriptors. Thanks for the clarification. -DG David Greenman Co-founder/Principal Architect, The FreeBSD Project - http://www.freebsd.org Creator of high-performance Internet servers - http://www.terasolutions.com To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199907060453.VAA09190>