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