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

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Tue, Feb 21, 2012 at 12:30:56PM -0500, Greg Troxel wrote:
> 
> My colleague Bev Schwartz found that in src/sys/dev/pci/if_bnx.c,
> workqueue_enqueue is called with a struct work * that points to a static
> variable.  But, this struct is used as a linked list, and reuse of the
> struct risks creating a circular list.
> 
> bnx's workqueue callback does not free the struct work *.  Other
> workqueue uses seem to alloc before enqueue and then free after the work
> is done.
> 
> It turns out that under our workloads we have yet to see a case where
> wk->wk_entry == wk.
> 
> Can anyone explain why the current code is ok, or how it should change?

I agree it's not clean, and it should probably be changed.
It works because each workqueue (which are per-bnx instance) can have
only one work enqueued at a time, so wk->wk_entry is always NULL.
If the workqueue implementation is ever changed to use something else
but a SIMPLE_QUEUE, this will fail.

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--