Subject: Re: mbuf external storage sharing
To: None <jonathan@dsg.stanford.edu>
From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
List: tech-net
Date: 10/08/2004 11:29:33
> First, My apologies for my delay in responding to your message.
no problem.
> [...]
> >> For it seems to contradict the
> >> conclusions in the FreeBSD-5 paper I cited earlier.
> >
> >how contradict?
> >allocating the little header is what exactly freebsd5 does.
> >only difference is, freebsd5's header contains only reference count.
>
> Going by Bosko Milekic's paper [I need to reread more recent source
> code]: FreeBSD-5 uses m_clget() to atomically allocate one physically-noncontigous little header and one external mbuf (cluster mbuf).
>
> Both the little mbuf header (or even a tiny header-only mbuf, in other
> approaches) and the cluster are allocated from a per-cpu bucket.
> Also, each CPU has to disable recursive network interrupts in any
> case. Thus, if one designs the data structures carefully, one need
> not acquire spinlocks on the mbuf-cluster pool [sic] or the
> little-mbuf pool [sic]: there are no races between the CPU and itself,
> so need for spinlocks.
(assuming that you mean m_getcl,)
as i wrote in the previous mail,
the optimization, which might be good idea, does not contradict but
is independent to what my patch does.
> [...]
>
> >> >not relevant to my patch.
> >>
> >> Yes and no: [....]
>
> >be more specific, please.
> >you can easily modify pool_cache to use per-cpu cache.
> >how is it related to my patch?
>
>
> I would prefer an approach whereby the per-cpu caches are race-free,
> and thus the common case (i.e., atomically allocating both an mbuf
> header and a cluster, satisifed from the per-cpu pool) need not
> acquire per-mbuf spinlocks at all.
>
> That approach also generaalizes well to other allocations than for NIC
> device-input DMA queues. (e.g.,sosend() in the non-loanout case?),
> whereas I'm skeptical the approach in your patch will be applicable.
>
> Last, I do appreciate your patch, and the thought that has gone into
> it. I fully support an m_clget()-stype interface, for an atomic "grab
> a hdr-mbuf plus cluster" atomically, with efficient locking of the
> joint operation. I support making that operation MP-safe. I just
> happen to think we can do slightly better and more genarlly than in
> your proposed patch.
i don't understand what you mean, sorry.
it seems that you're confused between different topics.
there're (at least) two topics you're mixing up:
- how to share/unshare mbuf external storage
it's what my patch attmpts to change.
both of my patch and freebsd5 allocate a small header which
contains a reference count. no much difference here, afaik.
to manipulate the reference count, freebsd5 uses atomic_ operations
and my patch uses spinlock. i used spinlock just because we don't
have MI-exposed atomic_ operations.
(it might be good to have MI-exposed atomic_ operations like
freebsd and linux. but it's a separate topic.)
- how to allocate/free mbufs and clusters
freebsd5 has some optimizations, including m_getcl.
although they might be good and i have no objections against them,
they're orthogonal to my patch.
YAMAMOTO Takashi