Date: Tue, 29 Aug 2000 11:59:06 +0100 From: David Malone <dwmalone@maths.tcd.ie> To: Archie Cobbs <archie@whistle.com> Cc: bmilekic@dsuper.net, dwmalone@maths.tcd.ie, freebsd-net@FreeBSD.ORG, iedowse@maths.tcd.ie Subject: Re: Proposal to clarify mbuf handling rules Message-ID: <200008291159.aa60672@salmon.maths.tcd.ie> In-Reply-To: Your message of "Mon, 28 Aug 2000 13:41:16 PDT." <200008282041.NAA71818@bubba.whistle.com>
next in thread | previous in thread | raw e-mail | index | archive | help
> (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. I think it would be a nice idea not to tie this to clusters, but to have the M_RDONLY as an option which is given at the time which external storage is first attached to a mbuf. This means we set M_RDONLY for sendfile bufs, but don't set it for mbuf's with no external storage, regular clusters or for jumbo ethernet storage (once we've checked this with Bill Paul naturally). We will have to be careful with the M_WRITABLE flag, as it is stored in the mbufs rather than the externam storage, and there may be multiple mbufs referencing this storage. Think of the following situations: 1) When we first make a mbuf we set M_WRITABLE according to !M_RDONLY. 2) If we make a second mbuf which references this original mbuf we can clear M_WRITABLE in both mbufs, as we have access to both. 3) Adding references from now on isn't a problem as M_WRITABLE is clear. 4) Now, consider the situation where we are freeing the second last mbuf referencing this storage. We have access to the mbuf we are freeing, but not to the remain mbuf, in which we want to set M_WRITABLE. I can't see an obvious way around this, so we may find situations where M_WRITABLE is not set, but the storage is writable. This would translate to a M_IS_WRITABLE macro like: #define M_IS_WRITABLE(m) ((m)->m_flags & M_WRITABLE) || \ (!((m)->m_flags & M_RDONLY) && !MEXT_IS_REF(m) && \ ((m)->m_flags |= M_WRITABLE) \ ) Note, this macro actually sets the M_WRITABLE flag, which is a bit icky - but seems reasonable given that M_WRITABLE is supposed to be cache for the value of !((m)->m_flags & M_RDONLY) && !MEXT_IS_REF(m). The alternative is to just add the M_RDONLY flag and define: #define M_IS_WRITABLE(m) (!((m)->m_flags & M_RDONLY) && !MEXT_IS_REF(m)) I think in real terms we may find this is a cleaner solution; This check is not going to be a hugely expensive operation compared to the alternative above. Finally, it might actually be better to have the opposit sense for the M_RDONLY flag (call is M_MAYBEWRITEABLE or somethign more catchy). This would mean that if someone initially forgets to set the flag, it will be zero and the storage will not accidently be written to. I think it would also give us a better chance of catching things we're accidently writing to with KASSERTs. David. 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? <200008291159.aa60672>