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/26/2007 08:35:58
> yamt@mwd.biglobe.ne.jp (YAMAMOTO Takashi) wrote:
> > > Currently, WQ_PERCPU flag could be used only after all secondary CPUs
> > > starts.
> > 
> > how about having a patchable variable max_cpu_seqid?
> 
> I am not sure what would you want to achieve with patching.
> Can you explain more?

it simplifies per-cpu data allocations before cpu attach,
while you still don't need to recompile a kernel for a particular machine.
i guess it can even be a kernel boot parameter for some ports.

reallocation on cpu hotplug events would be cool, but it's likely very
complicated for subsystems used by memory allocation.  eg. per-cpu pool_cache.
i'm not sure if it's worth to do it.

> > - why name the kthread with curcpu's id in the case of ci==NULL?
> > 
> 
> Changed to 0. But LWP might migrate..

how about just dropping "/%d" part for the unbound case?

> > - i personally prefer more descrictive names.
> >  eg. cpu_seqid(ci), CACHE_LINE_SIZE
> 
> Do you want to have an inline function?
> #define	ci_seqid		ci_data.cpu_seqid
> Are these names OK?

yes and yes.

> > - why u_int rather than cpuid_t?
> > 
> 
> Changed to cpuid_t. This should not be confused with MD cpuid.

i don't think u_int makes it much less confusing.
if you care, invent a new type like cpu_seqid_t.

> > - is wq_size necessary?  can't it be just recalculated?
> > 
> 
> Yes, it can be recalculated. But I would like to get settled:
>   a) we do not care about ncpu changes (attaching/dettaching)
>   b) we might start preparing for this, eg. save ncpu (or wq_size) value
> So just take a) for now?

i think it's reasonable to make attaching/detaching callbacks provide
both of old and new values of ncpu.

> > - 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) */
> 
> Reasonable. I guess there could be more such usage in the feature - perhaps
> it would be reasonable to invent some global constant?

probably.

> Any objections to move CACHE_LINE_SIZE to cpu.h?

i prefer param.h, but i don't care much.

YAMAMOTO Takashi