On Fri, May 31, 2002 at 12:54:39PM -0700, Julian Elischer wrote:
>
>
> On Fri, 31 May 2002, Bosko Milekic wrote:
>
> >
> > I don't remember why the ext_size here was this originally (as you
> > mention, it was imported that way), but it certainly seems bogus and
> > you catching it now is hopefully going to solve some really wierd and
> > difficult-to-debug problems we've had involving mbufs over the years.
> >
> It was imported from FreeBSD-1.x by rod.
>
> The size argument was for two reasons.
> 1/ to allow the M_LEADINGSPACE and M_TRAILINGSPACE calculations
> to be done if it was a single reference object.
> 2/ to allow the free function (in the case of non cluster external
> objects) to know what sized object they had in the case that they needed
> this information.
Yeah, we still need the field to exist because of (2). I just meant
that we should remove the explicit setting of 0 of the field in
m_split(), and that's what Archie suggested.
> I know because I added it, because I needed to do both of these at TRW in
> '90-'95 under MACH/BSD and when I moved the code to FreeBSD1.x it cam
> along.. there was no M_LEADINGSPACE/M_TRAILINGSPACE macro at that time..
> I did it in my code explicitly.
>
> It was not used in standard code because only in my own protocol stack
> did I know that the damned object was not shared and writable..
> This has improved with time..
>
> Having the size set to 0 however stopped users from hitting
> cases where the WRITABILITY of the ext objext has not been correctly
> tracked as it always returns "not enough space" (sometimes -ve).
Right now, in -CURRENT, we have a M_WRITABLE macro, as you noticed,
that evalutes true unless one of these conditions is true:
1) M_RDONLY is set in the mbuf
2) mbuf has external buffer attached and ref. count on that buffer
(e.g., cluster, whatever) is > 1.
This macro (M_WRITABLE) _is_ currently used in M_LEADINGSPACE.
If the mbuf being checked is not writable, M_LEADINGSPACE
will evalute to 0. However, I noticed just now that it is
not used in M_TRAILINGSPACE, where it probably should be.
M_TRAILINGSPACE should probably look like this:
#define M_TRAILINGSPACE(m) \
((m)->m_flags & M_EXT ? \
(M_WRITABLE(m) ? \
(m)->m_ext.ext_buf + (m)->m_ext.ext_size - \
((m)->m_data + (m)->m_len) : 0) : \
&(m)->m_dat[MLEN] - ((m)->m_data + (m)->m_len))
This is the same philosophy adopted by M_LEADINGSPACE.
I have no idea how I missed that when we did the M_WRITABLE stuff. Or
did we leave it out for some specific reason? Ian Dowse or David Malone
would know, as the WRITABLE code was thanks to them. :-)
> If we change this we need to audit the WRITABILTY..
> e.g. it is not checked in M_TRAILINGSPACE
Right.
> Julian
--
Bosko Milekic
[EMAIL PROTECTED]
[EMAIL PROTECTED]
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-net" in the body of the message
