From owner-cvs-all  Mon Jul  5 21:53:47 1999
Delivered-To: cvs-all@freebsd.org
Received: from implode.root.com (root.com [209.102.106.178])
	by hub.freebsd.org (Postfix) with ESMTP
	id C8AC214EF5; Mon,  5 Jul 1999 21:53:42 -0700 (PDT)
	(envelope-from root@implode.root.com)
Received: from implode.root.com (localhost [127.0.0.1])
	by implode.root.com (8.8.8/8.8.5) with ESMTP id VAA09190;
	Mon, 5 Jul 1999 21:53:19 -0700 (PDT)
Message-Id: <199907060453.VAA09190@implode.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 
In-reply-to: Your message of "Mon, 05 Jul 1999 19:06:43 PDT."
             <19990706020643.B10D214C94@hub.freebsd.org> 
From: David Greenman <dg@root.com>
Reply-To: dg@root.com
Date: Mon, 05 Jul 1999 21:53:19 -0700
Sender: owner-cvs-all@FreeBSD.ORG
Precedence: bulk

>> >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