Subject: Re: Patches for EST and SMP
To: None <tech-kern@netbsd.org>
From: Andrew Doran <ad@netbsd.org>
List: tech-kern
Date: 03/18/2007 17:54:57
Hi,

On Sun, Mar 18, 2007 at 11:10:00AM +0100, Juraj Hercek wrote:

> >On Saturday 17 March 2007, Juan RP wrote:

> >http://www.netbsd.org/~xtraeme/msr_ipi_handlers.diff
> >http://www.netbsd.org/~xtraeme/est_smp.diff
> >http://www.netbsd.org/~xtraeme/powernow_k8_smp.diff

Looks good to me! I have a couple of comments on the patch for the MSR IPI
handlers. I realise it's probably just for debugging, but you can't safely
call printf() from an IPI handler, since the the IPI handlers run at a
higher interrupt priority level than the tty subsystem. Also my contribution
was fairly small so I think it's not worth putting my name in the copyright.

> I've looked at the changes to est.c and seen an #ifdefs for 
> multiprocessor support. Wouldn't be possible to hide it behind the 
> wrmsr/rdmsr functions? (I don't know if it is/isn't against some policy 
> or whatever - I'm still newbie regarding netbsd kernel...).

Personally, I would drop the #ifdef MULTIPROCESSOR and always include the
code. As time goes on there will be less and less difference between kernels
compiled with options MULTIPROCESSOR and without. What I'd like to do for
x86 eventually is nuke the option and patch out the expensive operations at
runtime (memory barriers, lock prefixes and the kernel_lock).

Also regarding the PENTIUM4_ONDEMAND_CLOCKMOD, I don't agree with phone@. I
doubt it adds a lot of code overhead - I would be inclined to always include
it or at least have one central option that enables CPU / frequency control.

Cheers,
Andrew