From owner-freebsd-net@FreeBSD.ORG Mon Sep 9 07:44:21 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 DC3A0A9E for ; Mon, 9 Sep 2013 07:44:20 +0000 (UTC) (envelope-from andre@freebsd.org) Received: from c00l3r.networx.ch (c00l3r.networx.ch [62.48.2.2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 348EF2D05 for ; Mon, 9 Sep 2013 07:44:19 +0000 (UTC) Received: (qmail 64804 invoked from network); 9 Sep 2013 08:24:01 -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 ; 9 Sep 2013 08:24:01 -0000 Message-ID: <522D7C46.6030004@freebsd.org> Date: Mon, 09 Sep 2013 09:44:06 +0200 From: Andre Oppermann User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/20130801 Thunderbird/17.0.8 MIME-Version: 1.0 To: Navdeep Parhar Subject: Re: Please review: lazy ext refcount initialization References: <521FD217.5080603@FreeBSD.org> In-Reply-To: <521FD217.5080603@FreeBSD.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: "freebsd-net@freebsd.org" 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: Mon, 09 Sep 2013 07:44:21 -0000 On 30.08.2013 00:58, Navdeep Parhar wrote: > I'd like to merge r254342 from user/np/cxl_tuning to head if there are > no objections. I don't object in principle, though I'm wonder whether we should have a more generic way of passing this kind of flags to the allocator? We probably get more demands for special allocations or variants in the future. And now would be the time as it is still in its infancy and we can easily adjust all consumers before it becomes widespread. See more comments inline. > http://svnweb.freebsd.org/base/user/np/cxl_tuning/sys/kern/kern_mbuf.c?r1=254334&r2=254342&diff_format=u > http://svnweb.freebsd.org/base/user/np/cxl_tuning/sys/sys/mbuf.h?r1=254334&r2=254342&diff_format=u > > --------------------- > Perform lazy initialization of a cluster's refcount if possible. This > doesn't change anything for the common cases where the constructor is > given an mbuf to attach to the cluster, or when the cluster is obtained > with m_cljget(NULL, ...) and attached later with m_cljset(). But it > allows for an alternate usage scenario where the cluster is managed > EXT_EXTREF style without ever having to look up its "usual" refcount via > uma_find_refcnt. > --------------------- > > Regards, > Navdeep > > diff -r 9753d3e51363 -r c9388a59fba6 sys/kern/kern_mbuf.c > --- a/sys/kern/kern_mbuf.c Thu Aug 29 11:16:04 2013 -0700 > +++ b/sys/kern/kern_mbuf.c Thu Aug 29 11:16:04 2013 -0700 > @@ -503,8 +503,6 @@ mb_dtor_pack(void *mem, int size, void * > static int > mb_ctor_clust(void *mem, int size, void *arg, int how) > { > - struct mbuf *m; > - u_int *refcnt; > int type; > uma_zone_t zone; > > @@ -535,10 +533,11 @@ mb_ctor_clust(void *mem, int size, void > break; > } > > - m = (struct mbuf *)arg; > - refcnt = uma_find_refcnt(zone, mem); > - *refcnt = 1; > - if (m != NULL) { > + if (arg != NULL) { > + struct mbuf *m = arg; > + u_int *refcnt = uma_find_refcnt(zone, mem); We typically do not declare variables inside {} statements but only at the top of a function. Also declaration-initialization is not really permitted by style(9) even though it is used in a few places. -- Andre > + > + *refcnt = 1; > m->m_ext.ext_buf = (caddr_t)mem; > m->m_data = m->m_ext.ext_buf; > m->m_flags |= M_EXT; > @@ -549,6 +548,10 @@ mb_ctor_clust(void *mem, int size, void > m->m_ext.ext_type = type; > m->m_ext.ext_flags = 0; > m->m_ext.ref_cnt = refcnt; > + } else { > +#ifdef INVARIANTS > + *uma_find_refcnt(zone, mem) = 0; > +#endif > } > > return (0); > diff -r 9753d3e51363 -r c9388a59fba6 sys/sys/mbuf.h > --- a/sys/sys/mbuf.h Thu Aug 29 11:16:04 2013 -0700 > +++ b/sys/sys/mbuf.h Thu Aug 29 11:16:04 2013 -0700 > @@ -721,6 +721,7 @@ m_cljset(struct mbuf *m, void *cl, int t > m->m_ext.ext_type = type; > m->m_ext.ext_flags = 0; > m->m_ext.ref_cnt = uma_find_refcnt(zone, cl); > + *m->m_ext.ref_cnt = 1; > m->m_flags |= M_EXT; > > } > > _______________________________________________ > freebsd-net@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-net > To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org" > >