Discussion:
minor bug in mbuf code
(too old to reply)
Beverly Schwartz
2013-09-20 17:06:32 UTC
Permalink
Following the code in m_freem, there are different code paths for when an mbuf is using its own space vs. when it is using an external buffer.

For the code path for an mbuf using its own space, we get to MFREE in sys/sys/mbuf.h. In this code, before returning the mbuf to the pool, m->m_type is set to M_FREE.

For the code path for an mbuf using external space, m->m_type is not touched when the mbuf is returned to the pool.

This is a minor problem in that there is no code that is checking that an mbuf's m_type is M_FREE when it comes out of the pool, and the m_get family does set this field.

However, for anyone doing debugging and looking at potentially hanging pointers, it would be helpful if all mbufs that have been returned to the pool to actually have m->m_type set to MT_FREE.

Would someone be willing to commit this very minor change to the repository?

-Bev


Author: Bev Schwartz <***@bbn.com>
Date: Mon Sep 16 11:45:04 2013 -0400

mark mbuf associated w/ cluster MT_FREE when returned to pool

If an mbuf is *not* associated with an mbuf cluster, when it is
returned to a pool, it is marked "MT_FREE".

When an mbuf *is* associated with an mbuf cluster, it goes through
a different code path to be returned to the pool, and this different
code path doesn't mark the mbuf as type MT_FREE.

The reason for the different (and more complex) code path for
freeing mbufs when the mbuf is associated with a cluster is that
the mbuf that "owns" the cluster cannot be freed until there
are no more references to the cluster.

This bug doesn't affect the functionality of NetBSD as it is now,
but if ever some code checks that an mbuf coming out of the pool
is, in fact, type MT_FREE, there will be a problem. Better to
fix the problem and reduce future confusion.

diff --git a/netbsd/src/sys/kern/uipc_mbuf.c b/netbsd/src/sys/kern/uipc_mbuf.c
index 84ffe60..542347a 100644
--- a/netbsd/src/sys/kern/uipc_mbuf.c
+++ b/netbsd/src/sys/kern/uipc_mbuf.c
@@ -1646,6 +1646,7 @@ m_ext_free(struct mbuf *m)
}
}
if (dofree) {
+ m->m_type = MT_FREE;
pool_cache_put(mb_cache, m);
}
}


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Christos Zoulas
2013-09-20 19:13:48 UTC
Permalink
Post by Beverly Schwartz
Following the code in m_freem, there are different code paths for when
an mbuf is using its own space vs. when it is using an external buffer.
For the code path for an mbuf using its own space, we get to MFREE in
sys/sys/mbuf.h. In this code, before returning the mbuf to the pool,
m->m_type is set to M_FREE.
For the code path for an mbuf using external space, m->m_type is not
touched when the mbuf is returned to the pool.
This is a minor problem in that there is no code that is checking that
an mbuf's m_type is M_FREE when it comes out of the pool, and the m_get
family does set this field.
However, for anyone doing debugging and looking at potentially hanging
pointers, it would be helpful if all mbufs that have been returned to
the pool to actually have m->m_type set to MT_FREE.
Would someone be willing to commit this very minor change to the repository?
Done...

christos


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Loading...