Date: Wed, 30 Aug 2000 11:33:49 -0700 (PDT) From: Archie Cobbs <archie@whistle.com> To: Bosko Milekic <bmilekic@dsuper.net> Cc: David Malone <dwmalone@maths.tcd.ie>, freebsd-net@FreeBSD.ORG Subject: Re: Proposal to clarify mbuf handling rules Message-ID: <200008301833.LAA31594@bubba.whistle.com> In-Reply-To: <Pine.BSF.4.21.0008291843410.9530-100000@jehovah.technokratis.com> "from Bosko Milekic at Aug 29, 2000 08:39:35 pm"
next in thread | previous in thread | raw e-mail | index | archive | help
Bosko Milekic writes:
> > > (a) The mbuf data should not be written to under any circumstances.
> > > even if it is a cluster mbuf and the reference count becomes 1.
> >
> > > (b) The mbuf data may be written to. This implies that if this mbuf
> > > is a cluster, then the reference count is 1.
> >
> > > (c) The mbuf data may not be written to, BUT if it is a cluster mbuf
> > > and the reference count decreases to 1, then it may then be
> > > written to.
>
> All of these different possibilities add quite a bit of hysteria for
> just figuring out which of the above a given mbuf falls under from within
> the mbuf subsystem alone.
> I think that it's rather clear that if we do intend to implement some
> sort of permissions structure with respect to external buffers that we
> need to have the mbufs referring to these ext_bufs share a unique
> permissions field.
> The solution: Change mext_refcnt union to be a mext_descr union with
> an additional field falling within like so:
>
> union mext_descr {
> union mext_descr *next_desc;
> struct {
> u_long refcnt;
> u_int perms;
> } desc;
> };
>
> The cost: Additional 4 bytes per every M_EXT mbuf. (These unions are
> typically allocated en-masse from mb_map during bootup and more are added
> from mb_map on demand, assuming we have the space in mb_map, otherwise,
> ... we'll probably have to implement another tsleep mechanism).
>
> perms is to be modified atomically and can contain, for now:
>
> M_RDWR 0x00000000
> M_RDONLY 0x00000001
> [... more to come ...?]
>
> (Notice that the mext_descr _only_ contains SMP-safe atomically
> modifiable and shareable settings for the ext_buf -- this is like shared
> space for M_EXT mbufs referring to the same ext_buf -- the SMP-friendly
> version of the doubly-linked list solution).
>
> This solution would allow on-the-fly modification of ext_bufs
> permissions.
>
> If this works for you guys, and if you think it's worth it, then we
> should also add an ext_flags in the m_ext structure while we're add it,
> because it will cost us nothing (m_ext is within a union containing ample
> amount of scratch space in an M_EXT mbuf) and because it will allow me to
> remove the code that assumes that if ext_free is NULL, that we're dealing
> with an mcluster (which is true for now); but the new check of the flag
> would make the code a little faster at 0 extra overhead. The idea would
> be to have that ext_flags contain one of M_CLUSTER, M_WHATEVER, M_OTHER.
It makes sense to put the info about the shared mbuf data into the
(single) union mext_descr that all the mbuf's point to. But why
not put ext_flags in there as well?
Also, why are "perms" and "refcnt" in the same union? It seems like
you will lose the "perms" information when you increase refcnt to 2,
leading to the same problem mentioned before (a shared mbuf data
region going from 2 -> 1 reference does not become writable again).
What is "next_desc" used for? How does that affect this?
-Archie
___________________________________________________________________________
Archie Cobbs * Whistle Communications, Inc. * http://www.whistle.com
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?200008301833.LAA31594>