Date: Tue, 29 Aug 2000 16:55:20 -0700 (PDT) From: Archie Cobbs <archie@whistle.com> To: David Malone <dwmalone@maths.tcd.ie> Cc: bmilekic@dsuper.net, iedowse@maths.tcd.ie, freebsd-net@FreeBSD.ORG Subject: Re: Proposal to clarify mbuf handling rules Message-ID: <200008292355.QAA80880@bubba.whistle.com> In-Reply-To: <200008291159.aa60672@salmon.maths.tcd.ie> "from David Malone at Aug 29, 2000 11:59:06 am"
next in thread | previous in thread | raw e-mail | index | archive | help
David Malone 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. > > 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). Right.. M_RDONLY means "the originator of this M_EXT mbuf doesn't want it to be written to, ever, period." M_WRITABLE would mean "M_RDONLY is not set, and moreover you can go ahead and write to the mbuf without further checking." > 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. Ah.. didn't think of that. > 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. I agree.. you are probably right. Let's get rid of M_WRITABLE altogether. We can keep M_RDONLY and use M_IS_WRITABLE(m) as a macro. Except it needs to be this instead I think: #define M_IS_WRITABLE(m) (!((m)->m_flags & M_RDONLY) \ && (!((m)->m_flags & M_EXT) || !MEXT_IS_REF(m))) > 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. Not sure about this idea.. it seems like in practice mbufs are usually writable, whearas being M_RDONLY is the exception. If we used M_MAYBEWRITEABLE then we'd have to change a lot more code. -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?200008292355.QAA80880>