[PATCH} x86_64 PM_TRACE support.

[PATCH} x86_64 PM_TRACE support.

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 6453 bytes --]

Hi all.

As promised I took another look at the patch and at what Randy had
prepared to fix the IA64 compilation error. I did some more work on it,
and believe that the following is the tidiest correct solution I can
come up with. It differs from the version that caused the compilation
error primarily in that:

* the #include <asm/resume-trace.h> is inside the #ifdef 
  CONFIG_PM_TRACE.
* now-unnecessary protection for multiple #includes and ifdef testing of
  CONFIG_PM_TRACE in the asm code were removed.
* do-nothing definitions for !PM_TRACE restored to
  include/linux/resume-trace.h.

We're therefore depending upon kernel/power/Kconfig having the right
depends condition. As far as I can see, IA64 doesn't define CONFIG_X86.
Is that correct, or do we need to have (X86 && !IA64)?

Randy, since this is substantially different to the work you did, I
haven't added your signed-off-by as we previously discussed.

Signed-off-by: Nigel Cunningham <nigel@nigel.suspend2.net>
CC: Randy Dunlap <rdunlap@xenotime.net>
CC: Andi Kleen <ak@suse.de>

 arch/x86_64/kernel/vmlinux.lds.S  |    7 +++++++
 drivers/base/power/trace.c        |    5 ++++-
 include/asm-i386/resume-trace.h   |   13 +++++++++++++
 include/asm-x86_64/resume-trace.h |   13 +++++++++++++
 include/linux/resume-trace.h      |   19 +++++--------------
 kernel/power/Kconfig              |    2 +-
 6 files changed, 43 insertions(+), 16 deletions(-)
diff -ruNp 200.patch-old/arch/x86_64/kernel/vmlinux.lds.S 200.patch-new/arch/x86_64/kernel/vmlinux.lds.S
--- 200.patch-old/arch/x86_64/kernel/vmlinux.lds.S	2007-05-26 14:18:22.000000000 +1000
+++ 200.patch-new/arch/x86_64/kernel/vmlinux.lds.S	2007-05-26 14:23:52.000000000 +1000
@@ -52,6 +52,13 @@ SECTIONS
 
   RODATA
 
