From owner-freebsd-net@FreeBSD.ORG Sat Sep 2 07:53:15 2006 Return-Path: X-Original-To: freebsd-net@freebsd.org Delivered-To: freebsd-net@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 59EB616A51F for ; Sat, 2 Sep 2006 07:53:15 +0000 (UTC) (envelope-from andre@freebsd.org) Received: from c00l3r.networx.ch (c00l3r.networx.ch [62.48.2.2]) by mx1.FreeBSD.org (Postfix) with ESMTP id 9971F43D46 for ; Sat, 2 Sep 2006 07:53:14 +0000 (GMT) (envelope-from andre@freebsd.org) Received: (qmail 93960 invoked from network); 2 Sep 2006 07:39:02 -0000 Received: from c00l3r.networx.ch (HELO [127.0.0.1]) ([62.48.2.2]) (envelope-sender ) by c00l3r.networx.ch (qmail-ldap-1.03) with SMTP for ; 2 Sep 2006 07:39:02 -0000 Message-ID: <44F9386A.30804@freebsd.org> Date: Sat, 02 Sep 2006 09:53:14 +0200 From: Andre Oppermann User-Agent: Thunderbird 1.5.0.5 (Windows/20060719) MIME-Version: 1.0 To: Maxim Konovalov References: <44F35A65.3080605@cisco.com> <20060828224452.GK37035@funkthat.com> <44F45A2A.8030405@freebsd.org> <20060902081043.J32527@mp2.macomnet.net> In-Reply-To: <20060902081043.J32527@mp2.macomnet.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: freebsd-net@freebsd.org, Randall Stewart Subject: Re: Problem with uipc_mbuf.c X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 02 Sep 2006 07:53:15 -0000 Maxim Konovalov wrote: > On Tue, 29 Aug 2006, 17:15+0200, Andre Oppermann wrote: >> John-Mark Gurney wrote: >>> Randall Stewart wrote this message on Mon, Aug 28, 2006 at 17:04 -0400: >>>> atomic_fetchadd_int(m->m_ext.ref_cnt, -1) == 0) { >>> ^ >>> >>> This should be 1 not 0.. as apparently fetchadd_int returns the >>> old value (at least that's what atomic(9) says), which means that >>> if we ever race on this comparision, we won't free though we >>> should of... >>> >>> if we look at refcount.h, it does: >>> return (atomic_fetchadd_int(count, -1) == 1); >>> >>> which release a reference and apparently returns true if it needs to >>> be free'd... >>> >>> Though the wierd part is that andre, "fixed" it to be 0 in 1.157: >>> Fix a logic error introduced with mandatory mbuf cluster >>> refcounting and freeing of mbufs+clusters back to the packet zone. >> Honestly I'm a bit confused myself now and have to dig up things from >> when I did the change. However I'm certain there was a problem and the >> commit fixed it in some way (not necessarily the correct way). Before >> the 'fix' there were some larger leaks going on. > > So what's the conclusion? Perhaps it's worth to add an XXX comment in > meantime. Please give me until Thursday to resolve this issue. -- Andre