Re: [patch 13/14] x86_64: Use common functions in cluster and physflat mode
On Fri, 10 Sep 2005, Andi Kleen wrote: > On Fri, Sep 09, 2005 at 10:07:28AM -0700, Zwane Mwaikambo wrote: > > On Tue, 6 Sep 2005, Ashok Raj wrote: > > > > > On Tue, Sep 06, 2005 at 01:16:28AM +0200, Andi Kleen wrote: > > > > On Sat, Sep 03, 2005 at 02:33:30PM -0700, [EMAIL PROTECTED] wrote: > > > > > > > > > > From: Ashok Raj <[EMAIL PROTECTED]> > > > > > > > > > > Newly introduced physflat_* shares way too much with cluster with > > > > > only a very > > > > > differences. So we introduce some common functions in that can be > > > > > reused in > > > > > both cases. > > > > On a slightly different topic, how come we're using physflat for hotplug > > cpu? > > The original idea was to always use physflat mode for hotplug because > that does all the sequencing stuff and avoids the shortcut races. > But then Ashok decided it was better to add more ifdefs to flat mode > instead and I gave up protesting at some point. Ok so you wanted to segragate them, i can understand that, but didn't we have a version which worked around the races by doing the same thing, hotplug or not? Is this the one where you weren't pleased with the supposed execution penalty? Thanks, Zwane - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 13/14] x86_64: Use common functions in cluster and physflat mode
On Fri, Sep 09, 2005 at 10:07:28AM -0700, Zwane Mwaikambo wrote: > On Tue, 6 Sep 2005, Ashok Raj wrote: > > > On Tue, Sep 06, 2005 at 01:16:28AM +0200, Andi Kleen wrote: > > > On Sat, Sep 03, 2005 at 02:33:30PM -0700, [EMAIL PROTECTED] wrote: > > > > > > > > From: Ashok Raj <[EMAIL PROTECTED]> > > > > > > > > Newly introduced physflat_* shares way too much with cluster with only > > > > a very > > > > differences. So we introduce some common functions in that can be > > > > reused in > > > > both cases. > > On a slightly different topic, how come we're using physflat for hotplug > cpu? The original idea was to always use physflat mode for hotplug because that does all the sequencing stuff and avoids the shortcut races. But then Ashok decided it was better to add more ifdefs to flat mode instead and I gave up protesting at some point. -Andi > > -#ifndef CONFIG_CPU_HOTPLUG > /* In the CPU hotplug case we cannot use broadcast mode > because that opens a race when a CPU is removed. > -Stay at physflat mode in this case. > -It is bad to do this unconditionally though. Once > -we have ACPI platform support for CPU hotplug > -we should detect hotplug capablity from ACPI tables and > -only do this when really needed. -AK */ > +Stay at physflat mode in this case. - AK */ > +#ifdef CONFIG_HOTPLUG_CPU > if (num_cpus <= 8) > genapic = _flat; > > Thanks, > Zwane > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 13/14] x86_64: Use common functions in cluster and physflat mode
On Fri, Sep 09, 2005 at 10:07:28AM -0700, Zwane Mwaikambo wrote: > On a slightly different topic, how come we're using physflat for hotplug > cpu? > > -#ifndef CONFIG_CPU_HOTPLUG > /* In the CPU hotplug case we cannot use broadcast mode > because that opens a race when a CPU is removed. > -Stay at physflat mode in this case. > -It is bad to do this unconditionally though. Once > -we have ACPI platform support for CPU hotplug > -we should detect hotplug capablity from ACPI tables and > -only do this when really needed. -AK */ > +Stay at physflat mode in this case. - AK */ > +#ifdef CONFIG_HOTPLUG_CPU > if (num_cpus <= 8) > genapic = _flat; What you say was true before this patch, (Although now that you point out i realize the ifdef CONFIG_HOTPLUG_CPU is not required). Think Andi is fixing this in his next drop to -mm* When physflat was introduced, it also switched to use physflat mode for #cpus <=8 when hotplug is enabled, since it doesnt use shortcuts and so is also safer (although slower). http://marc.theaimsgroup.com/?l=linux-kernel=112317686712929=2 The link above made using genapic_flat safer by using the flat_send_IPI_mask(), and hence i switched back to using logical flat mode when #cpus <=8, since that a little more efficient than the send_IPI_mask_sequence() used in physflat mode. In general we need flat_mode - #cpus <= 8 (Hotplug defined or not, so we use mask version for safety) physflat or cluster_mode when #cpus >8. If we choose physflat as default for #cpus <=8 (with hotplug) would make IPI performance worse, since it would do one cpu at a time, and requires 2 writes per cpu for each IPI v.s just 2 for a flat mode mask version of the API. -- Cheers, Ashok Raj - Open Source Technology Center - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 13/14] x86_64: Use common functions in cluster and physflat mode
On Tue, 6 Sep 2005, Ashok Raj wrote: > On Tue, Sep 06, 2005 at 01:16:28AM +0200, Andi Kleen wrote: > > On Sat, Sep 03, 2005 at 02:33:30PM -0700, [EMAIL PROTECTED] wrote: > > > > > > From: Ashok Raj <[EMAIL PROTECTED]> > > > > > > Newly introduced physflat_* shares way too much with cluster with only a > > > very > > > differences. So we introduce some common functions in that can be reused > > > in > > > both cases. On a slightly different topic, how come we're using physflat for hotplug cpu? -#ifndef CONFIG_CPU_HOTPLUG /* In the CPU hotplug case we cannot use broadcast mode because that opens a race when a CPU is removed. - Stay at physflat mode in this case. - It is bad to do this unconditionally though. Once - we have ACPI platform support for CPU hotplug - we should detect hotplug capablity from ACPI tables and - only do this when really needed. -AK */ + Stay at physflat mode in this case. - AK */ +#ifdef CONFIG_HOTPLUG_CPU if (num_cpus <= 8) genapic = _flat; Thanks, Zwane - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 13/14] x86_64: Use common functions in cluster and physflat mode
On Tue, 6 Sep 2005, Ashok Raj wrote: On Tue, Sep 06, 2005 at 01:16:28AM +0200, Andi Kleen wrote: On Sat, Sep 03, 2005 at 02:33:30PM -0700, [EMAIL PROTECTED] wrote: From: Ashok Raj [EMAIL PROTECTED] Newly introduced physflat_* shares way too much with cluster with only a very differences. So we introduce some common functions in that can be reused in both cases. On a slightly different topic, how come we're using physflat for hotplug cpu? -#ifndef CONFIG_CPU_HOTPLUG /* In the CPU hotplug case we cannot use broadcast mode because that opens a race when a CPU is removed. - Stay at physflat mode in this case. - It is bad to do this unconditionally though. Once - we have ACPI platform support for CPU hotplug - we should detect hotplug capablity from ACPI tables and - only do this when really needed. -AK */ + Stay at physflat mode in this case. - AK */ +#ifdef CONFIG_HOTPLUG_CPU if (num_cpus = 8) genapic = apic_flat; Thanks, Zwane - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 13/14] x86_64: Use common functions in cluster and physflat mode
On Fri, Sep 09, 2005 at 10:07:28AM -0700, Zwane Mwaikambo wrote: On a slightly different topic, how come we're using physflat for hotplug cpu? -#ifndef CONFIG_CPU_HOTPLUG /* In the CPU hotplug case we cannot use broadcast mode because that opens a race when a CPU is removed. -Stay at physflat mode in this case. -It is bad to do this unconditionally though. Once -we have ACPI platform support for CPU hotplug -we should detect hotplug capablity from ACPI tables and -only do this when really needed. -AK */ +Stay at physflat mode in this case. - AK */ +#ifdef CONFIG_HOTPLUG_CPU if (num_cpus = 8) genapic = apic_flat; What you say was true before this patch, (Although now that you point out i realize the ifdef CONFIG_HOTPLUG_CPU is not required). Think Andi is fixing this in his next drop to -mm* When physflat was introduced, it also switched to use physflat mode for #cpus =8 when hotplug is enabled, since it doesnt use shortcuts and so is also safer (although slower). http://marc.theaimsgroup.com/?l=linux-kernelm=112317686712929w=2 The link above made using genapic_flat safer by using the flat_send_IPI_mask(), and hence i switched back to using logical flat mode when #cpus =8, since that a little more efficient than the send_IPI_mask_sequence() used in physflat mode. In general we need flat_mode - #cpus = 8 (Hotplug defined or not, so we use mask version for safety) physflat or cluster_mode when #cpus 8. If we choose physflat as default for #cpus =8 (with hotplug) would make IPI performance worse, since it would do one cpu at a time, and requires 2 writes per cpu for each IPI v.s just 2 for a flat mode mask version of the API. -- Cheers, Ashok Raj - Open Source Technology Center - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 13/14] x86_64: Use common functions in cluster and physflat mode
On Fri, Sep 09, 2005 at 10:07:28AM -0700, Zwane Mwaikambo wrote: On Tue, 6 Sep 2005, Ashok Raj wrote: On Tue, Sep 06, 2005 at 01:16:28AM +0200, Andi Kleen wrote: On Sat, Sep 03, 2005 at 02:33:30PM -0700, [EMAIL PROTECTED] wrote: From: Ashok Raj [EMAIL PROTECTED] Newly introduced physflat_* shares way too much with cluster with only a very differences. So we introduce some common functions in that can be reused in both cases. On a slightly different topic, how come we're using physflat for hotplug cpu? The original idea was to always use physflat mode for hotplug because that does all the sequencing stuff and avoids the shortcut races. But then Ashok decided it was better to add more ifdefs to flat mode instead and I gave up protesting at some point. -Andi -#ifndef CONFIG_CPU_HOTPLUG /* In the CPU hotplug case we cannot use broadcast mode because that opens a race when a CPU is removed. -Stay at physflat mode in this case. -It is bad to do this unconditionally though. Once -we have ACPI platform support for CPU hotplug -we should detect hotplug capablity from ACPI tables and -only do this when really needed. -AK */ +Stay at physflat mode in this case. - AK */ +#ifdef CONFIG_HOTPLUG_CPU if (num_cpus = 8) genapic = apic_flat; Thanks, Zwane - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 13/14] x86_64: Use common functions in cluster and physflat mode
On Fri, 10 Sep 2005, Andi Kleen wrote: On Fri, Sep 09, 2005 at 10:07:28AM -0700, Zwane Mwaikambo wrote: On Tue, 6 Sep 2005, Ashok Raj wrote: On Tue, Sep 06, 2005 at 01:16:28AM +0200, Andi Kleen wrote: On Sat, Sep 03, 2005 at 02:33:30PM -0700, [EMAIL PROTECTED] wrote: From: Ashok Raj [EMAIL PROTECTED] Newly introduced physflat_* shares way too much with cluster with only a very differences. So we introduce some common functions in that can be reused in both cases. On a slightly different topic, how come we're using physflat for hotplug cpu? The original idea was to always use physflat mode for hotplug because that does all the sequencing stuff and avoids the shortcut races. But then Ashok decided it was better to add more ifdefs to flat mode instead and I gave up protesting at some point. Ok so you wanted to segragate them, i can understand that, but didn't we have a version which worked around the races by doing the same thing, hotplug or not? Is this the one where you weren't pleased with the supposed execution penalty? Thanks, Zwane - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 13/14] x86_64: Use common functions in cluster and physflat mode
On Tue, Sep 06, 2005 at 01:16:28AM +0200, Andi Kleen wrote: > On Sat, Sep 03, 2005 at 02:33:30PM -0700, [EMAIL PROTECTED] wrote: > > > > From: Ashok Raj <[EMAIL PROTECTED]> > > > > Newly introduced physflat_* shares way too much with cluster with only a > > very > > differences. So we introduce some common functions in that can be reused in > > both cases. > > > > In addition the following are also fixed. > > - Use of non-existent CONFIG_CPU_HOTPLUG option renamed to actual one in > > use. > > - Removed comment that ACPI would provide a way to select this dynamically > > since ACPI_CONFIG_HOTPLUG_CPU already exists that indicates platform > > support > > for hotplug via ACPI. In addition CONFIG_HOTPLUG_CPU only indicates > > logical > > offline/online which is even used by Suspend/Resume folks where the same > > support (for no-broadcast) is required. > > > (hmm did I reply to that? I think I did but my mailer seems to have > lost the r flag. My apologies if it's a duplicate) > > I didn't like that one because it makes the code less readable than > before imho. I did a separate patch for the CPU_HOTPLUG typo. The code is less readable? Now iam confused. Attached the link to patch below to refresh your memory. http://marc.theaimsgroup.com/?l=linux-kernel=112293577309653=2 diffstat would show we have fewer lines ~40 less lines of code. physflat basicaly copied/cloned some useful code in cluster and some from flat mode genapic code. I would have consolidated the code in the first place when you put the physflat mode. Again this was just my habit, cant step over code bloat and duplication. Which part of the code is unreadable to you? If you are happy with just renamed functions with copied body of the code which is what physflat did, thats fine. I was just puzzeled at the convoluted and less readable part of the code. If there is something you like to point out, i would be happy to fix it.. or you can if you prefer it that way. -- Cheers, Ashok Raj - Open Source Technology Center - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 13/14] x86_64: Use common functions in cluster and physflat mode
On Tue, Sep 06, 2005 at 01:16:28AM +0200, Andi Kleen wrote: On Sat, Sep 03, 2005 at 02:33:30PM -0700, [EMAIL PROTECTED] wrote: From: Ashok Raj [EMAIL PROTECTED] Newly introduced physflat_* shares way too much with cluster with only a very differences. So we introduce some common functions in that can be reused in both cases. In addition the following are also fixed. - Use of non-existent CONFIG_CPU_HOTPLUG option renamed to actual one in use. - Removed comment that ACPI would provide a way to select this dynamically since ACPI_CONFIG_HOTPLUG_CPU already exists that indicates platform support for hotplug via ACPI. In addition CONFIG_HOTPLUG_CPU only indicates logical offline/online which is even used by Suspend/Resume folks where the same support (for no-broadcast) is required. (hmm did I reply to that? I think I did but my mailer seems to have lost the r flag. My apologies if it's a duplicate) I didn't like that one because it makes the code less readable than before imho. I did a separate patch for the CPU_HOTPLUG typo. The code is less readable? Now iam confused. Attached the link to patch below to refresh your memory. http://marc.theaimsgroup.com/?l=linux-kernelm=112293577309653w=2 diffstat would show we have fewer lines ~40 less lines of code. physflat basicaly copied/cloned some useful code in cluster and some from flat mode genapic code. I would have consolidated the code in the first place when you put the physflat mode. Again this was just my habit, cant step over code bloat and duplication. Which part of the code is unreadable to you? If you are happy with just renamed functions with copied body of the code which is what physflat did, thats fine. I was just puzzeled at the convoluted and less readable part of the code. If there is something you like to point out, i would be happy to fix it.. or you can if you prefer it that way. -- Cheers, Ashok Raj - Open Source Technology Center - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
