[PATCH RFC] x86: fix confusing name of /proc/cpuinfo "ht" flag

[PATCH RFC] x86: fix confusing name of /proc/cpuinfo "ht" flag

[Bartlomiej Zolnierkiewicz]

"ht" flag indicates only ability to detect siblings not HT presence itself.

Inspired by:
http://www.codemonkey.org.uk/2009/11/10/common-hyperthreading-misconception/

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
It could be that there are some user-space programs depending on "ht"
flag so the patch is marked as RFC..

 arch/x86/include/asm/cpufeature.h |    4 ++--
 arch/x86/kernel/cpu/capflags.c    |    2 +-
 arch/x86/kernel/cpu/common.c      |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

Index: b/arch/x86/include/asm/cpufeature.h
===================================================================
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -42,7 +42,7 @@
 #define X86_FEATURE_XMM		(0*32+25) /* "sse" */
 #define X86_FEATURE_XMM2	(0*32+26) /* "sse2" */
 #define X86_FEATURE_SELFSNOOP	(0*32+27) /* "ss" CPU self snoop */
-#define X86_FEATURE_HT		(0*32+28) /* Hyper-Threading */
+#define X86_FEATURE_HT_DETECTION (0*32+28) /* Hyper-Threading detection */
 #define X86_FEATURE_ACC		(0*32+29) /* "tm" Automatic clock control */
 #define X86_FEATURE_IA64	(0*32+30) /* IA-64 processor */
 #define X86_FEATURE_PBE		(0*32+31) /* Pending Break Enable */
@@ -220,7 +220,7 @@ extern const char * const x86_power_flag
 #define cpu_has_xmm2		boot_cpu_has(X86_FEATURE_XMM2)
 #define cpu_has_xmm3		boot_cpu_has(X86_FEATURE_XMM3)
 #define cpu_has_aes		boot_cpu_has(X86_FEATURE_AES)
-#define cpu_has_ht		boot_cpu_has(X86_FEATURE_HT)
+#define cpu_has_ht_detection	boot_cpu_has(X86_FEATURE_HT_DETECTION)
 #define cpu_has_mp		boot_cpu_has(X86_FEATURE_MP)
 #define cpu_has_nx		boot_cpu_has(X86_FEATURE_NX)
 #define cpu_has_k6_mtrr		boot_cpu_has(X86_FEATURE_K6_MTRR)
Index: b/arch/x86/kernel/cpu/capflags.c
===================================================================
--- a/arch/x86/kernel/cpu/capflags.c
+++ b/arch/x86/kernel/cpu/capflags.c
@@ -27,7 +27,7 @@ const char * const x86_cap_flags[NCAPINT
 	[X86_FEATURE_XMM]                = "sse",
 	[X86_FEATURE_XMM2]               = "sse2",
 	[X86_FEATURE_SELFSNOOP]          = "ss",
-	[X86_FEATURE_HT]                 = "ht",
+	[X86_FEATURE_HT_DETECTION]       = "ht_detection",
 	[X86_FEATURE_ACC]                = "tm",
 	[X86_FEATURE_IA64]               = "ia64",
 	[X86_FEATURE_PBE]                = "pbe",
Index: b/arch/x86/kernel/cpu/common.c
===================================================================
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -433,7 +433,7 @@ void __cpuinit detect_ht(struct cpuinfo_
 	u32 eax, ebx, ecx, edx;
 	int index_msb, core_bits;
 
-	if (!cpu_has(c, X86_FEATURE_HT))
+	if (!cpu_has(c, X86_FEATURE_HT_DETECTION))
 		return;
 
 	if (cpu_has(c, X86_FEATURE_CMP_LEGACY))

Re: [PATCH RFC] x86: fix confusing name of /proc/cpuinfo "ht" flag

[Chris Friesen]

On 11/11/2009 02:34 PM, Bartlomiej Zolnierkiewicz wrote:
> 
> "ht" flag indicates only ability to detect siblings not HT presence itself.
> 
> Inspired by:
> http://www.codemonkey.org.uk/2009/11/10/common-hyperthreading-misconception/
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> ---
> It could be that there are some user-space programs depending on "ht"
> flag so the patch is marked as RFC..

If a cpu is capable of ht but doesn't have any siblings, do we actually
care?

If many people consider the "ht" flag to indicate that hyperthreading is
actually available, what about instead changing the meaning of the "ht"
flag to indicate that there actually are siblings?

Chris

Re: [PATCH RFC] x86: fix confusing name of /proc/cpuinfo "ht" flag

[Jeff Garzik]

On 11/11/2009 03:34 PM, Bartlomiej Zolnierkiewicz wrote:
>
> "ht" flag indicates only ability to detect siblings not HT presence itself.
>
> Inspired by:
> http://www.codemonkey.org.uk/2009/11/10/common-hyperthreading-misconception/
>
> Signed-off-by: Bartlomiej Zolnierkiewicz<bzolnier@gmail.com>
> ---
> It could be that there are some user-space programs depending on "ht"
> flag so the patch is marked as RFC..

I think that is the only problem with this patch -- it is an ABI change. 
  See as it's been this way for years, a comment is probably the best 
you can do...

	Jeff

Re: [PATCH RFC] x86: fix confusing name of /proc/cpuinfo "ht" flag

[Ingo Molnar]

* Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:

> "ht" flag indicates only ability to detect siblings not HT presence 
> itself.
> 
> Inspired by:
> http://www.codemonkey.org.uk/2009/11/10/common-hyperthreading-misconception/

i think instead of changing 'ht' to 'ht_detect', it would be even more 
intuitive to only expose the 'ht' flag if the ht-detect capability is 
there _and_ if the number of siblings is 2 or more.

That way we dont change it - we just 'hide' the 'ht' string on the 
category of systems that can enumerate HT via the CPUID but dont 
actually have HyperThreading.

Hm?

	Ingo

Re: [PATCH RFC] x86: fix confusing name of /proc/cpuinfo "ht" flag

[H. Peter Anvin]

On 11/11/2009 10:59 PM, Ingo Molnar wrote:
> 
> * Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
> 
>> "ht" flag indicates only ability to detect siblings not HT presence 
>> itself.
>>
>> Inspired by:
>> http://www.codemonkey.org.uk/2009/11/10/common-hyperthreading-misconception/
> 
> i think instead of changing 'ht' to 'ht_detect', it would be even more 
> intuitive to only expose the 'ht' flag if the ht-detect capability is 
> there _and_ if the number of siblings is 2 or more.
> 
> That way we dont change it - we just 'hide' the 'ht' string on the 
> category of systems that can enumerate HT via the CPUID but dont 
> actually have HyperThreading.
> 

/proc/cpuinfo is a user-space visible ABI.  Changing it is bad chicken.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH RFC] x86: fix confusing name of /proc/cpuinfo "ht" flag

[Ingo Molnar]

* H. Peter Anvin <hpa@zytor.com> wrote:

> On 11/11/2009 10:59 PM, Ingo Molnar wrote:
> > 
> > * Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
> > 
> >> "ht" flag indicates only ability to detect siblings not HT presence 
> >> itself.
> >>
> >> Inspired by:
> >> http://www.codemonkey.org.uk/2009/11/10/common-hyperthreading-misconception/
> > 
> > i think instead of changing 'ht' to 'ht_detect', it would be even 
> > more intuitive to only expose the 'ht' flag if the ht-detect 
> > capability is there _and_ if the number of siblings is 2 or more.
> > 
> > That way we dont change it - we just 'hide' the 'ht' string on the 
> > category of systems that can enumerate HT via the CPUID but dont 
> > actually have HyperThreading.
> 
> /proc/cpuinfo is a user-space visible ABI.  Changing it is bad 
> chicken.

Almost ... something is only an ABI if an actual application relies on 
the 'ht' flag being there on non-hyperthreading CPUs. We dont know 
whether there's any, but the likelyhood looks pretty low. Could park it 
a branch for some time to see whether anything trips up.

	Ingo

Re: [PATCH RFC] x86: fix confusing name of /proc/cpuinfo "ht" flag

[H. Peter Anvin]

On 11/12/2009 12:13 AM, Ingo Molnar wrote:
>>
>> /proc/cpuinfo is a user-space visible ABI.  Changing it is bad 
>> chicken.
> 
> Almost ... something is only an ABI if an actual application relies on 
> the 'ht' flag being there on non-hyperthreading CPUs. We dont know 
> whether there's any, but the likelyhood looks pretty low. Could park it 
> a branch for some time to see whether anything trips up.
> 

No, please.  We have had people who have "helpfully" tried to change
/proc/cpuinfo in the past, and what we find is that application
developers don't tell us -- they grumble among themselves and put out a
new release saying "fixes breakage introduced by Linux ..."

It's an ABI.  Keep it stable, please.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH RFC] x86: fix confusing name of /proc/cpuinfo "ht" flag

[Alexander Clouter]

Chris Friesen <cfriesen@nortel.com> wrote:
>> 
>> "ht" flag indicates only ability to detect siblings not HT presence itself.
>> 
>> Inspired by:
>> http://www.codemonkey.org.uk/2009/11/10/common-hyperthreading-misconception/
>> 
>> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>> ---
>> It could be that there are some user-space programs depending on "ht"
>> flag so the patch is marked as RFC..
> 
> If a cpu is capable of ht but doesn't have any siblings, do we actually
> care?
> 
I have a Core2 Duo U7600 with siblings set to 2 and the 'ht' flag and I 
know the CPU in my lapdog does not actually support hyperthreading.

> If many people consider the "ht" flag to indicate that hyperthreading is
> actually available, what about instead changing the meaning of the "ht"
> flag to indicate that there actually are siblings?
> 
Could be determined surely by "grep siblings /proc/cpuinfo"?

Cheers

-- 
Alexander Clouter
.sigmonster says: Old musicians never die, they just decompose.

Re: [PATCH RFC] x86: fix confusing name of /proc/cpuinfo "ht" flag

[Ingo Molnar]

* H. Peter Anvin <hpa@zytor.com> wrote:

> On 11/12/2009 12:13 AM, Ingo Molnar wrote:
> >>
> >> /proc/cpuinfo is a user-space visible ABI.  Changing it is bad 
> >> chicken.
> > 
> > Almost ... something is only an ABI if an actual application relies on 
> > the 'ht' flag being there on non-hyperthreading CPUs. We dont know 
> > whether there's any, but the likelyhood looks pretty low. Could park it 
> > a branch for some time to see whether anything trips up.
> > 
> 
> No, please.  We have had people who have "helpfully" tried to change 
> /proc/cpuinfo in the past, and what we find is that application 
> developers don't tell us -- they grumble among themselves and put out 
> a new release saying "fixes breakage introduced by Linux ..."
> 
> It's an ABI.  Keep it stable, please.

That's generally true, but i'm not suggesting that: i'm suggesting to 
_clear_ the HT flag from the cpufeatures if there's only one sibling. 
It's meaningless in that case and as the link quoted by the original 
patch shows many people are confused by that.

I have such a box so i can test it. (but i dont expect any problems)

	Ingo

Re: [PATCH RFC] x86: fix confusing name of /proc/cpuinfo "ht" flag

[Dave Jones]

On Thu, Nov 12, 2009 at 06:59:08PM +0100, Ingo Molnar wrote:
 > > It's an ABI.  Keep it stable, please.
 > 
 > That's generally true, but i'm not suggesting that: i'm suggesting to 
 > _clear_ the HT flag from the cpufeatures if there's only one sibling. 
 > It's meaningless in that case and as the link quoted by the original 
 > patch shows many people are confused by that.
 > 
 > I have such a box so i can test it. (but i dont expect any problems)

I agree that it's an ABI change, but any software depending on its current
state has to implement a fallback for the case where 'ht' isn't present anyway
unless there's some program that only runs on ht capable hardware, which
sounds just crazy.

The only potential for breakage that I can see is that code that is tuned
to be run in the HT case will stop running in cases where it shouldn't.
Which sounds like a positive thing to me.

	Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH RFC] x86: fix confusing name of /proc/cpuinfo "ht" flag

[H. Peter Anvin]

On 11/12/2009 10:37 AM, Dave Jones wrote:
> On Thu, Nov 12, 2009 at 06:59:08PM +0100, Ingo Molnar wrote:
>  > > It's an ABI.  Keep it stable, please.
>  > 
>  > That's generally true, but i'm not suggesting that: i'm suggesting to 
>  > _clear_ the HT flag from the cpufeatures if there's only one sibling. 
>  > It's meaningless in that case and as the link quoted by the original 
>  > patch shows many people are confused by that.
>  > 
>  > I have such a box so i can test it. (but i dont expect any problems)
> 
> I agree that it's an ABI change, but any software depending on its current
> state has to implement a fallback for the case where 'ht' isn't present anyway
> unless there's some program that only runs on ht capable hardware, which
> sounds just crazy.
> 
> The only potential for breakage that I can see is that code that is tuned
> to be run in the HT case will stop running in cases where it shouldn't.
> Which sounds like a positive thing to me.

The most likely breakage would be some stupid licensing scheme.

The other aspect is that we as much as possible have tried to stay to
the hardware-documented names of these strings.  Inventing new strings
is generally a bad idea.

	-hpa

Re: [PATCH RFC] x86: fix confusing name of /proc/cpuinfo "ht" flag

[H. Peter Anvin]

On 11/12/2009 09:59 AM, Ingo Molnar wrote:
>>
>> No, please.  We have had people who have "helpfully" tried to change 
>> /proc/cpuinfo in the past, and what we find is that application 
>> developers don't tell us -- they grumble among themselves and put out 
>> a new release saying "fixes breakage introduced by Linux ..."
>>
>> It's an ABI.  Keep it stable, please.
> 
> That's generally true, but i'm not suggesting that: i'm suggesting to 
> _clear_ the HT flag from the cpufeatures if there's only one sibling. 
> It's meaningless in that case and as the link quoted by the original 
> patch shows many people are confused by that.
> 
> I have such a box so i can test it. (but i dont expect any problems)
> 

Ah, okay, that makes more sense.  Someone else in the tread wanted to
rename the flag.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH RFC] x86: fix confusing name of /proc/cpuinfo "ht" flag

[Ingo Molnar]

* H. Peter Anvin <hpa@zytor.com> wrote:

> On 11/12/2009 10:37 AM, Dave Jones wrote:
> > On Thu, Nov 12, 2009 at 06:59:08PM +0100, Ingo Molnar wrote:
> >  > > It's an ABI.  Keep it stable, please.
> >  > 
> >  > That's generally true, but i'm not suggesting that: i'm suggesting to 
> >  > _clear_ the HT flag from the cpufeatures if there's only one sibling. 
> >  > It's meaningless in that case and as the link quoted by the original 
> >  > patch shows many people are confused by that.
> >  > 
> >  > I have such a box so i can test it. (but i dont expect any problems)
> > 
> > I agree that it's an ABI change, but any software depending on its current
> > state has to implement a fallback for the case where 'ht' isn't present anyway
> > unless there's some program that only runs on ht capable hardware, which
> > sounds just crazy.
> > 
> > The only potential for breakage that I can see is that code that is tuned
> > to be run in the HT case will stop running in cases where it shouldn't.
> > Which sounds like a positive thing to me.
> 
> The most likely breakage would be some stupid licensing scheme.
> 
> The other aspect is that we as much as possible have tried to stay to 
> the hardware-documented names of these strings.  Inventing new strings 
> is generally a bad idea.

Agreed - and we rejected such patches a couple of times in the past and 
for good reasons. Some /proc details are rarely used by apps (so they 
are no real ABIs) but cpuinfo is frequently parsed.

Clearing the ht flag on non-hyperthreading CPUs would be a limited 
quirk/fix in essence applicable to a relatively narrow range of CPUs - 
and easily undone, should it cause any problems. So if Bart wants to 
take a stab at that it would be a nice solution to the problem at hand 
...

	Ingo

Re: [PATCH RFC] x86: fix confusing name of /proc/cpuinfo "ht" flag

[Bartlomiej Zolnierkiewicz]

On Friday 13 November 2009 08:42:48 Ingo Molnar wrote:
> 
> * H. Peter Anvin <hpa@zytor.com> wrote:
> 
> > On 11/12/2009 10:37 AM, Dave Jones wrote:
> > > On Thu, Nov 12, 2009 at 06:59:08PM +0100, Ingo Molnar wrote:
> > >  > > It's an ABI.  Keep it stable, please.
> > >  > 
> > >  > That's generally true, but i'm not suggesting that: i'm suggesting to 
> > >  > _clear_ the HT flag from the cpufeatures if there's only one sibling. 
> > >  > It's meaningless in that case and as the link quoted by the original 
> > >  > patch shows many people are confused by that.
> > >  > 
> > >  > I have such a box so i can test it. (but i dont expect any problems)
> > > 
> > > I agree that it's an ABI change, but any software depending on its current
> > > state has to implement a fallback for the case where 'ht' isn't present anyway
> > > unless there's some program that only runs on ht capable hardware, which
> > > sounds just crazy.
> > > 
> > > The only potential for breakage that I can see is that code that is tuned
> > > to be run in the HT case will stop running in cases where it shouldn't.
> > > Which sounds like a positive thing to me.
> > 
> > The most likely breakage would be some stupid licensing scheme.
> > 
> > The other aspect is that we as much as possible have tried to stay to 
> > the hardware-documented names of these strings.  Inventing new strings 
> > is generally a bad idea.
> 
> Agreed - and we rejected such patches a couple of times in the past and 
> for good reasons. Some /proc details are rarely used by apps (so they 
> are no real ABIs) but cpuinfo is frequently parsed.
> 
> Clearing the ht flag on non-hyperthreading CPUs would be a limited 
> quirk/fix in essence applicable to a relatively narrow range of CPUs - 
> and easily undone, should it cause any problems. So if Bart wants to 
> take a stab at that it would be a nice solution to the problem at hand 
> ...

I'm rather busy with other/real stuff so if anybody wants to beat me
to making the proper quirk just feel free to do it.

-- 
Bartlomiej Zolnierkiewicz