Skip to contentSkip to conversation

Fix MTRR strings definition.

PATCH Dave Jones 2 participants 5 messages Aug 23, 2004 – Aug 24, 2004

lore
Share & tools
lore

Dave Jones

author
#0 lore raw

Instead of deleting the extern from include/asm/mtrr.h, I believe the correct fix would be to move the strings back to the include file where they belong.
The reason behind this, is that there are userspace apps (admittedly few, but we even ship two in Documentation/mtrr.txt) that rely upon these definitions being in that header. This has been broken for all 2.6 releases so far. Patch below fixes things back the way it was in 2.4

Andi, I don't have gcc 3.5 to hand, I trust this fixes whatever problem you saw there too ?

Dave

Restore mtrr_strings header definitions to how things were in 2.4

Signed-off-by Dave Jones <[email protected]>
arch/i386/kernel/cpu/mtrr/if.c renamed from arch/i386/kernel/cpu/mtrr/if.c~ +1 -11
@@ -6,6 +6,7 @@
 #include <asm/uaccess.h>
 
 #define LINE_SIZE 80
+#define MTRR_NEED_STRINGS
 
 #include <asm/mtrr.h>
 #include "mtrr.h"

@@ -16,17 +17,6 @@
 
 #define FILE_FCOUNT(f) (((struct seq_file *)((f)->private_data))->private)
 
