Re: workqueue in if_bnx: static struct work * seems wrong

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

On Friday 02 March 2012 10:00:44 Manuel Bouyer wrote:
> On Fri, Mar 02, 2012 at 09:57:00AM -0700, Sverre Froyen wrote:
> > Sorry, I was following up to the latest message on the thread and doing a
> > poor job of quoting.
> > 
> > I was really thinking about the change that prompted this discussion
> > (if_bnx.c rev 1.44). In particular the replacement of
> > 
> >     bnx_alloc_pkts(sc)
> > 
> > with
> > 
> >     workqueue_enqueue(sc->bnx_wq, &bnx_wk, NULL);
> >     SET(sc->bnx_flags, BNX_ALLOC_PKTS_FLAG);
> > 
> > which was made in order to avoid memory allocation in interrupt context.
> > 
> > It seems to me that this could be handled by the pool code instead.
> > Perhaps as follows:
> > 
> > 1) Create pool_caches of header structures (one per buffer size).
> > 
> > 2) Use a custom allocator that does the bus_dma dance and attaches the
> > resulting memory pointers to the header structure.
> 
> The problem is actually in bus_dma, so using pool_cache here won't help.
> The problematic bus_dma routine (I don't remmeber which one, sorry) could
> still be called from interrupt context.

It's bus_dmamem_map which is using a pool with IPL_NONE. This is one of the 
issues I was referring to. I think the trick might be to

1) allow pool_cache_get to return existing items for an IPL_NONE cache even 
when called in interrupt context (this will require changes to the pool / 
pool_cache locking code).

2) fix the code that manages the low water mark so that it does not run in 
interrupt context (perhaps use a separate thread to grow the pools).

The idea is that the low water mark would guarantee available buffers for the 
drivers, most of the time.

Regards,
Sverre