Subject: Re: workqueue(9), per-CPU queues (part 2)
To: None <rmind@NetBSD.org>
From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
List: tech-kern
Date: 07/25/2007 21:14:10
> Hello,
> here is the patch [1] which makes workqueue(9) structures more CPU-cache
> friendly, as suggested by Andrew, and implements MI CPU ID, which could be
> used as an index, as discussed previously. Also, there are fixes for other
> discussed problems.
> Currently, WQ_PERCPU flag could be used only after all secondary CPUs starts.

how about having a patchable variable max_cpu_seqid?

> For now, it would be OK, but this should be changed in a future. There should
> be an alternative for CPU_INFO_ITERATOR and CPU_INFO_FOREACH too,
> thought - now it looks weird, from the view of dynamism.
> 
> Please review.
> 
> [1]. http://www.netbsd.org/~rmind/subr_workqueue.diff
> 
> -- 
> Best regards,
> Mindaugas
> www.NetBSD.org

looks good to me.

- why name the kthread with curcpu's id in the case of ci==NULL?

- i personally prefer more descrictive names.
 eg. cpu_seqid(ci), CACHE_LINE_SIZE

- why u_int rather than cpuid_t?

- is wq_size necessary?  can't it be just recalculated?

- q_ci doesn't seem necessary.

- how about:
	#if defined(MULTIPROCESSOR)
	#define	WQ_CACHE_LINE_SIZE	CACHE_LINE_SIZE
	#else /* defined(MULTIPROCESSOR) */
	#define	WQ_CACHE_LINE_SIZE	(ALIGNBYTES + 1)
	#endif /* defined(MULTIPROCESSOR) */

YAMAMOTO Takashi