Date: Sat, 15 Jul 2000 09:38:44 -0700 From: Alfred Perlstein <bright@wintelcom.net> To: Bosko Milekic <bmilekic@dsuper.net> Cc: net@FreeBSD.ORG, dg@FreeBSD.ORG, dillon@FreeBSD.ORG Subject: Re: mbuf refcnt and sendfile Message-ID: <20000715093844.D25571@fw.wintelcom.net> In-Reply-To: <Pine.BSF.4.21.0007151222010.19154-100000@jehovah.technokratis.com>; from bmilekic@dsuper.net on Sat, Jul 15, 2000 at 12:25:03PM -0400 References: <20000715065506.Y25571@fw.wintelcom.net> <Pine.BSF.4.21.0007151222010.19154-100000@jehovah.technokratis.com>
next in thread | previous in thread | raw e-mail | index | archive | help
* Bosko Milekic <bmilekic@dsuper.net> [000715 09:22] wrote: > > > On Sat, 15 Jul 2000, Alfred Perlstein wrote: > > > http://www.freebsd.org/cgi/query-pr.cgi?pr=19866 > > > > David, I'm pretty sure you didn't like the 'fix' for the mbuf > > cluster refcount presented in this PR (linking all copies using > > a doubly linked list), I have presented an alternative: > > > > Instead of keeping them in a linked list there should be an int/char * > > in the mbuf header that works the same way mclrefcnt does. Instead > > of managing a linked list all one has to do is copy the pointer into > > the new mbuf header and increment it, and decrease it on free, when > > it's zero the deref code is called. > > This takes away from the transparency of the system and further adds > the necessity of having to splimp() for something as simple as checking > whether an mbuf's ext_buf is referenced by more than one object. If you > keep the counter outside of the isolated mbuf (which is what you're > suggesting with the pointer idea), then you have to splimp() before you > check the value of the referred counter. And why exactly don't you have to splimp if it's a linked list? I assume you mean an object may be referenced by more than just mbufs, the solution is simple, you just keep that count seperate: if (myobj->mbrefcnt == 0 && myobj->otherrefcnt == 0) myfree(myobj); Of course your ext_free function will have to do the same, but you can't avoid this. Also, the refcounting can be done with atomic operations therefore you won't need spl, if you atomically decrease the ref and it's zero, then the network stack doesn't have access to it. Lastly your changes bloat the size of an mbuf by 12 bytes, although really only 8 because ext_args isn't needed. > > I was wondering what your thoughts on this are? > > > > I also had an idea to save on sf_buf's in sendfile: > > > > Forget about them, set the m_ext->ext_buf to point directly at the > > vm_page_t backing the mbuf, you don't need the extra indirection. > > If the ext_buf referencing remains transparent, then you don't really > need the extra indirection management, as far as I know! But I'm not > sure! David probably knows this interface WAAAY better (he wrote it!) :-) All the code I've seen really doesn't use it for external clusters that aren't mbuf clusters. Here's the response to the PR: * Bosko Milekic <bmilekic@dsuper.net> [000715 08:30] wrote: > > I disagree. Especially since you haven't said why it's better than > what's proposed. The overhead of the list management isn't good, a single count can be manipulated atomically which will be a win when we SMP-ify things. > Of course, here's the reason why: If you are holding a POINTER to an > outside reference counter, then you make think > that you're accomodating things for counters outside the subsystem > but in reality, if you are allocating objects of dynamic size with > malloc() at some point, you don't really want to have to allocate and > manage a reference count scheme for that one object anyway. > I don't know about you but I want to be able to do malloc(), > MEXTADD() the ext_buf to the mbuf, and be ready to go. Then, if > m_copym ever gets called on my mbuf (or my mbuf is in the chain), I > want it to be taken care of by itself, such that when the mbuf is > freed, the ext_buf necessarily won't be. I don't want to have to > malloc() extra space for a counter. You don't need to, the mbuf system can still do that for you. > So the point is: the mbuf subsystem is supposed to transparently > manage the reference count scheme for itself and if the ext_free > routine is called and there is an external reference (you can pass it > using the ext_args pointer) then you don't necessarily need to free > the object. > > If you still think it should be otherwise, let me know why and I'll > change it, but I would like to get this off my back as soon as > possible. I strongly object to the implemetation you propose, however I also strongly agree that it the current way isn't good. You can either malloc() extra space for a counter, or you can allocate them seperately like so: struct mbrefcnt { struct mbrefcnt *mbref_next; /* freelist */ char mbref_cnt; /* count */ }; and have the mbuf code manage these structures. -- -Alfred Perlstein - [bright@wintelcom.net|alfred@freebsd.org] "I have the heart of a child; I keep it in a jar on my desk." 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?20000715093844.D25571>