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/
