tech-net archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

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

On Sun, 26 Feb 2012 22:05:59 +0100, Manuel Bouyer wrote:
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

Yep, you are right. I just had a look at workqueue(9) implementation this morning, and multiple bnx(4) instances would badly break with anything besides SIMPLEQ.

Having a wk allocated in sc is the way to go.

Jean-Yves Migeon

Home | Main Index | Thread Index | Old Index