Date: Mon, 09 Sep 2013 09:44:06 +0200 From: Andre Oppermann <andre@freebsd.org> To: Navdeep Parhar <np@FreeBSD.org> Cc: "freebsd-net@freebsd.org" <freebsd-net@freebsd.org> Subject: Re: Please review: lazy ext refcount initialization Message-ID: <522D7C46.6030004@freebsd.org> In-Reply-To: <521FD217.5080603@FreeBSD.org> References: <521FD217.5080603@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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" > >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?522D7C46.6030004>