Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 01 Sep 1998 22:07:51 +0000
From:      Mike Smith <mike@smith.net.au>
To:        Stefan Bethke <stb@hanse.de>
Cc:        net@FreeBSD.ORG, freebsd-current@FreeBSD.ORG
Subject:   Re: Problems with MGET(m, M_WAIT, *) [was: Semantics of ...] 
Message-ID:  <199809012207.WAA00723@word.smith.net.au>
In-Reply-To: Your message of "Wed, 02 Sep 1998 00:36:28 %2B0200." <Pine.BSF.3.96.980901232001.28484I-100000@transit.hanse.de> 

next in thread | previous in thread | raw e-mail | index | archive | help
> > > > What are the expected semantics of MGET(m, M_WAIT, *)? I would suggest
> > > > that by specifing M_WAIT, the caller wants to sleep until a mbuf becomes
> > > > available, as it is already the case if the vm map must be extended.
> > > 
> > > It should sleep, but actually doing so while avoiding deadlocks is
> > > problematic.  Since the mbuf allocator as currently formulated is
> > > going away, callers to mget should expect that the allocation might
> > > fail, but that M_WAIT makes it ``try harder'' as it were.

This comment is significant in the context of a question you ask later.

> According to TCP Illustrated Vol II, p. 42:
> 
>     Even though the caller specifies M_WAIT, ther return value must still be
>     checked, since [...] waiting for an mbuf does not guarantee that one
>     will be available.
> 
> Although our mbuf allocator is modified, it pretty much describes the
> semantics.

Yes.

> I've tried to locate pieces of code in a recent -current that use M_WAIT,
> but don't check for the return value. Interestingly enought, only 23
> occurences of 160 total in the source use M_WAIT at all. For m_get(), its 22
> to 110. 

How many of those that don't wait handle a NULL return correctly?  Any 
idea how many of them *might* be able to wait?

> Checking the 45 hits for pieces of code shows the following problem
> areas:
>
> For a total of 22 occurences, the return value isn't checked, in:
> kern/uipc_socket:479 and :484, :1059 (including sosend())
> netinet/ip_output.c:869, :884, :927, and :1345
> netinet/raw_ip.c:285
> netinet/tcp_usrreq.c:641
> netkey/key.c:2199
> nfs/krpc_subr.c:283 and :474
> nfs/nfs_serv.c:517 and :685
> nfs/nfs_socket.c:285 and :1188
> nfs/nfs_subs.c:589, :684, :713, :746, :889, :921, :970, and :1063
> 
> In additional two occurences, I'm not sure, but I regard it as quite likely
> that the function called will stumble over a NULL mbuf pointer, or a pointer
> to it:
> kern/uipc_socket.c:597
> netinet/ip_mroute.c:425
> 
> So I have the following questions:
> As Garrett said, the current mbuf allocator is going to go away. Is it? I
> tried to find anything about it in the list archives, but to no avail.

I believe that Garrett is working on a replacement.  It's possible that 
it will be using the zone allocator, but I don't remember why I thought 
that.

> Will the semantics of the new mbuf allocator be the same (M_WAIT may
> return 0)? I guess so, but the one(s) working on the new allocator might
> want to confirm that.

If I read the comment I referenced above correctly, it's describing how 
consumers should behave in order to work with the pending new allocator.

> If so, should I start on trying to fix it, or will it be easy enougth to
> fix these bugs when the new allocator is coming? At least some these
> occurences seam to be fixable quite easily.

Definitely fix them.  If you run into ones that might need more 
attention (ie. not an easy fix), then applying either a warning comment 
or a #warning diagnostic to them to flag the problems might actually be 
a very good idea.

Thanks for the analysis!
-- 
\\  Sometimes you're ahead,       \\  Mike Smith
\\  sometimes you're behind.      \\  mike@smith.net.au
\\  The race is long, and in the  \\  msmith@freebsd.org
\\  end it's only with yourself.  \\  msmith@cdrom.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?199809012207.WAA00723>