-static char *mtrr_strings[MTRR_NUM_TYPES] =
-{
-    "uncachable",               /* 0 */
-    "write-combining",          /* 1 */
-    "?",                        /* 2 */
-    "?",                        /* 3 */
-    "write-through",            /* 4 */
-    "write-protect",            /* 5 */
-    "write-back",               /* 6 */
-};
-
 char *mtrr_attrib_to_str(int x)
 {
 	return (x <= 6) ? mtrr_strings[x] : "?";
include/asm-i386/mtrr.h renamed from include/asm-i386/mtrr.h~ +13 -0
@@ -65,6 +65,19 @@
 #define MTRR_TYPE_WRBACK     6
 #define MTRR_NUM_TYPES       7
 
+#ifdef MTRR_NEED_STRINGS
+static char *mtrr_strings[MTRR_NUM_TYPES] =
+{
+	"uncachable",		/* 0 */
+	"write-combining",	/* 1 */
+	"?",			/* 2 */
+	"?",			/* 3 */
+	"write-through",	/* 4 */
+	"write-protect",	/* 5 */
+	"write-back",		/* 6 */
+};
+#endif
+
 #ifdef __KERNEL__
 
 /*  The following functions are for use by other drivers  */
include/asm-x86_64/mtrr.h renamed from include/asm-x86_64/mtrr.h~ +13 -0
@@ -69,6 +69,19 @@
 #define MTRR_TYPE_WRBACK     6
 #define MTRR_NUM_TYPES       7
 
+#ifdef MTRR_NEED_STRINGS
+static char *mtrr_strings[MTRR_NUM_TYPES] =
+{
+	"uncachable",		/* 0 */
+	"write-combining",	/* 1 */
+	"?",			/* 2 */
+	"?",			/* 3 */
+	"write-through",	/* 4 */
+	"write-protect",	/* 5 */
+	"write-back",		/* 6 */
+};
+#endif
+
 #ifdef __KERNEL__
 
 extern char *mtrr_strings[MTRR_NUM_TYPES];
#1 lore raw
Quoted 8 lines · On Tue, 24 Aug 2004 00:23:20 +0100 Dave Jones <[email protected]> wrote:

Instead of deleting the extern from include/asm/mtrr.h, I believe the correct fix would be to move the strings back to the include file where they belong.
The reason behind this, is that there are userspace apps (admittedly few, but we even ship two in Documentation/mtrr.txt) that rely upon these definitions being in that header. This has been broken for all 2.6 releases so far. Patch below fixes things back the way it was in 2.4

That's rather ugly. It would be cleaner to just have a macro that expands to the strings, and everybody who wants to use it declares an own array using that macro.

Quoted 2 lines

Andi, I don't have gcc 3.5 to hand, I trust this fixes whatever problem you saw there too ?

3.5 doesn't like it when something is declared both extern and static. Your new patch has this problem again.

-Andi

author
#2 lore raw
Quoted 12 lines · Andi Kleen
Quoted 8 lines · On Tue, 24 Aug 2004 00:23:20 +0100 Dave Jones <[email protected]> wrote:

Instead of deleting the extern from include/asm/mtrr.h, I believe the correct fix would be to move the strings back to the include file where they belong.
The reason behind this, is that there are userspace apps (admittedly few, but we even ship two in Documentation/mtrr.txt) that rely upon these definitions being in that header. This has been broken for all 2.6 releases so far. Patch below fixes things back the way it was in 2.4

Quoted 3 lines

That's rather ugly. It would be cleaner to just have a macro that expands to the strings, and everybody who wants to use it declares an own array using that macro.

feel free to go rewrite the userspace that uses this.

Quoted 5 lines
Quoted 2 lines

Andi, I don't have gcc 3.5 to hand, I trust this fixes whatever problem you saw there too ?

3.5 doesn't like it when something is declared both extern and static. Your new patch has this problem again.

The extern definitions no longer exist.

Dave

#3 lore raw
Quoted 13 lines · On Tue, 24 Aug 2004 12:00:01 +0100 Dave Jones <[email protected]> wrote:
Quoted 12 lines · Andi Kleen
Quoted 8 lines · On Tue, 24 Aug 2004 00:23:20 +0100 Dave Jones <[email protected]> wrote:

Instead of deleting the extern from include/asm/mtrr.h, I believe the correct fix would be to move the strings back to the include file where they belong.
The reason behind this, is that there are userspace apps (admittedly few, but we even ship two in Documentation/mtrr.txt) that rely upon these definitions being in that header. This has been broken for all 2.6 releases so far. Patch below fixes things back the way it was in 2.4

Quoted 5 lines
Quoted 3 lines

That's rather ugly. It would be cleaner to just have a macro that expands to the strings, and everybody who wants to use it declares an own array using that macro.

feel free to go rewrite the userspace that uses this.

Umm - since when do we care about user space gratuously including kernel headers? I normally avoid breaking user space by this without need, but depending on a static variable declaration in a header is really far too ugly to be kept alive.

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

Andi, I don't have gcc 3.5 to hand, I trust this fixes whatever problem you saw there too ?

3.5 doesn't like it when something is declared both extern and static. Your new patch has this problem again.

The extern definitions no longer exist.

Your patch was:

include/asm-x86_64/mtrr.h renamed from include/asm-x86_64/mtrr.h~ +2 -0
@@ -69,6 +69,19 @@
 #define MTRR_TYPE_WRBACK     6
 #define MTRR_NUM_TYPES       7
 
+#ifdef MTRR_NEED_STRINGS
+static char *mtrr_strings[MTRR_NUM_TYPES] =

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+{
+ "uncachable", /* 0 */
+ "write-combining", /* 1 */
+ "?", /* 2 */
+ "?", /* 3 */
+ "write-through", /* 4 */
+ "write-protect", /* 5 */
+ "write-back", /* 6 */
+};
+#endif
+
#ifdef __KERNEL__

extern char *mtrr_strings[MTRR_NUM_TYPES];
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

-Andi

author
#4 lore raw
Quoted 2 lines · Andi Kleen
Quoted 1 line

The extern definitions no longer exist.

Your patch was:

Oops. I missed the x86-64 one as I thought you killed that when you killed the i386 one.

Dave

Quoted 25 lines
include/asm-x86_64/mtrr.h renamed from include/asm-x86_64/mtrr.h~ +2 -0
@@ -69,6 +69,19 @@
 #define MTRR_TYPE_WRBACK     6
 #define MTRR_NUM_TYPES       7
 
+#ifdef MTRR_NEED_STRINGS
+static char *mtrr_strings[MTRR_NUM_TYPES] =

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

include/asm-x86_64/mtrr.h renamed from include/asm-x86_64/mtrr.h~ +11 -0
+{
+	"uncachable",		/* 0 */
+	"write-combining",	/* 1 */
+	"?",			/* 2 */
+	"?",			/* 3 */
+	"write-through",	/* 4 */
+	"write-protect",	/* 5 */
+	"write-back",		/* 6 */
+};
+#endif
+
 #ifdef __KERNEL__
 
 extern char *mtrr_strings[MTRR_NUM_TYPES];

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Quoted 1 line

-Andi

---end quoted text---