Date: Fri, 17 Aug 2012 10:09:45 -0400 From: John Baldwin <jhb@freebsd.org> To: freebsd-net@freebsd.org Cc: net@freebsd.org, Vijay Singh <vijju.singh@gmail.com> Subject: Re: a query in mb_free_ext() Message-ID: <201208171009.45212.jhb@freebsd.org> In-Reply-To: <CALCNsJSXt%2BYxCfg629nQk7a4f3JaaK=xdqXG8MqNXR3JrhC8Dw@mail.gmail.com> References: <CALCNsJSXt%2BYxCfg629nQk7a4f3JaaK=xdqXG8MqNXR3JrhC8Dw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday, August 16, 2012 11:33:12 pm Vijay Singh wrote:
> Does anyone here understand this bit of code in mb_free_ext()?
>
> /* Free attached storage if this mbuf is the only reference to it.
*/
> if (*(m->m_ext.ref_cnt) == 1 ||
> atomic_fetchadd_int(m->m_ext.ref_cnt, -1) == 1) {
> switch (m->m_ext.ext_type) {
> case EXT_PACKET: /* The packet zone is special. */
> if (*(m->m_ext.ref_cnt) == 0)
> *(m->m_ext.ref_cnt) = 1;
> uma_zfree(zone_pack, m);
> return; /* Job done. */
>
> Why would *(m->m_ext.ref_cnt) == 0 be true when the if condition
> checks only for ref_cnt to be 1? Should the atomic_fetchadd_int() be
> checked for <= 1?
atomic_fetchadd_int() returns the old value (so the value before the
decrement). It would be simpler to read if the code was written as:
if (atomic_fetchadd_int(m->m_ext.ref_cnt, -1) == 1) {
switch (m->m_ext.ext_type) {
case EXT_PACKET:
/* Keep a reference on the cluster while it is in the packet zone. */
*m->m_ext.ref_cnt = 1;
uma_zfree(zone_pack, m);
return;
...
}
The check for == 1 and then a check against == 0 is a (possibly dubious)
micro-optimization to handle the case where we are not racing with another
CPU to drop the reference count on the cluster and can see that we have the
sole reference in that it avoids the atomic_fetchadd_int() and the store in
that case. OTOH, the check to see if the store should be done (and the
resulting branch) is probably worse than just always doing the store.
> Also, why is the packet zone special? Any history here?
To my knowledge, the packet zone stores mbufs that have an attached cluster
so that an mbuf+cluster can be allocated in one operation when needed rather
than requiring separate allocations of an mbuf and a cluster and then tying
the two together. m_getcl() pulls items out of this zone, and this zone
builds mbuf+cluster items from the backing mbuf and cluster zones when it
is depleted.
--
John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201208171009.45212.jhb>