Subject: Re: Patches for EST and SMP
To: None <juan@xtrarom.org>
From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
List: tech-kern
Date: 03/17/2007 16:47:44
> Moving to tech-kern only.
> 
> On Saturday 17 March 2007, YAMAMOTO Takashi wrote:
> 
> > if something behaves in unexpected ways, please investigate why it does.
> > please don't commit the code which you don't understand.
> > to me, the iteration seems completely bogus.
> 
> x86_broadcast_ipi() doesn't seem to get the primary cpu, that's why I added 
> the second iter.
> 
> Why don't you explain the things are there wrong? I'm not a MASTER of
> UNIVERSE like you.

because it isn't the first time and i already know how it's hard to
explain you what's wrong...
and, actually, i explained what's wrong here.  ie. the iteration is
completely bogus.

you don't need to be a MASTER of UNIVERSE, but you need to understand
relevant code before commiting something.  don't you agree?

you want to use ipi in the first place because you want to run these
operations on each cpus.  it doesn't make sense unless the callback
only operates the cpu on which it's running.

> > you did it because you thought it was necessary for some reasons, didn't
> > you?
> 
> Andrew told me that it's necessary to run the ipi handler in the primary
> cpu, this is what I'm doing.

i guess he told you about a cpu calling x86_broadcast_ipi,
not a CPU_IS_PRIMARY cpu.

> I iter over cii, and ci points to the primary cpu; I assign ci->ci_msr_rvalue
> the value of the msr_type.

so?  do you mean a cpu on which msr_cpu_broadcast_read is running
should be a primary cpu, in the sense of CPU_IS_PRIMARY?  if so, why?

> > > > - what's the point of ci_msr_rvalue?
> > >
> > > Just to know what's the returned value of the MSR in cpuN.
> >
> > to me, no one in your patch seems to use it in a meaningful way.
> 
> Erm, why? ci_msr_rvalue could have a different value of the MSR,
> it's necessary to know that it has the same value.

why?  i don't know.  it's your code.
please grep ci_msr_rvalue in your patch and see how they are used.

YAMAMOTO Takashi