Re: [PATCH v6 19/29] memcg: infrastructure to match an allocation to the right cache
On Thu 08-11-12 12:05:13, Michal Hocko wrote: > On Tue 06-11-12 09:03:54, Michal Hocko wrote: > > On Mon 05-11-12 16:28:37, Andrew Morton wrote: > > > On Thu, 1 Nov 2012 16:07:35 +0400 > > > Glauber Costa wrote: > > > > > > > +static __always_inline struct kmem_cache * > > > > +memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp) > > > > > > I still don't understand why this code uses __always_inline so much. > > > > AFAIU, __always_inline (resp. __attribute__((always_inline))) is the > > same thing as inline if optimizations are enabled > > (http://ohse.de/uwe/articles/gcc-attributes.html#func-always_inline). > > And this doesn't tell the whole story because there is -fearly-inlining > which enabled by default and it makes a difference when optimizations > are enabled so __always_inline really enforces inlining. and -fearly-inlining is another doc trap. I have tried with -O2 -fno-early-inlining and __always_inline code has been inlined with gcc 4.3 and 4.7 while simple inline is ignored so it really seems that __always_inline is always inlined but man page is little a bit mean to tell us all the details. -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 19/29] memcg: infrastructure to match an allocation to the right cache
On Tue 06-11-12 09:03:54, Michal Hocko wrote: > On Mon 05-11-12 16:28:37, Andrew Morton wrote: > > On Thu, 1 Nov 2012 16:07:35 +0400 > > Glauber Costa wrote: > > > > > +static __always_inline struct kmem_cache * > > > +memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp) > > > > I still don't understand why this code uses __always_inline so much. > > AFAIU, __always_inline (resp. __attribute__((always_inline))) is the > same thing as inline if optimizations are enabled > (http://ohse.de/uwe/articles/gcc-attributes.html#func-always_inline). And this doesn't tell the whole story because there is -fearly-inlining which enabled by default and it makes a difference when optimizations are enabled so __always_inline really enforces inlining. -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 19/29] memcg: infrastructure to match an allocation to the right cache
On Tue 06-11-12 09:03:54, Michal Hocko wrote: On Mon 05-11-12 16:28:37, Andrew Morton wrote: On Thu, 1 Nov 2012 16:07:35 +0400 Glauber Costa glom...@parallels.com wrote: +static __always_inline struct kmem_cache * +memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp) I still don't understand why this code uses __always_inline so much. AFAIU, __always_inline (resp. __attribute__((always_inline))) is the same thing as inline if optimizations are enabled (http://ohse.de/uwe/articles/gcc-attributes.html#func-always_inline). And this doesn't tell the whole story because there is -fearly-inlining which enabled by default and it makes a difference when optimizations are enabled so __always_inline really enforces inlining. -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 19/29] memcg: infrastructure to match an allocation to the right cache
On Thu 08-11-12 12:05:13, Michal Hocko wrote: On Tue 06-11-12 09:03:54, Michal Hocko wrote: On Mon 05-11-12 16:28:37, Andrew Morton wrote: On Thu, 1 Nov 2012 16:07:35 +0400 Glauber Costa glom...@parallels.com wrote: +static __always_inline struct kmem_cache * +memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp) I still don't understand why this code uses __always_inline so much. AFAIU, __always_inline (resp. __attribute__((always_inline))) is the same thing as inline if optimizations are enabled (http://ohse.de/uwe/articles/gcc-attributes.html#func-always_inline). And this doesn't tell the whole story because there is -fearly-inlining which enabled by default and it makes a difference when optimizations are enabled so __always_inline really enforces inlining. and -fearly-inlining is another doc trap. I have tried with -O2 -fno-early-inlining and __always_inline code has been inlined with gcc 4.3 and 4.7 while simple inline is ignored so it really seems that __always_inline is always inlined but man page is little a bit mean to tell us all the details. -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 19/29] memcg: infrastructure to match an allocation to the right cache
On Wed, 7 Nov 2012 08:04:03 +0100 Glauber Costa wrote: > On 11/06/2012 01:28 AM, Andrew Morton wrote: > > On Thu, 1 Nov 2012 16:07:35 +0400 > > Glauber Costa wrote: > > > >> +static __always_inline struct kmem_cache * > >> +memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp) > > > > I still don't understand why this code uses __always_inline so much. > > > > I don't recall seeing the compiler producing out-of-line versions of > > "static inline" functions (and perhaps it has special treatment for > > functions which were defined in a header file?). > > > > And if the compiler *does* decide to uninline the function, perhaps it > > knows best, and the function shouldn't have been declared inline in the > > first place. > > > > > > If it is indeed better to use __always_inline in this code then we have > > a heck of a lot of other "static inline" definitions whcih we need to > > convert! So, what's going on here? > > > > The original motivation is indeed performance related. We want to make > sure it is inline so it will figure out quickly the "I am not a memcg > user" case and keep it going. The slub, for instance, is full of > __always_inline functions to make sure that the fast path contains > absolutely no function calls. So I was just following this here. Well. Do we really know that inlining is best in all these cases? And in future, as the code evolves? If for some reason the compiler chooses not to inline the function, maybe it was right. Small code footprint has benefits. > I can remove the marker without a problem and leave it to the compiler > if you think it is best It's a minor thing. But __always_inline is rather specialised and readers of this code will be wondering why it was done here. Unless we can actually demonstrate benefit from __always_inline, I'd suggest following convention here. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 19/29] memcg: infrastructure to match an allocation to the right cache
On 11/06/2012 01:28 AM, Andrew Morton wrote: > On Thu, 1 Nov 2012 16:07:35 +0400 > Glauber Costa wrote: > >> +static __always_inline struct kmem_cache * >> +memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp) > > I still don't understand why this code uses __always_inline so much. > > I don't recall seeing the compiler producing out-of-line versions of > "static inline" functions (and perhaps it has special treatment for > functions which were defined in a header file?). > > And if the compiler *does* decide to uninline the function, perhaps it > knows best, and the function shouldn't have been declared inline in the > first place. > > > If it is indeed better to use __always_inline in this code then we have > a heck of a lot of other "static inline" definitions whcih we need to > convert! So, what's going on here? > The original motivation is indeed performance related. We want to make sure it is inline so it will figure out quickly the "I am not a memcg user" case and keep it going. The slub, for instance, is full of __always_inline functions to make sure that the fast path contains absolutely no function calls. So I was just following this here. I can remove the marker without a problem and leave it to the compiler if you think it is best -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 19/29] memcg: infrastructure to match an allocation to the right cache
On Mon 05-11-12 16:28:37, Andrew Morton wrote: > On Thu, 1 Nov 2012 16:07:35 +0400 > Glauber Costa wrote: > > > +static __always_inline struct kmem_cache * > > +memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp) > > I still don't understand why this code uses __always_inline so much. AFAIU, __always_inline (resp. __attribute__((always_inline))) is the same thing as inline if optimizations are enabled (http://ohse.de/uwe/articles/gcc-attributes.html#func-always_inline). Which is the case for the kernel. I was always wondering why we have this __always_inline thingy. It has been introduced back in 2004 by Andi but the commit log doesn't say much: " [PATCH] gcc-3.5 fixes Trivial gcc-3.5 build fixes. " Andi what was the original motivation for this attribute? > I don't recall seeing the compiler producing out-of-line versions of > "static inline" functions and if it decides then __always_inline will not help, right? -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 19/29] memcg: infrastructure to match an allocation to the right cache
On Mon 05-11-12 16:28:37, Andrew Morton wrote: On Thu, 1 Nov 2012 16:07:35 +0400 Glauber Costa glom...@parallels.com wrote: +static __always_inline struct kmem_cache * +memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp) I still don't understand why this code uses __always_inline so much. AFAIU, __always_inline (resp. __attribute__((always_inline))) is the same thing as inline if optimizations are enabled (http://ohse.de/uwe/articles/gcc-attributes.html#func-always_inline). Which is the case for the kernel. I was always wondering why we have this __always_inline thingy. It has been introduced back in 2004 by Andi but the commit log doesn't say much: [PATCH] gcc-3.5 fixes Trivial gcc-3.5 build fixes. Andi what was the original motivation for this attribute? I don't recall seeing the compiler producing out-of-line versions of static inline functions and if it decides then __always_inline will not help, right? -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 19/29] memcg: infrastructure to match an allocation to the right cache
On 11/06/2012 01:28 AM, Andrew Morton wrote: On Thu, 1 Nov 2012 16:07:35 +0400 Glauber Costa glom...@parallels.com wrote: +static __always_inline struct kmem_cache * +memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp) I still don't understand why this code uses __always_inline so much. I don't recall seeing the compiler producing out-of-line versions of static inline functions (and perhaps it has special treatment for functions which were defined in a header file?). And if the compiler *does* decide to uninline the function, perhaps it knows best, and the function shouldn't have been declared inline in the first place. If it is indeed better to use __always_inline in this code then we have a heck of a lot of other static inline definitions whcih we need to convert! So, what's going on here? The original motivation is indeed performance related. We want to make sure it is inline so it will figure out quickly the I am not a memcg user case and keep it going. The slub, for instance, is full of __always_inline functions to make sure that the fast path contains absolutely no function calls. So I was just following this here. I can remove the marker without a problem and leave it to the compiler if you think it is best -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 19/29] memcg: infrastructure to match an allocation to the right cache
On Wed, 7 Nov 2012 08:04:03 +0100 Glauber Costa glom...@parallels.com wrote: On 11/06/2012 01:28 AM, Andrew Morton wrote: On Thu, 1 Nov 2012 16:07:35 +0400 Glauber Costa glom...@parallels.com wrote: +static __always_inline struct kmem_cache * +memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp) I still don't understand why this code uses __always_inline so much. I don't recall seeing the compiler producing out-of-line versions of static inline functions (and perhaps it has special treatment for functions which were defined in a header file?). And if the compiler *does* decide to uninline the function, perhaps it knows best, and the function shouldn't have been declared inline in the first place. If it is indeed better to use __always_inline in this code then we have a heck of a lot of other static inline definitions whcih we need to convert! So, what's going on here? The original motivation is indeed performance related. We want to make sure it is inline so it will figure out quickly the I am not a memcg user case and keep it going. The slub, for instance, is full of __always_inline functions to make sure that the fast path contains absolutely no function calls. So I was just following this here. Well. Do we really know that inlining is best in all these cases? And in future, as the code evolves? If for some reason the compiler chooses not to inline the function, maybe it was right. Small code footprint has benefits. I can remove the marker without a problem and leave it to the compiler if you think it is best It's a minor thing. But __always_inline is rather specialised and readers of this code will be wondering why it was done here. Unless we can actually demonstrate benefit from __always_inline, I'd suggest following convention here. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 19/29] memcg: infrastructure to match an allocation to the right cache
On Thu, 1 Nov 2012 16:07:35 +0400 Glauber Costa wrote: > +static __always_inline struct kmem_cache * > +memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp) I still don't understand why this code uses __always_inline so much. I don't recall seeing the compiler producing out-of-line versions of "static inline" functions (and perhaps it has special treatment for functions which were defined in a header file?). And if the compiler *does* decide to uninline the function, perhaps it knows best, and the function shouldn't have been declared inline in the first place. If it is indeed better to use __always_inline in this code then we have a heck of a lot of other "static inline" definitions whcih we need to convert! So, what's going on here? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 19/29] memcg: infrastructure to match an allocation to the right cache
On Thu, 1 Nov 2012 16:07:35 +0400 Glauber Costa glom...@parallels.com wrote: +static __always_inline struct kmem_cache * +memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp) I still don't understand why this code uses __always_inline so much. I don't recall seeing the compiler producing out-of-line versions of static inline functions (and perhaps it has special treatment for functions which were defined in a header file?). And if the compiler *does* decide to uninline the function, perhaps it knows best, and the function shouldn't have been declared inline in the first place. If it is indeed better to use __always_inline in this code then we have a heck of a lot of other static inline definitions whcih we need to convert! So, what's going on here? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
