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

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

On Sun, Feb 26, 2012 at 05:22:50PM +0100, Jean-Yves Migeon wrote:
> [...]
> Note that this choice was influenced because I wanted my patch to
> remain close to the one in OpenBSD. Besides, the wk struct is unused
> in the current code so allocating it to free it a few seconds later
> is just extra overhead for no real purpose.

wk is unused by the caller, but workqueue_enqueue() will still write
to it (it's used to maintain the queue, precisely). That's
why you have to wait for a wk to complete before enqueuing it again.

In the way it's used here, there could be a problem when multiple bnx(4)
instances are present (all instances uses the same wk). Because of
the worqueue(9) implementation (and that's not in the specifications) it
works, because it internally uses a simple list and as there's only one
work per workqueue, the next pointer is always NULL (so it's not a problem
to share the same wk by different workqueues). But it's a side effect
of implementation; it's it's changed to use e.g. a circular list,
things will break badly.
I think the correct thing to do would be to have the wk allocated in the
softc.

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