Re: [patch 09/14] x86_64: Don't call enforce_max_cpus when hotplug is enabled

Re: [patch 09/14] x86_64: Don't call enforce_max_cpus when hotplug is enabled

[Ashok Raj]

Hi Andi

On Mon, Sep 05, 2005 at 06:48:21AM +0200, Andi Kleen wrote:
> On Sat, Sep 03, 2005 at 02:33:26PM -0700, akpm@osdl.org wrote:
> > 
> > From: Ashok Raj <ashok.raj@intel.com>
> > 
> > No need to enforce_max_cpus when hotplug code is enabled.  This nukes out
> > cpu_present_map and cpu_possible_map making it impossible to add new cpus in
> > the system.
> 
> I see the point, but the implementation is wrong. If anything
> we shouldn't do it neither for the !HOTPLUG_CPU case.Why did 
> you not do it unconditionally? 
> 
> I would prefer to keep the special cases for hotplug to be
> as narrow as possible.

Link to earlier discussion below

http://marc.theaimsgroup.com/?l=linux-kernel&m=112317327529855&w=2

I had suggested that we remove it completely in our discussion but i didnt
hear anything from you after that, so i thought that was acceptable.

You had suggested in that discussion that it would be better to add an 
option for startup. Iam opposed to adding any option, when we certainly know 
there are no users. Earlier based on your suggestion i added a startup
option to choose ipi broadcast mode, which you promptly removed when you
put physflat changes. I think its better to not add any option without
real need. Do you agree?

Please reply if you want me to remove the !HOTPLUG case which is my 
preference as well, and maybe while the memory is fresh, we can stick
with it this time when we are in the same page :-(

> 
> -Andi

-- 
Cheers,
Ashok Raj
- Open Source Technology Center

Re: [patch 09/14] x86_64: Don't call enforce_max_cpus when hotplug is enabled

[Andi Kleen]

On Tue, Sep 06, 2005 at 03:56:17PM -0700, Ashok Raj wrote:
> Hi Andi
> 
> On Mon, Sep 05, 2005 at 06:48:21AM +0200, Andi Kleen wrote:
> > On Sat, Sep 03, 2005 at 02:33:26PM -0700, akpm@osdl.org wrote:
> > > 
> > > From: Ashok Raj <ashok.raj@intel.com>
> > > 
> > > No need to enforce_max_cpus when hotplug code is enabled.  This nukes out
> > > cpu_present_map and cpu_possible_map making it impossible to add new cpus in
> > > the system.
> > 
> > I see the point, but the implementation is wrong. If anything
> > we shouldn't do it neither for the !HOTPLUG_CPU case.Why did 
> > you not do it unconditionally? 
> > 
> > I would prefer to keep the special cases for hotplug to be
> > as narrow as possible.
> 
> Link to earlier discussion below
> 
> http://marc.theaimsgroup.com/?l=linux-kernel&m=112317327529855&w=2
> 
> I had suggested that we remove it completely in our discussion but i didnt
> hear anything from you after that, so i thought that was acceptable.

Just because I don't follow up on everything doesn't necessarily
mean the patch is acceptable.

> 
> You had suggested in that discussion that it would be better to add an 
> option for startup. Iam opposed to adding any option, when we certainly know 

I suggested to auto detect it based on ACPI information. I don't 
think I ever wrote anything about an option.

If that is not possible it's better to always use the sequence mechanism.

> there are no users. Earlier based on your suggestion i added a startup
> option to choose ipi broadcast mode, which you promptly removed when you
> put physflat changes. I think its better to not add any option without
> real need. Do you agree?

Yes. 

> Please reply if you want me to remove the !HOTPLUG case which is my 
> preference as well, and maybe while the memory is fresh, we can stick
> with it this time when we are in the same page :-(

Yes, as I wrote earlier hotplug should be removed.

-Andi

P.S.: Don't bother sending me such "blame game" mails again. I will
just d them next time because they're a waste of time.

Re: [patch 09/14] x86_64: Don't call enforce_max_cpus when hotplug is enabled

[Ashok Raj]

On Wed, Sep 07, 2005 at 08:49:50AM +0200, Andi Kleen wrote:
> > 
> > You had suggested in that discussion that it would be better to add an 
> > option for startup. Iam opposed to adding any option, when we certainly know 
> 
> I suggested to auto detect it based on ACPI information. I don't 
> think I ever wrote anything about an option.
> 
> If that is not possible it's better to always use the sequence mechanism.

Using ACPI or any other method to choose broadcast or use mask version
of IPI in flat mode for <=8 cpus has no real value. I had posted a 
small stat program that showed using mask IPI provides same performance numbers.

We didnt choose that method only because there is no perf gain except code 
bloat. I dont understand putting all that complexity without any real merrit.

Moreover CONFIG_HOTPLUG_CPU does not imply physical CPU hotplug, which i had
tried to convey several times. 

It is important to understand that there is no just ONE RIGHT way
and that we consider alternatives for the right reason.

> 
> 
> P.S.: Don't bother sending me such "blame game" mails again. I will
> just d them next time because they're a waste of time.

Sorry Andi if you felt that way. I was trying to get some consistent feedback
and that you also consider and weight in what we explain instead of being
a one way street.

Certainly my intend was not to blame you, but to explain with clarity
so we dont end up reworking some trivial patches for a long time.

If you feel that way, i deeply apologize, and repeat, thats not my intend.
> 

-- 
Cheers,
Ashok Raj
- Open Source Technology Center