Please review: lazy ext refcount initialization

[Andre Oppermann]

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 at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscribe at freebsd.org"
>
>