Date: Fri, 9 Feb 2001 08:48:26 +0600 (ALMT) From: Boris Popov <bp@butya.kz> To: Bosko Milekic <bmilekic@technokratis.com> Cc: freebsd-arch@FreeBSD.ORG, freebsd-net@FreeBSD.ORG Subject: Re: Sequential mbuf read/write extensions Message-ID: <Pine.BSF.4.21.0102090831010.24710-100000@lion.butya.kz> In-Reply-To: <013201c09203$971be9c0$1f90c918@jehovah>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 8 Feb 2001, Bosko Milekic wrote:
> in mb_init(), the m->m_pkthdr.rcvif = NULL; can be ommitted, as
> MGETHDR() will do that. The m->m_len = 0 should stay for now.
Ok.
> drivers that pre-allocate mbufs + clusters, they typically know the
> `count'), it turns out that it is cheaper to compute the count from
> the size than the optimal size from the count, in the mbuf case. If
> you don't mind, I would strongly recommend moving m_getm() to
> uipc_mbuf.c. Code that doesn't know the `size' but knows the `count'
Agreed, that why this function have a prefix 'm_' :)
[code sample skipped]
> For this to work, though, m_getm() needs to be modified to free all of
> `top' chain if it can't get either a cluster or an mbuf. I don't know
> if this was intentional, but it seems to me that there is a subtle
> problem in m_getm() as it is now:
>
> if (len > MINCLSIZE) {
> MCLGET(m, M_TRYWAIT);
> if ((m->m_flags & M_EXT) == 0) {
> m_freem(m); <------ frees only one mbuf
^^^^^^^^^^ cluster is not in the chain yet, so it have to be
freed.
> return NULL;
> }
> }
>
> I think what may happen here is that you will leak your `top' chain if
> you fail to allocate a cluster.
The original semantic was not to free an entire chain because
m_getm() do not reallocates original (top) mbuf(s) (which may contain
data) and only adds new mbufs/clusters if possible. So, the calls like
m_get(mb->mb_top) will not left the wild pointer. There is also simple way
to deal with such behavior:
mtop = m_get(...);
if (mtop == NULL)
fail;
if (m_getm(mtop) == NULL) {
m_freem(mtop);
fail;
}
Probably m_getm() should return error code rather than pointer to
mbuf to avoid confusion.
--
Boris Popov
http://www.butya.kz/~bp/
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-net" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.0102090831010.24710-100000>