Re: [RFC][PATCH] tracing/module: Move tracepoint out of module.h

[Steven Rostedt <rostedt@goodmis.org>]

On Thu, 2012-01-26 at 11:28 +0100, Ingo Molnar wrote:

> How much more do we save if we move all of try_module_get() out 
> of line? It still seems a rather thick inline function with 
> preempt section and all. I'd *really* suggest that it should all 
> be uninlined.
> 

Here's the #'s

   text	   data	    bss	    dec	    hex	filename
7489488	2249584	9719808	19458880	128eb40	vmlinux-prepatch
   text	   data	    bss	    dec	    hex	filename
7482458	2248048	9719808	19450314	128c9ca	vmlinux-postpatch
   text	   data	    bss	    dec	    hex	filename
7477393	2248080	9719808	19445281	128b621	vmlinux-updatedpatch

Thus we get an additional 5k it seems, for a total of 13k savings.

The below patch is what I used. Note, since the tracepoint requires the
instruction pointer, I created stub versions that call a "do_" version
of the function to pass in the local instruction counter.

Rusty, can you give your acked-by on that patch?

Thanks!

-- Steve

diff --git a/include/linux/module.h b/include/linux/module.h
index 3cb7839..2337f97 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -21,8 +21,6 @@
 #include <linux/percpu.h>
 #include <asm/module.h>
 
-#include <trace/events/module.h>
-
 /* Not Yet Implemented */
 #define MODULE_SUPPORTED_DEVICE(name)
 
@@ -439,34 +437,17 @@ void __symbol_put(const char *symbol);
 #define symbol_put(x) __symbol_put(MODULE_SYMBOL_PREFIX #x)
 void symbol_put_addr(void *addr);
 
-/* Sometimes we know we already have a refcount, and it's easier not
-   to handle the error case (which only happens with rmmod --wait). */
+extern void do_module_get(struct module *module, int ip);
+extern int do_try_module_get(struct module *module, int ip);
+
 static inline void __module_get(struct module *module)
 {
-	if (module) {
-		preempt_disable();
-		__this_cpu_inc(module->refptr->incs);
-		trace_module_get(module, _THIS_IP_);
-		preempt_enable();
-	}
+	do_module_get(module, _THIS_IP_);
 }
 
 static inline int try_module_get(struct module *module)
 {
-	int ret = 1;
-
-	if (module) {
-		preempt_disable();
-
-		if (likely(module_is_live(module))) {
-			__this_cpu_inc(module->refptr->incs);
-			trace_module_get(module, _THIS_IP_);
-		} else
-			ret = 0;
-
-		preempt_enable();
-	}
-	return ret;
+	return do_try_module_get(module, _THIS_IP_);
 }
 
 extern void module_put(struct module *module);
diff --git a/kernel/module.c b/kernel/module.c
index 178333c..8955199 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -928,6 +928,42 @@ void module_put(struct module *module)
 }
 EXPORT_SYMBOL(module_put);
 
+static void inc_module(struct module *module, unsigned long ip)
+{
+	__this_cpu_inc(module->refptr->incs);
+	trace_module_get(module, ip);
+}
+
+/* Sometimes we know we already have a refcount, and it's easier not
+   to handle the error case (which only happens with rmmod --wait). */
+void do_module_get(struct module *module, int ip)
+{
+	if (module) {
+		preempt_disable();
+		inc_module(module, ip);
+		preempt_enable();
+	}
+}
+EXPORT_SYMBOL(do_module_get);
+
+int do_try_module_get(struct module *module, int ip)
+{
+	int ret = 1;
+
+	if (module) {
+		preempt_disable();
+
+		if (likely(module_is_live(module)))
+			inc_module(module, ip);
+		else
+			ret = 0;
+
+		preempt_enable();
+	}
+	return ret;
+}
+EXPORT_SYMBOL(do_try_module_get);
+
 #else /* !CONFIG_MODULE_UNLOAD */
 static inline void print_unload_info(struct seq_file *m, struct module *mod)
 {