🚀 go-pugleaf

	There is a potential problem with the proposed modifications
to the mbuf allocator.  The "cantwait" argument to the allocation
routines is more than just an indication of whether it is ok to sleep.
It is also a subtle hint that the routine was not called from a
(potentially plimp) interrupt routine.  If an allocation routine
is called from, e.g., the ethernet interrupt it would be incorrect
to (re)enter the network at plnet because (1) the network code may
inadvertently lower the pl and (2) the network code may itself have
been interrupted at a critical section.
	Unfortunately, to be useful, most *_drain routines must
ultimately access global structures which are protected only by
splnet's and thus are eventually likely to cause corruption.  It
may be possible to check the current pl on entry to the allocator
(tricky because of the macros) and distinguish three classes of
request (from least to most restrictive):  task-time (may sleep
and/or call the network), interrupt-time where previous pl was
less than plnet (may call the network), and interrupt-time where
previous pl was plnet or higher.  Of course, since you cannot
find the true previous pl, you would have to assume that the current
pl is higher than the previous and work from there.  This scheme
breaks down if any plnet code calls the allocator in the expectation
that it won't reenter, but such cases could be fixed.
	There is a somewhat different approach to the mbuf
problem which might also be helpful.  History:  The allocator
in 2.9 ignored the "cantwait" argument--it never slept at all.
Needless to say, problems arose frequently.  My first attempt
to improve the situation was to add appropriate sleeps, making
the system run much the way 4.3 does now, i.e., top-level
code can wait for mbufs but most other code can't.  This helped
very little.  Typically, a send call would block in the
allocator until an mbuf became available and then call the
tcp send routine which would promptly request an mbuf "DONTWAIT"
and fail.  The solution was to make allocation requests which
*could* wait *always* wait until some fraction (say, 50%) of
mbufs were free.  This is effect reserved half of the mbufs
for code that couldn't wait and improved matters significantly.

				Dan Lanciani
				ddl@harvard.*

In article <2144@husc6.harvard.edu> ddl@husc6.harvard.edu (Dan Lanciani) writes:

>	There is a potential problem with the proposed modifications
>to the mbuf allocator.  The "cantwait" argument to the allocation
>routines is more than just an indication of whether it is ok to sleep.
>It is also a subtle hint that the routine was not called from a
>(potentially plimp) interrupt routine.  If an allocation routine
>is called from, e.g., the ethernet interrupt it would be incorrect
>to (re)enter the network at plnet because (1) the network code may
>inadvertently lower the pl and (2) the network code may itself have
>been interrupted at a critical section.

>	Unfortunately, to be useful, most *_drain routines must
>ultimately access global structures which are protected only by
>splnet's and thus are eventually likely to cause corruption.  

	Thanks for the hints/warning.

	i'd like to "explore"/"discuss" this a bit.

	Inadvertent lowering of the priority is the bugaboo to be
	afraid of as i understand it, the consequences of recursive
	interrupts, etc are well known to me (i only blew it twice in
	getting if_ec.c working for 2.10.1BSD).

	The m_more() routine is required to be called at splimp(), not splnet().

	The m_expand() routine which is called by m_more() also is required 
	to be called at splimp().

	The tcp_drain() routine i proposed (actually i'm running it as i type)
	raises spl to splimp() out of paranoia.  Besides which, the *_drain 
	routines are only called from m_expand() which is at splimp already.

	Last i looked, splimp() is higher than splnet() so there is not
	any inadvertent lowering of priority.

	Yes, if in the future, the *_drain routines get "smarter" or more
	complicated, then problems probably will arise.  Given the current
	setup everything seems in order.

	What have i missed?  Please be kind - it was late when i "had to
	do something about the ftp mget" problem ;-)

>The solution was to make allocation requests which
>*could* wait *always* wait until some fraction (say, 50%) of
>mbufs were free.  This is effect reserved half of the mbufs
>for code that couldn't wait and improved matters significantly.

	Hmmmm, that sounds interesting!  Might be worth a shot.  Thanks!

	Steven M. Schultz
	sms@wlv.imsd.contel.com

In article <33315@wlbr.IMSD.CONTEL.COM>, sms@WLV.IMSD.CONTEL.COM (Steven M. Schultz(Y)) writes:
| 	Inadvertent lowering of the priority is the bugaboo to be
| 	afraid of as i understand it, the consequences of recursive
| 	interrupts, etc are well known to me (i only blew it twice in
| 	getting if_ec.c working for 2.10.1BSD).

	This is no more or less to be feared than corruption of global
data structures.

| 	The m_more() routine is required to be called at splimp(), not splnet().
| 
| 	The m_expand() routine which is called by m_more() also is required 
| 	to be called at splimp().

	This is done so that the allocator can be called from plimp
interrupt routines.  The mbuf allocator is one of the few pieces of
the network that may be called by such routines.  Another is the
arpinput code, but that's easy to get rid of and I digress...

| 	The tcp_drain() routine i proposed (actually i'm running it as i type)
| 	raises spl to splimp() out of paranoia.  Besides which, the *_drain 
| 	routines are only called from m_expand() which is at splimp already.

	This piece of paranoia both does no good and is misleading.  It
protects your tcp_drain routine from being interrupted, but this isn't
the problem.  The problem is that your tcp_drain routine can be *called*
from a plimp interrupt routine.  In any case, the problem is not
with tcp_drain, but with the modification to the memory allocator.

| 	Last i looked, splimp() is higher than splnet() so there is not
| 	any inadvertent lowering of priority.

	But your tcp_drain does not stand by itself.  It calls tcp_close.
Tcp_close calls (among other things) in_pcbdetach.  In_pcbdetach access
the linked list of protocol control blocks.  It also calls sofree.

| 	Yes, if in the future, the *_drain routines get "smarter" or more
| 	complicated, then problems probably will arise.  Given the current
| 	setup everything seems in order.

	You can't get much more complicated than calling the tcp_close
function...

| 	What have i missed?  Please be kind - it was late when i "had to
| 	do something about the ftp mget" problem ;-)

	From the structure of your loop in tcp_drain, you obviously
noticed that tcp_close will delete protocol control blocks from the
tcb chain.  How would you feel if you were a happy little plnet
routine walking the tcb chain and then the tcp_drain routine was called
by the ethernet interrupt's asking for an mbuf, thus invalidating
your pointer?
	For a more definite example, consider the tcp_fasttimo routine
and think about what would happen if you deleted the protocol control
block that it was currently looking at.

				Dan Lanciani
				ddl@harvard.*