Re: Network drivers and memory allocation in interrupt context

[Sverre Froyen <sverre%viewmark.com@localhost>]

On On 2011-12-08 at 21:03 David Young wrote
> On Thu, Dec 08, 2011 at 07:06:29PM -0700, Sverre Froyen wrote:
> > Hi,
> > 
> > I now have a semi-working pool_cache based memory allocator for network
> > drivers (tested using the iwn driver). I am uncertain, however, about how
> > to get it fully functioning. The issue is memory allocation in interrupt
> > context.
> 
> Is there some reason that the code in ixgbe(4) cannot be adapted to your
> needs?  For ixgbe(4), I had to solve the precise problem that stops you,
> now.

The iwn driver's iwn_tx_done function requests buffer space in interrupt 
context. If it does not receive a buffer, the interface loops and basically 
stops working. It seems that I have two choices, (1) allocate a new buffer in 
interrupt context or (2) make sure that I have a large enough pool of free 
buffers so that the iwn_tx_done request succeeds.

Option 1 is not available because the bus_dmamem_map call fails in interrupt 
context (but see below).

For option 2, I would like to utilize the machinery of pool_cache / pool. I 
found that the pool_cache_setlowat can be used to guarantee a minimum number 
of free items in the pool (the man page could be clearer on this). When I 
attempt to utilize this with the iwn driver, however, I invariably run into a 
kassert panic in some lower level pool that is using an ipl level of IPL_NONE. 
I believe that the issue is that my pool needs to use IPL_VM (as otherwise 
pool_cache_get_paddr will trigger a panic when called in interrupt context), 
which together with my pool_cache_setlowat setting, results in an attempt to 
grow the pool in interrupt context.

It seems to me that the pool_get kassert:

        if ((cpu_intr_p() || cpu_softintr_p()) && pp->pr_ipl == IPL_NONE &&
            !cold && panicstr == NULL)
                panic("pool '%s' is IPL_NONE, but called from "
                    "interrupt context\n", pp->pr_wchan);

is a bit heavy handed. Say that a pool has an ipl level of IPL_NONE. Why 
cannot pool_get return an already allocated item from this pool even in 
interrupt context? Unfortunately, the mutex(9) choices in pool / pool_cache 
appears to prevent this as the adaptive mutexes that are used for IPL_NONE 
cannot be acquired from interrupt context. A simpleminded solution might be to 
replace the adaptive mutex with a spin mutex plus a condition variable and use 
the same locking for all ipl values. Something like this:

------------------------------------------------------------------------------
mutex mutex_spin
condition variable cv
bool busy = false

/*** Code sections that will never sleep ***/

mutex_enter(mutex_spin)

if (! busy)
        do work
else
        set / return error

mutex_exit(mutex_spin)

/*** Code sections that may sleep ***/

mutex_enter(mutex_spin)

while (! busy)
        cv_wait(cv, mutex_spin)
busy = true
mutex_exit(mutex_spin)

do work that may include sleep

mutex_enter(mutex_spin)
busy = false
cv_signal(cv)

mutex_exit(mutex_spin)
------------------------------------------------------------------------------

Before I take stab at this, however, I would appreciate some feedback:

Am I way off base here and there are reasons why this cannot poossible work?

Does the locking code look reasonable?

Am I attempting to use the pool / pool_cache code in a way that it was never 
intended?

Thanks,
Sverre