Messages in this thread Patch in this message | | | Date | Thu, 02 Sep 2004 16:12:36 -0700 | From | Tim Bird <> | Subject | Re: [2.6 patch] kill __always_inline |
| |
Adrian Bunk wrote: >>>>>>The patch below removes __always_inline again: >>>>>
>>If the compiler supports attribute((always_inline)) then the kernel build >>system will use that. If the compiler doesn't support >>attribute((always_inline)) then we just emit `inline' from cpp and hope >>that it works out. > > > That's exactly how `inline' is already #define'd in the Linux kernel. > > And __always_inline is currently simply #define'd to `inline' ...
Sorry, but the way this is currently done in the Linux kernel is, IMHO, braindead.
I ran into a lot of problems with inline handling while I was working on a piece of instrumentation code that used gcc's -finstrument-functions. I found that the kernel redefinitions for inline were not being universally applied (see the patch below for how I fixed it for my situation). I found that the compiler version affected whether I really got an attribute(always_inline) or not. Also, for some uses, the include order (based on configuration options) affected whether I got the redefinition.
I didn't do a full survey of all inline uses. But the errors I found with this made me nervous.
IMHO, a better approach, if it really is desired to force always_inline, would be to globally replace 'inline' (and friends) with kern_inline or something similar, so you at least get a compiler error if you're picking up the traditional gcc semantics instead of the kernel-mandated semantics.
But my preference would be to do as Andi Kleen has suggested, which is to revert to the use of (un-redefined) 'inline' for optional inlines, and use '__always_inline' for those special cases that REALLY need to be inlines. Most (but probably not all) of these are marked with 'extern inline' in the code today. (Unfortunately, there are a few optional ones marked with 'extern inline' as well - which complicates things). I realize this is riskier, but it seems to make more sense in the long run.
Finally, I think it's bad form to change the meaning of a compiler keyword. It misleading for 'inline' to mean something different in the kernel than it does everywhere else. It means a developer can't use standard gcc documentation to understand kernel code, without inside knowledge. This can be painful for casual or new kernel developers.
Regards, -- Tim
diff -ruN -X ../../dontdiff linux-2.6.7/include/asm-ppc/delay.h branch_KFI/include/asm-ppc/delay.h --- linux-2.6.7/include/asm-ppc/delay.h 2004-06-15 22:19:42.000000000 -0700 +++ branch_KFI/include/asm-ppc/delay.h 2004-08-11 15:30:22.000000000 -0700 @@ -2,6 +2,7 @@ #ifndef _PPC_DELAY_H #define _PPC_DELAY_H
+#include <linux/compiler.h> /* for inline weirdness */ #include <asm/param.h>
/* diff -ruN -X ../../dontdiff linux-2.6.7/include/asm-ppc/processor.h branch_KFI/include/asm-ppc/processor.h --- linux-2.6.7/include/asm-ppc/processor.h 2004-06-15 22:18:38.000000000 -0700 +++ branch_KFI/include/asm-ppc/processor.h 2004-08-11 15:35:25.000000000 -0700 @@ -10,6 +10,7 @@
#include <linux/config.h> #include <linux/stringify.h> +#include <linux/compiler.h> /* for inline weirdness */
#include <asm/ptrace.h> #include <asm/types.h> diff -ruN -X ../../dontdiff linux-2.6.7/include/linux/compiler-gcc3.h branch_KFI/include/linux/compiler-gcc3.h --- linux-2.6.7/include/linux/compiler-gcc3.h 2004-06-15 22:18:45.000000000 -0700 +++ branch_KFI/include/linux/compiler-gcc3.h 2004-08-11 14:16:34.000000000 -0700 @@ -3,7 +3,7 @@ /* These definitions are for GCC v3.x. */ #include <linux/compiler-gcc.h>
-#if __GNUC_MINOR__ >= 1 && __GNUC_MINOR__ < 4 +#if __GNUC_MINOR__ >= 1 # define inline __inline__ __attribute__((always_inline)) # define __inline__ __inline__ __attribute__((always_inline)) # define __inline __inline__ __attribute__((always_inline))
============================= Tim Bird Architecture Group Co-Chair, CE Linux Forum Senior Staff Engineer, Sony Electronics E-mail: tim.bird@am.sony.com ============================= - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |