2007-05-09 20:04:05

by Pete Zaitcev

[permalink] [raw]
Subject: Andi, you broke my laptop :-)

Hi, Andi:

The attached patch (actually, git show output) makes my Dell 1501 to hang
on boot. Sorry, I have no clue why... The culprit is found with git bisect.
But yes, it's an AMD MK-36. I use an x86_64 kernel. It is 100% reproducible.

Cheers,
-- Pete

commit c5bcb5635a03da3158f121ae20ccbbf72b4fc62a
Author: Andi Kleen <[email protected]>
Date: Wed May 2 19:27:21 2007 +0200

[PATCH] x86: Use RDTSCP for synchronous get_cycles if possible

RDTSCP is already synchronous and doesn't need an explicit CPUID.
This is a little faster and more importantly avoids VMEXITs on Hypervisors.

Original patch from Joerg Roedel, but reworked by AK
Also includes miscompilation fix by Eric Biederman

Cc: "Joerg Roedel" <[email protected]>

Signed-off-by: Andi Kleen <[email protected]>

diff --git a/include/asm-i386/tsc.h b/include/asm-i386/tsc.h
index 0181f9d..3f3c1fa 100644
--- a/include/asm-i386/tsc.h
+++ b/include/asm-i386/tsc.h
@@ -38,6 +38,15 @@ static __always_inline cycles_t get_cycles_sync(void)
unsigned eax;

/*
+ * Use RDTSCP if possible; it is guaranteed to be synchronous
+ * and doesn't cause a VMEXIT on Hypervisors
+ */
+ alternative_io(ASM_NOP3, ".byte 0x0f,0x01,0xf9", X86_FEATURE_RDTSCP,
+ "=A" (ret), "0" (0ULL) : "ecx", "memory");
+ if (ret)
+ return ret;
+
+ /*
* Don't do an additional sync on CPUs where we know
* RDTSC is already synchronous:
*/


2007-05-10 10:13:16

by Benny Halevy

[permalink] [raw]
Subject: Re: Andi, you broke my laptop :-)

Andy, Pete, this patch also causes our test machines to hang hard during boot.
x86_64 smp kernel, single cpu Athlon 64 machine,
cpuinfo below.

Benny

$ cat /proc/cpuinfo
processor : 0
vendor_id : AuthenticAMD
cpu family : 15
model : 79
model name : AMD Athlon(tm) 64 Processor 3800+
stepping : 2
cpu MHz : 2412.397
cache size : 512 KB
fpu : yes
fpu_exception : yes
cpuid level : 1
wp : yes
flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 syscall nx mmxext fxsr_opt rdtscp lm 3dnowext 3dnow up pni cx16 lahf_lm svm cr8_legacy
bogomips : 4828.89
TLB size : 1024 4K pages
clflush size : 64
cache_alignment : 64
address sizes : 40 bits physical, 48 bits virtual
power management: ts fid vid ttp tm stc

Pete Zaitcev <[email protected]> wrote:
>
> Hi, Andi:
>
> The attached patch (actually, git show output) makes my Dell 1501 to hang
> on boot. Sorry, I have no clue why... The culprit is found with git bisect.
> But yes, it's an AMD MK-36. I use an x86_64 kernel. It is 100% reproducible.
>
> Cheers,
> -- Pete
>
> commit c5bcb5635a03da3158f121ae20ccbbf72b4fc62a
> Author: Andi Kleen <[email protected]>
> Date: Wed May 2 19:27:21 2007 +0200
>
> [PATCH] x86: Use RDTSCP for synchronous get_cycles if possible
>
> RDTSCP is already synchronous and doesn't need an explicit CPUID.
> This is a little faster and more importantly avoids VMEXITs on Hypervisors.
>
> Original patch from Joerg Roedel, but reworked by AK
> Also includes miscompilation fix by Eric Biederman
>
> Cc: "Joerg Roedel" <[email protected]>
>
> Signed-off-by: Andi Kleen <[email protected]>
>
> diff --git a/include/asm-i386/tsc.h b/include/asm-i386/tsc.h
> index 0181f9d..3f3c1fa 100644
> --- a/include/asm-i386/tsc.h
> +++ b/include/asm-i386/tsc.h
> @@ -38,6 +38,15 @@ static __always_inline cycles_t get_cycles_sync(void)
> unsigned eax;
>
> /*
> + * Use RDTSCP if possible; it is guaranteed to be synchronous
> + * and doesn't cause a VMEXIT on Hypervisors
> + */
> + alternative_io(ASM_NOP3, ".byte 0x0f,0x01,0xf9", X86_FEATURE_RDTSCP,
> + "=A" (ret), "0" (0ULL) : "ecx", "memory");
> + if (ret)
> + return ret;
> +
> + /*
> * Don't do an additional sync on CPUs where we know
> * RDTSC is already synchronous:
> */
> -
> 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/

2007-05-10 13:01:54

by Andi Kleen

[permalink] [raw]
Subject: Re: Andi, you broke my laptop :-)

On Wed, May 09, 2007 at 12:56:16PM -0700, Pete Zaitcev wrote:
> Hi, Andi:
>
> The attached patch (actually, git show output) makes my Dell 1501 to hang
> on boot. Sorry, I have no clue why... The culprit is found with git bisect.
> But yes, it's an AMD MK-36. I use an x86_64 kernel. It is 100% reproducible.

MK-36? Does it have SVM?

Anyways we previously had issues with this being miscompiled, but
I thought the latest patch should have been ok. What compiler do you use?

Can you send me a disassembly listing of arch/x86_64/kernel/time.o?

-Andi

2007-05-10 13:36:34

by Joerg Roedel

[permalink] [raw]
Subject: Re: Andi, you broke my laptop :-)

On Thu, May 10, 2007 at 03:01:44PM +0200, Andi Kleen wrote:
> On Wed, May 09, 2007 at 12:56:16PM -0700, Pete Zaitcev wrote:
> > Hi, Andi:
> >
> > The attached patch (actually, git show output) makes my Dell 1501 to hang
> > on boot. Sorry, I have no clue why... The culprit is found with git bisect.
> > But yes, it's an AMD MK-36. I use an x86_64 kernel. It is 100% reproducible.
>
> MK-36? Does it have SVM?
>
> Anyways we previously had issues with this being miscompiled, but
> I thought the latest patch should have been ok. What compiler do you use?
>
> Can you send me a disassembly listing of arch/x86_64/kernel/time.o?

I debugged this problem a bit and my compiler[1]interprets the =A
constraint as %rax instead of %edx:%eax on x86_64 which causes the
problem. The appended patch provides a workaround for this and fixed the
hang on my machine.

[1] gcc version 4.1.3 20070429 (prerelease) (Debian 4.1.2-5)

Cc: Andi Kleen <[email protected]>
Signed-off-by: Joerg Roedel <[email protected]>

--
| AMD Saxony Limited Liability Company & Co. KG
Operating | Wilschdorfer Landstr. 101, 01109 Dresden, Germany
System | Register Court Dresden: HRA 4896
Research | General Partner authorized to represent:
Center | AMD Saxony LLC (Wilmington, Delaware, US)
| General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy


Attachments:
(No filename) (1.39 kB)
x86_64_get_cycles_sync_fix.patch (1.86 kB)
Download all attachments

2007-05-10 14:19:53

by Andi Kleen

[permalink] [raw]
Subject: Re: Andi, you broke my laptop :-)

On Thu, May 10, 2007 at 03:35:56PM +0200, Joerg Roedel wrote:
> On Thu, May 10, 2007 at 03:01:44PM +0200, Andi Kleen wrote:
> > On Wed, May 09, 2007 at 12:56:16PM -0700, Pete Zaitcev wrote:
> > > Hi, Andi:
> > >
> > > The attached patch (actually, git show output) makes my Dell 1501 to hang
> > > on boot. Sorry, I have no clue why... The culprit is found with git bisect.
> > > But yes, it's an AMD MK-36. I use an x86_64 kernel. It is 100% reproducible.
> >
> > MK-36? Does it have SVM?
> >
> > Anyways we previously had issues with this being miscompiled, but
> > I thought the latest patch should have been ok. What compiler do you use?
> >
> > Can you send me a disassembly listing of arch/x86_64/kernel/time.o?
>
> I debugged this problem a bit and my compiler[1]interprets the =A
> constraint as %rax instead of %edx:%eax on x86_64 which causes the
> problem. The appended patch provides a workaround for this and fixed the
> hang on my machine.

Hmm yes now I can reproduce it too. I didn't see any hangs so i suppose
my (and that of most -mm tester's) compiled binary always happened to have a
suitable value in edx

Thanks for the patch.

-Andi

2007-05-10 14:25:54

by Benny Halevy

[permalink] [raw]
Subject: Re: Andi, you broke my laptop :-)

Joerg's patch works for me too.

Thanks,

Benny

Andi Kleen wrote:
> On Thu, May 10, 2007 at 03:35:56PM +0200, Joerg Roedel wrote:
>> On Thu, May 10, 2007 at 03:01:44PM +0200, Andi Kleen wrote:
>>> On Wed, May 09, 2007 at 12:56:16PM -0700, Pete Zaitcev wrote:
>>>> Hi, Andi:
>>>>
>>>> The attached patch (actually, git show output) makes my Dell 1501 to hang
>>>> on boot. Sorry, I have no clue why... The culprit is found with git bisect.
>>>> But yes, it's an AMD MK-36. I use an x86_64 kernel. It is 100% reproducible.
>>> MK-36? Does it have SVM?
>>>
>>> Anyways we previously had issues with this being miscompiled, but
>>> I thought the latest patch should have been ok. What compiler do you use?
>>>
>>> Can you send me a disassembly listing of arch/x86_64/kernel/time.o?
>> I debugged this problem a bit and my compiler[1]interprets the =A
>> constraint as %rax instead of %edx:%eax on x86_64 which causes the
>> problem. The appended patch provides a workaround for this and fixed the
>> hang on my machine.
>
> Hmm yes now I can reproduce it too. I didn't see any hangs so i suppose
> my (and that of most -mm tester's) compiled binary always happened to have a
> suitable value in edx
>
> Thanks for the patch.
>
> -Andi
> -
> 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/


--
Benny Halevy
Panasas Inc.
Accelerating Time to Results(TM) with Clustered Storage

http://www.panasas.com
[email protected]
Tel/Fax: +972-3-647-8340
Mobile: +972-54-802-8340

2007-05-10 16:39:41

by Pete Zaitcev

[permalink] [raw]
Subject: Re: Andi, you broke my laptop :-)

On Thu, 10 May 2007 15:35:56 +0200, "Joerg Roedel" <[email protected]> wrote:

> I debugged this problem a bit and my compiler[1]interprets the =A
> constraint as %rax instead of %edx:%eax on x86_64 which causes the
> problem. The appended patch provides a workaround for this and fixed the
> hang on my machine.
>
> [1] gcc version 4.1.3 20070429 (prerelease) (Debian 4.1.2-5)

> alternative_io(ASM_NOP3, ".byte 0x0f,0x01,0xf9", X86_FEATURE_RDTSCP,
> - "=A" (ret), "0" (0ULL) : "ecx", "memory");
> + ASM_OUTPUT2("=a" (eax), "=d" (edx)),
> + "a" (0U), "d" (0U) : "ecx", "memory");

This works for me. Thanks, Joerg.

gcc version 4.1.2 20070424 (Red Hat 4.1.2-11)

-- Pete