From owner-freebsd-net@FreeBSD.ORG Tue Aug 29 16:08: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 7875F16A4DF; Tue, 29 Aug 2006 16:08:15 +0000 (UTC) (envelope-from jmg@hydrogen.funkthat.com) Received: from hydrogen.funkthat.com (gate.funkthat.com [69.17.45.168]) by mx1.FreeBSD.org (Postfix) with ESMTP id EF71543D55; Tue, 29 Aug 2006 16:08:14 +0000 (GMT) (envelope-from jmg@hydrogen.funkthat.com) Received: from hydrogen.funkthat.com (x2sgmxjm4tfgpyjb@localhost.funkthat.com [127.0.0.1]) by hydrogen.funkthat.com (8.13.6/8.13.3) with ESMTP id k7TG8EHs034246; Tue, 29 Aug 2006 09:08:14 -0700 (PDT) (envelope-from jmg@hydrogen.funkthat.com) Received: (from jmg@localhost) by hydrogen.funkthat.com (8.13.6/8.13.3/Submit) id k7TG8DgY034245; Tue, 29 Aug 2006 09:08:13 -0700 (PDT) (envelope-from jmg) Date: Tue, 29 Aug 2006 09:08:13 -0700 From: John-Mark Gurney To: Randall Stewart Message-ID: <20060829160813.GL37035@funkthat.com> Mail-Followup-To: Randall Stewart , freebsd-net@freebsd.org, andre@freebsd.org References: <44F35A65.3080605@cisco.com> <20060828224452.GK37035@funkthat.com> <44F44CAA.2000203@cisco.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <44F44CAA.2000203@cisco.com> User-Agent: Mutt/1.4.2.1i X-Operating-System: FreeBSD 5.4-RELEASE-p6 i386 X-PGP-Fingerprint: B7 EC EF F8 AE ED A7 31 96 7A 22 B3 D8 56 36 F4 X-Files: The truth is out there X-URL: http://resnet.uoregon.edu/~gurney_j/ X-Resume: http://resnet.uoregon.edu/~gurney_j/resume.html Cc: freebsd-net@freebsd.org, andre@freebsd.org Subject: Re: Problem with uipc_mbuf.c X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: John-Mark Gurney List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 29 Aug 2006 16:08:15 -0000 Randall Stewart wrote this message on Tue, Aug 29, 2006 at 10:18 -0400: > Ok, I confirmed it... > > Changing it to be > > atomic_fetchadd_int(m->m_ext.ref_cnt, -1) == 1) > > Fixes the problem.. no more leaks :-D And if you are curious XADD is: The XADD (exchange and add) instruction swaps two operands and then stores the sum of the two operands in the destination operand. The status flags in the EFLAGS register indicate the result of the addition. This instruction can be combined with the LOCK prefix (see “LOCK—Assert LOCK# Signal Prefix” in Chapter 3, “Instruction Set Reference, A-M” of the IA-32 Intel® Architecture Software Developer’s Manual, Volume 2A) in a multiprocessing system to allow multiple processors to execute one DO loop. >From IA-32 Intel® Architecture Software Developer’s Manual Volume 1: Basic Architecture It's useful to keep a copy of them around for times like these... > 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. > > > > > >>I am thinking about restoring the old code.. since > >>it appears to work... > >> > >>Any comments or help would be appreciated.. > > > > > >Lets see what andre has to say about this. -- John-Mark Gurney Voice: +1 415 225 5579 "All that I will do, has been done, All that I have, has not."