[Ingo Molnar - Mon, Dec 17, 2007 at 03:53:17PM +0100]
|
| * Jan Beulich <[EMAIL PROTECTED]> wrote:
|
| > > good catch! Applied your patch to x86.git - queued it up for
| > > v2.6.25. I bet there are tons of other instances where we use signed
| > > instead of unsigned and get worse code generation.
| >
| > Yes, definitely. This patch was kind of a testing one whether this is
| > a welcome change. As it appears to be, I'll probably produce more as I
| > run into respective cases.
|
| they are definitely welcome! Especially when fields are also used in
| integer multiplications or divisions then the cost of signedness can be
| quite significant. We regularly do such int -> uint sweeps in the
| scheduler code and it's a great code size reductor.
|
| Btw., a sidenote: if you are touching files that you care about, and if
| you've got a few spare cycles, you might also want to take a look at
| checkpatch.pl output:
|
| $ scripts/checkpatch.pl --file include/asm-x86/pda.h
| ...
| total: 23 errors, 1 warnings, 133 lines checked
|
| as it might have a few low hanging fruits ripe for cleanup ;-)
|
| Ingo
|
Hi, I've tried to make a one ;) It's over last (today synced) linus tree.
Cyrill
---
From: Cyrill Gorcunov <[EMAIL PROTECTED]>
Subject: [PATCH] x86: cleanup pda.h to fulfil checkpatch
---
Checkpatch still does complain about
if (0) { T__ tmp__; tmp__ = (val)
I'm not sure if we need this line at all.
include/asm-x86/pda.h | 36 ++++++++++++++++++------------------
1 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/include/asm-x86/pda.h b/include/asm-x86/pda.h
index 35962bb..00410ec 100644
--- a/include/asm-x86/pda.h
+++ b/include/asm-x86/pda.h
@@ -7,14 +7,14 @@
#include <linux/cache.h>
#include <asm/page.h>
-/* Per processor datastructure. %gs points to it while the kernel runs */
+/* Per processor datastructure. %gs points to it while the kernel runs */
struct x8664_pda {
struct task_struct *pcurrent; /* 0 Current process */
unsigned long data_offset; /* 8 Per cpu data offset from linker
address */
unsigned long kernelstack; /* 16 top of kernel stack for current */
unsigned long oldrsp; /* 24 user rsp for system call */
- int irqcount; /* 32 Irq nesting counter. Starts with -1 */
+ int irqcount; /* 32 Irq nesting counter. Starts with -1 */
int cpunumber; /* 36 Logical CPU number */
#ifdef CONFIG_CC_STACKPROTECTOR
unsigned long stack_canary; /* 40 stack canary value */
@@ -43,10 +43,10 @@ extern struct x8664_pda boot_cpu_pda[];
#define cpu_pda(i) (_cpu_pda[i])
-/*
+/*
* There is no fast way to get the base address of the PDA, all the accesses
* have to mention %fs/%gs. So it needs to be done this Torvaldian way.
- */
+ */
extern void __bad_pda_field(void) __attribute__((noreturn));
/*
@@ -57,7 +57,7 @@ extern struct x8664_pda _proxy_pda;
#define pda_offset(field) offsetof(struct x8664_pda, field)
-#define pda_to_op(op,field,val) do { \
+#define pda_to_op(op, field, val) do { \
typedef typeof(_proxy_pda.field) T__; \
if (0) { T__ tmp__; tmp__ = (val); } /* type checking */ \
switch (sizeof(_proxy_pda.field)) { \
@@ -66,7 +66,7 @@ extern struct x8664_pda _proxy_pda;
"+m" (_proxy_pda.field) : \
"ri" ((T__)val), \
"i"(pda_offset(field))); \
- break; \
+ break; \
case 4: \
asm(op "l %1,%%gs:%c2" : \
"+m" (_proxy_pda.field) : \
@@ -79,15 +79,15 @@ extern struct x8664_pda _proxy_pda;
"ri" ((T__)val), \
"i"(pda_offset(field))); \
break; \
- default: \
+ default: \
__bad_pda_field(); \
- } \
- } while (0)
+ } \
+ } while (0)
#define pda_from_op(op,field) ({ \
typeof(_proxy_pda.field) ret__; \
switch (sizeof(_proxy_pda.field)) { \
- case 2: \
+ case 2: \
asm(op "w %%gs:%c1,%0" : \
"=r" (ret__) : \
"i" (pda_offset(field)), \
@@ -99,25 +99,25 @@ extern struct x8664_pda _proxy_pda;
"i" (pda_offset(field)), \
"m" (_proxy_pda.field)); \
break; \
- case 8: \
+ case 8: \
asm(op "q %%gs:%c1,%0": \
"=r" (ret__) : \
"i" (pda_offset(field)), \
"m" (_proxy_pda.field)); \
break; \
- default: \
+ default: \
__bad_pda_field(); \
} \
ret__; })
-#define read_pda(field) pda_from_op("mov",field)
-#define write_pda(field,val) pda_to_op("mov",field,val)
-#define add_pda(field,val) pda_to_op("add",field,val)
-#define sub_pda(field,val) pda_to_op("sub",field,val)
-#define or_pda(field,val) pda_to_op("or",field,val)
+#define read_pda(field) pda_from_op("mov", field)
+#define write_pda(field, val) pda_to_op("mov", field, val)
+#define add_pda(field, val) pda_to_op("add", field, val)
+#define sub_pda(field, val) pda_to_op("sub", field, val)
+#define or_pda(field, val) pda_to_op("or", field, val)
/* This is not atomic against other CPUs -- CPU preemption needs to be off */
-#define test_and_clear_bit_pda(bit,field) ({ \
+#define test_and_clear_bit_pda(bit, field) ({ \
int old__; \
asm volatile("btr %2,%%gs:%c3\n\tsbbl %0,%0" \
: "=r" (old__), "+m" (_proxy_pda.field) \
--
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/
