From owner-freebsd-net@FreeBSD.ORG Wed Aug 21 18:23:35 2013 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 53A3C2A7; Wed, 21 Aug 2013 18:23:35 +0000 (UTC) (envelope-from nparhar@gmail.com) Received: from mail-pd0-x22c.google.com (mail-pd0-x22c.google.com [IPv6:2607:f8b0:400e:c02::22c]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 168242FBD; Wed, 21 Aug 2013 18:23:35 +0000 (UTC) Received: by mail-pd0-f172.google.com with SMTP id z10so765119pdj.3 for ; Wed, 21 Aug 2013 11:23:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding; bh=gd2Oz77EjH0D+tP2saus0g2LAd2FWuMApBPfye8MYaI=; b=pjyUbYujOwsGZXez6LvwRBUjp0YjTtVjvjn5y9xSheZi8WbiXLUMtfMMrizlHoVXNT ehePYcL7jIgW8yL9v3H4jLcjc2gF/yQH78BRsM3aHivXX3zrEvYis1MuM1MF/ZIfPppd 8eTvlIpaL2ZSmpKxc1mq03qqvsj5UsFgQONqRaglI2BiV8kOubf5u8tMUmRq/Y0BcOJ9 5n5NPUvJ3Fze0btNahR5iZZ0xRbv7mLVgXI12c3SWyWebE6/j7SvWOBE4dS2+mIigGzZ iRPDxvagG+C/NFE30SVOAi8CcCu7dbrz2e/xtZmQ1bm4SIGLXPXiHx/x32Z0taMEG+Ti uFXA== X-Received: by 10.68.186.193 with SMTP id fm1mr1052826pbc.163.1377109414700; Wed, 21 Aug 2013 11:23:34 -0700 (PDT) Received: from [10.192.166.0] (stargate.chelsio.com. [67.207.112.58]) by mx.google.com with ESMTPSA id bg3sm9742404pbb.44.1969.12.31.16.00.00 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 21 Aug 2013 11:23:33 -0700 (PDT) Sender: Navdeep Parhar Message-ID: <521505A3.4020103@FreeBSD.org> Date: Wed, 21 Aug 2013 11:23:31 -0700 From: Navdeep Parhar User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:17.0) Gecko/20130819 Thunderbird/17.0.8 MIME-Version: 1.0 To: Andre Oppermann Subject: Re: M_NOFREE removal (was Re: svn commit: r254520 - in head/sys: kern sys) References: <201308191116.r7JBGsc6065793@svn.freebsd.org> <521256CE.6070706@FreeBSD.org> <5212587A.2080202@freebsd.org> <52128937.1010407@freebsd.org> <52129E55.30803@freebsd.org> <5214D7E8.1080106@freebsd.org> <5214ED19.40907@FreeBSD.org> <5215047C.8080107@freebsd.org> In-Reply-To: <5215047C.8080107@freebsd.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: svn-src-head@freebsd.org, freebsd-net@freebsd.org, Peter Grehan X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 Aug 2013 18:23:35 -0000 On 08/21/13 11:18, Andre Oppermann wrote: > On 21.08.2013 18:38, Navdeep Parhar wrote: >> On 08/21/13 08:08, Andre Oppermann wrote: >>> On 20.08.2013 00:38, Peter Grehan wrote: >> >>> >>>> If there's an alternative to M_NOFREE, I'd be more than happy to use >>>> that. >>> >>> Set up your own (*ext_free) function and omit freeing of the mbuf >>> itself. Make >>> sure to properly track your mbufs to avoid leaking them. >>> >> >> How is this an alternate to M_NOFREE? Your suggestion indicates that >> you may have removed M_NOFREE thinking it did something other than what >> it actually did. And this suggestion doesn't even work -- note the >> uma_zfree(zone_mbuf, m) at the end of mb_free_ext that would run after >> any custom ext_free. >> >> To recap: M_NOFREE was the way to tell the mbuf subsystem that the mbuf >> does not come from zone_mbuf. Nothing more and nothing less. The mbuf >> was in other ways like any other mbuf and could have a pkthdr (or not), >> internal storage (or not), external cluster (or not), etc. etc. > > Mea culpa. You're totally right. I have mixed up my mental model with > another working tree I carry along for quite some time. In it (*ext_free) > completely replaces mb_free_ext() making it very easy to keep the mbuf. > We should move that way hopefully sooner than later, simplifying a couple > of things including externally managed refcounts. > > Sorry for the chaos and not getting it. > > M_NOFREE is back with r254605. > I saw that, thanks! I believe we need an extra patch to get M_NOFREE correct. I've had it forever in some of my internal repos but never committed it upstream (just plain forgot). Since this stuff is fresh in your mind, can you review this: diff -r cd78031b7885 sys/sys/mbuf.h --- a/sys/sys/mbuf.h Fri Aug 16 13:35:26 2013 -0700 +++ b/sys/sys/mbuf.h Wed Aug 21 10:55:57 2013 -0700 @@ -535,6 +535,8 @@ m_free(struct mbuf *m) { struct mbuf *n = m->m_next; + if ((m->m_flags & (M_PKTHDR|M_NOFREE)) == (M_PKTHDR|M_NOFREE)) + m_tag_delete_chain(m, NULL); if (m->m_flags & M_EXT) mb_free_ext(m); else if ((m->m_flags & M_NOFREE) == 0) It prevents leaks of tags from M_NOFREE+pkthdr mbufs. Regards, Navdeep