+  . = ALIGN(4);
+  .tracedata : AT(ADDR(.tracedata) - LOAD_OFFSET) {
+  	__tracedata_start = .;
+	*(.tracedata)
+  	__tracedata_end = .;
+  }
+
   . = ALIGN(PAGE_SIZE);        /* Align data segment to page size boundary */
 				/* Data */
   .data : AT(ADDR(.data) - LOAD_OFFSET) {
diff -ruNp 200.patch-old/drivers/base/power/trace.c 200.patch-new/drivers/base/power/trace.c
--- 200.patch-old/drivers/base/power/trace.c	2007-05-26 14:18:23.000000000 +1000
+++ 200.patch-new/drivers/base/power/trace.c	2007-05-26 14:23:52.000000000 +1000
@@ -142,6 +142,7 @@ void set_trace_device(struct device *dev
 {
 	dev_hash_value = hash_string(DEVSEED, dev->bus_id, DEVHASH);
 }
+EXPORT_SYMBOL(set_trace_device);
 
 /*
  * We could just take the "tracedata" index into the .tracedata
@@ -162,6 +163,7 @@ void generate_resume_trace(void *traceda
 	file_hash_value = hash_string(lineno, file, FILEHASH);
 	set_magic_time(user_hash_value, file_hash_value, dev_hash_value);
 }
+EXPORT_SYMBOL(generate_resume_trace);
 
 extern char __tracedata_start, __tracedata_end;
 static int show_file_hash(unsigned int value)
@@ -170,7 +172,8 @@ static int show_file_hash(unsigned int v
 	char *tracedata;
 
 	match = 0;
-	for (tracedata = &__tracedata_start ; tracedata < &__tracedata_end ; tracedata += 6) {
+	for (tracedata = &__tracedata_start ; tracedata < &__tracedata_end ;
+			tracedata += 2 + sizeof(unsigned long)) {
 		unsigned short lineno = *(unsigned short *)tracedata;
 		const char *file = *(const char **)(tracedata + 2);
 		unsigned int hash = hash_string(lineno, file, FILEHASH);
diff -ruNp 200.patch-old/include/asm-i386/resume-trace.h 200.patch-new/include/asm-i386/resume-trace.h
--- 200.patch-old/include/asm-i386/resume-trace.h	1970-01-01 10:00:00.000000000 +1000
+++ 200.patch-new/include/asm-i386/resume-trace.h	2007-05-26 16:07:31.000000000 +1000
@@ -0,0 +1,13 @@
+#define TRACE_RESUME(user) do {					\
+	if (pm_trace_enabled) {					\
+		void *tracedata;				\
+		asm volatile("movl $1f,%0\n"			\
+			".section .tracedata,\"a\"\n"		\
+			"1:\t.word %c1\n"			\
+			"\t.long %c2\n"				\
+			".previous"				\
+			:"=r" (tracedata)			\
+			: "i" (__LINE__), "i" (__FILE__));	\
+		generate_resume_trace(tracedata, user);		\
+	}							\
+} while (0)
diff -ruNp 200.patch-old/include/asm-x86_64/resume-trace.h 200.patch-new/include/asm-x86_64/resume-trace.h
--- 200.patch-old/include/asm-x86_64/resume-trace.h	1970-01-01 10:00:00.000000000 +1000
+++ 200.patch-new/include/asm-x86_64/resume-trace.h	2007-05-26 16:07:27.000000000 +1000
@@ -0,0 +1,13 @@
+#define TRACE_RESUME(user) do {					\
+	if (pm_trace_enabled) {					\
+		void *tracedata;				\
+		asm volatile("movq $1f,%0\n"			\
+			".section .tracedata,\"a\"\n"		\
+			"1:\t.word %c1\n"			\
+			"\t.quad %c2\n"				\
+			".previous"				\
+			:"=r" (tracedata)			\
+			: "i" (__LINE__), "i" (__FILE__));	\
+		generate_resume_trace(tracedata, user);		\
+	}							\
+} while (0)
diff -ruNp 200.patch-old/include/linux/resume-trace.h 200.patch-new/include/linux/resume-trace.h
--- 200.patch-old/include/linux/resume-trace.h	2007-05-26 14:18:47.000000000 +1000
+++ 200.patch-new/include/linux/resume-trace.h	2007-05-26 16:17:43.000000000 +1000
@@ -2,6 +2,7 @@
 #define RESUME_TRACE_H
 
 #ifdef CONFIG_PM_TRACE
+#include <asm/resume-trace.h>
 
 extern int pm_trace_enabled;
 
@@ -9,20 +10,10 @@ struct device;
 extern void set_trace_device(struct device *);
 extern void generate_resume_trace(void *tracedata, unsigned int user);
 
-#define TRACE_DEVICE(dev) set_trace_device(dev)
-#define TRACE_RESUME(user) do {					\
-	if (pm_trace_enabled) {					\
-		void *tracedata;				\
-		asm volatile("movl $1f,%0\n"			\
-			".section .tracedata,\"a\"\n"		\
-			"1:\t.word %c1\n"			\
-			"\t.long %c2\n"				\
-			".previous"				\
-			:"=r" (tracedata)			\
-			: "i" (__LINE__), "i" (__FILE__));	\
-		generate_resume_trace(tracedata, user);		\
-	}							\
-} while (0)
+#define TRACE_DEVICE(dev) do { \
+	if (pm_trace_enabled) \
+		set_trace_device(dev); \
+	} while(0)
 
 #else
 
diff -ruNp 200.patch-old/kernel/power/Kconfig 200.patch-new/kernel/power/Kconfig
--- 200.patch-old/kernel/power/Kconfig	2007-05-26 14:18:48.000000000 +1000
+++ 200.patch-new/kernel/power/Kconfig	2007-05-26 14:23:52.000000000 +1000
@@ -50,7 +50,7 @@ config DISABLE_CONSOLE_SUSPEND
 
 config PM_TRACE
 	bool "Suspend/resume event tracing"
-	depends on PM && PM_DEBUG && X86_32 && EXPERIMENTAL
+	depends on PM && PM_DEBUG && X86 && EXPERIMENTAL
 	default n
 	---help---
 	This enables some cheesy code to save the last PM event point in the


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH} x86_64 PM_TRACE support.

[Pavel Machek]

Hi!

> As promised I took another look at the patch and at what Randy had
> prepared to fix the IA64 compilation error. I did some more work on it,
> and believe that the following is the tidiest correct solution I can
> come up with. It differs from the version that caused the compilation
> error primarily in that:
> 
> * the #include <asm/resume-trace.h> is inside the #ifdef 
>   CONFIG_PM_TRACE.
> * now-unnecessary protection for multiple #includes and ifdef testing of
>   CONFIG_PM_TRACE in the asm code were removed.
> * do-nothing definitions for !PM_TRACE restored to
>   include/linux/resume-trace.h.
> 
> We're therefore depending upon kernel/power/Kconfig having the right
> depends condition. As far as I can see, IA64 doesn't define CONFIG_X86.
> Is that correct, or do we need to have (X86 && !IA64)?

ia64? did you mean x86-64?

Otherwise looks ok to me.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH} x86_64 PM_TRACE support.

[Julian Sikorski]

Pavel Machek pisze:
> Hi!
> 
>> As promised I took another look at the patch and at what Randy had
>> prepared to fix the IA64 compilation error. I did some more work on it,
>> and believe that the following is the tidiest correct solution I can
>> come up with. It differs from the version that caused the compilation
>> error primarily in that:
>>
>> * the #include <asm/resume-trace.h> is inside the #ifdef 
>>   CONFIG_PM_TRACE.
>> * now-unnecessary protection for multiple #includes and ifdef testing of
>>   CONFIG_PM_TRACE in the asm code were removed.
>> * do-nothing definitions for !PM_TRACE restored to
>>   include/linux/resume-trace.h.
>>
>> We're therefore depending upon kernel/power/Kconfig having the right
>> depends condition. As far as I can see, IA64 doesn't define CONFIG_X86.
>> Is that correct, or do we need to have (X86 && !IA64)?
> 
> ia64? did you mean x86-64?
> 
> Otherwise looks ok to me.
> 
IIRC enabling pm_trace on x86_64 was breaking compilation on ia64, so I
think Nigel meant the latter.

Re: [PATCH} x86_64 PM_TRACE support.

[Randy Dunlap]

On Sat, 26 May 2007 16:32:54 +1000 Nigel Cunningham wrote:

> Hi all.
> 
> As promised I took another look at the patch and at what Randy had
> prepared to fix the IA64 compilation error. I did some more work on it,
> and believe that the following is the tidiest correct solution I can
> come up with. It differs from the version that caused the compilation
> error primarily in that:
> 
> * the #include <asm/resume-trace.h> is inside the #ifdef 
>   CONFIG_PM_TRACE.
> * now-unnecessary protection for multiple #includes and ifdef testing of
>   CONFIG_PM_TRACE in the asm code were removed.
> * do-nothing definitions for !PM_TRACE restored to
>   include/linux/resume-trace.h.
> 
> We're therefore depending upon kernel/power/Kconfig having the right
> depends condition. As far as I can see, IA64 doesn't define CONFIG_X86.
> Is that correct, or do we need to have (X86 && !IA64)?

Correct, X86 is i386 and x86_64.  Not IA64.

> Randy, since this is substantially different to the work you did, I
> haven't added your signed-off-by as we previously discussed.

OK.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH} x86_64 PM_TRACE support.

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 1337 bytes --]

Hi.

On Sun, 2007-05-27 at 22:10 +0100, Julian Sikorski wrote:
> Pavel Machek pisze:
> > Hi!
> > 
> >> As promised I took another look at the patch and at what Randy had
> >> prepared to fix the IA64 compilation error. I did some more work on it,
> >> and believe that the following is the tidiest correct solution I can
> >> come up with. It differs from the version that caused the compilation
> >> error primarily in that:
> >>
> >> * the #include <asm/resume-trace.h> is inside the #ifdef 
> >>   CONFIG_PM_TRACE.
> >> * now-unnecessary protection for multiple #includes and ifdef testing of
> >>   CONFIG_PM_TRACE in the asm code were removed.
> >> * do-nothing definitions for !PM_TRACE restored to
> >>   include/linux/resume-trace.h.
> >>
> >> We're therefore depending upon kernel/power/Kconfig having the right
> >> depends condition. As far as I can see, IA64 doesn't define CONFIG_X86.
> >> Is that correct, or do we need to have (X86 && !IA64)?
> > 
> > ia64? did you mean x86-64?
> > 
> > Otherwise looks ok to me.
> > 
> IIRC enabling pm_trace on x86_64 was breaking compilation on ia64, so I
> think Nigel meant the latter.

Yes, it was breaking ia64, so I meant what I said - I can test x86_64
easily. I was concerned about ensuring the condition was right for ia64.

Regards,

Nigel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH} x86_64 PM_TRACE support.

[Dave Jones]

On Sat, May 26, 2007 at 04:32:54PM +1000, Nigel Cunningham wrote:

Hi Nigel,

 > As promised I took another look at the patch and at what Randy had
 > prepared to fix the IA64 compilation error. I did some more work on it,
 > and believe that the following is the tidiest correct solution I can
 > come up with. It differs from the version that caused the compilation
 > error primarily in that:
 > 
 > * the #include <asm/resume-trace.h> is inside the #ifdef 
 >   CONFIG_PM_TRACE.
 > * now-unnecessary protection for multiple #includes and ifdef testing of
 >   CONFIG_PM_TRACE in the asm code were removed.
 > * do-nothing definitions for !PM_TRACE restored to
 >   include/linux/resume-trace.h.
 > 
 > We're therefore depending upon kernel/power/Kconfig having the right
 > depends condition. As far as I can see, IA64 doesn't define CONFIG_X86.
 > Is that correct, or do we need to have (X86 && !IA64)?

Can you post a copy of this that isn't mangled by quoted-printable encoding?
Whilst it looks fine in my MUA, the diff ends up looking like..

diff -ruNp 200.patch-old/arch/x86_64/kernel/vmlinux.lds.S 200.patch-new/arc=
h/x86_64/kernel/vmlinux.lds.S
--- 200.patch-old/arch/x86_64/kernel/vmlinux.lds.S      2007-05-26 14:18:22.0000=
00000 +1000
+++ 200.patch-new/arch/x86_64/kernel/vmlinux.lds.S      2007-05-26 14:23:52.0000=
00000 +1000
@@ -52,6 +52,13 @@ SECTIONS
=20
   RODATA
=20
+  . =3D ALIGN(4);
+  .tracedata : AT(ADDR(.tracedata) - LOAD_OFFSET) {
+       __tracedata_start =3D .;
+       *(.tracedata)
+       __tracedata_end =3D .;
+  }


I'm beginning to think we really need a Documentation/Unhorking-MUAs-HOWTO
judging by the amount of broken encodings we seem to get to lkml these days.

	Dave

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

Re: [PATCH} x86_64 PM_TRACE support.

[Randy Dunlap]

On Wed, 30 May 2007 20:01:34 -0400 Dave Jones wrote:

> On Sat, May 26, 2007 at 04:32:54PM +1000, Nigel Cunningham wrote:
> 
> Hi Nigel,
> 
>  > As promised I took another look at the patch and at what Randy had
>  > prepared to fix the IA64 compilation error. I did some more work on it,
>  > and believe that the following is the tidiest correct solution I can
>  > come up with. It differs from the version that caused the compilation
>  > error primarily in that:
>  > 
>  > * the #include <asm/resume-trace.h> is inside the #ifdef 
>  >   CONFIG_PM_TRACE.
>  > * now-unnecessary protection for multiple #includes and ifdef testing of
>  >   CONFIG_PM_TRACE in the asm code were removed.
>  > * do-nothing definitions for !PM_TRACE restored to
>  >   include/linux/resume-trace.h.
>  > 
>  > We're therefore depending upon kernel/power/Kconfig having the right
>  > depends condition. As far as I can see, IA64 doesn't define CONFIG_X86.
>  > Is that correct, or do we need to have (X86 && !IA64)?
> 
> Can you post a copy of this that isn't mangled by quoted-printable encoding?
> Whilst it looks fine in my MUA, the diff ends up looking like..

yep :(

http://www.xenotime.net/linux/patches/x8664-pm-trace-support.patch

in case Nigel is asleep.


> I'm beginning to think we really need a Documentation/Unhorking-MUAs-HOWTO
> judging by the amount of broken encodings we seem to get to lkml these days.


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH} x86_64 PM_TRACE support.

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 1744 bytes --]

Hi.

On Wed, 2007-05-30 at 17:09 -0700, Randy Dunlap wrote:
> > On Sat, May 26, 2007 at 04:32:54PM +1000, Nigel Cunningham wrote:
> > 
> > Hi Nigel,
> > 
> >  > As promised I took another look at the patch and at what Randy had
> >  > prepared to fix the IA64 compilation error. I did some more work on it,
> >  > and believe that the following is the tidiest correct solution I can
> >  > come up with. It differs from the version that caused the compilation
> >  > error primarily in that:
> >  > 
> >  > * the #include <asm/resume-trace.h> is inside the #ifdef 
> >  >   CONFIG_PM_TRACE.
> >  > * now-unnecessary protection for multiple #includes and ifdef testing of
> >  >   CONFIG_PM_TRACE in the asm code were removed.
> >  > * do-nothing definitions for !PM_TRACE restored to
> >  >   include/linux/resume-trace.h.
> >  > 
> >  > We're therefore depending upon kernel/power/Kconfig having the right
> >  > depends condition. As far as I can see, IA64 doesn't define CONFIG_X86.
> >  > Is that correct, or do we need to have (X86 && !IA64)?
> > 
> > Can you post a copy of this that isn't mangled by quoted-printable encoding?
> > Whilst it looks fine in my MUA, the diff ends up looking like..
> 
> yep :(
> 
> http://www.xenotime.net/linux/patches/x8664-pm-trace-support.patch
> 
> in case Nigel is asleep.

Thanks Randy. I wasn't asleep, but was out for most of the day.

> > I'm beginning to think we really need a Documentation/Unhorking-MUAs-HOWTO
> > judging by the amount of broken encodings we seem to get to lkml these days.

That would be good. I just took a look through evo's settings, but can't
find whatever option is set wrong. I think it's devolved on me :\.

Regards,

Nigel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]