Subject: Re: CVS commit: src/sys
To: Mindaugas Rasiukevicius <rmind@netbsd.org>
From: Andrew Doran <ad@netbsd.org>
List: tech-kern
Date: 07/13/2007 12:40:11
On Thu, Jul 12, 2007 at 08:39:57PM +0000, Mindaugas Rasiukevicius wrote:

> Log Message:
> Implementation of per-CPU work-queues support for workqueue(9) interface.
> WQ_PERCPU flag for workqueue and additional argument for workqueue_enqueue()
> to assign a CPU might be used. Notes:
>  - For now, the list is used for workqueue_queue, which is non-optimal,
>    and will be changed with array, where index would be CPU ID.
>  - The data structures should be changed to be cache-friendly.
> 
> Reviewed by: <yamt>, <tech-kern>

Thanks for working on this, however, you didn't note that it was also
reviewed by me, that I objected to traversing the linked list, and that I
was unhappy about the code being committed until that was fixed.

On a system with multiple CPUs and busy workqueues, traversing the linked
list will burn CPU cycles unnecessarily and cause thousands of unnecessary
cache transactions a second. Hardly the end of the world, but that
ineffiency is easily avoidable given a little time in front of the keyboard.

Also, I note that you ignored my suggestion that workqueue_enqueue() should
default to handing work to the local CPU. With your changes it defaults to
the last kthread created, which seems completely arbitrary.

What exactly are you trying to accomplish?

Andrew