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