Re: [PATCH] Linux Markers 0.4 (+dynamic probe loader) for 2.6.17

From: Jeremy Fitzhardinge
Date: Wed Sep 20 2006 - 20:13:51 EST


Mathieu Desnoyers wrote:
+#define MARK_SYM(name) __asm__ ("__mark_kprobe_" #name ":")

This will need to be "asm volatile()" so that gcc doesn't get rid of it altogether. Also, there's nothing to make gcc keep this in any particular place in the instruction stream, since it doesn't have any data dependencies on anything else. A "memory" clobber might help, but ideally it would have some explicit data dependency on an important value at that point.

+#else +#define MARK_SYM(name)
+#endif
+
+
+#ifdef CONFIG_MARK_JUMP_CALL
+#define MARK_JUMP_CALL_PROTOTYPE(name) \
+ static void \
+ (*__mark_##name##_call)(const char *fmt, ...) \
+ asm ("__mark_"#name"_call") = \
+ __mark_empty_function
+#define MARK_JUMP_CALL(name, format, args...) \
+ do { \
+ preempt_disable(); \
+ (void) (__mark_##name##_call(format, ## args)); \

(*foo)(args) is the preferred style for calling function pointers, since it makes the pointerness explicit. Though in this case there's enough other complexity that the function call syntax is pretty much irrelevant here.

+ preempt_enable_no_resched(); \
+ } while(0)
+#else
+#define MARK_JUMP_CALL_PROTOTYPE(name)
+#define MARK_JUMP_CALL(name, format, args...)
+#endif
+
+#ifdef CONFIG_MARK_JUMP_INLINE
+#define MARK_JUMP_INLINE(name, format, args...) \
+ (void) (__mark_##name##_inline(format, ## args))
+#else
+#define MARK_JUMP_INLINE(name, format, args...)
+#endif
+
+#define MARK_JUMP(name, format, args...) \
+ do { \
+ __label__ over_label, call_label, inline_label; \
+ volatile static void *__mark_##name##_jump_over \
+ asm ("__mark_"#name"_jump_over") = \
+ &&over_label; \
+ volatile static void *__mark_##name##_jump_call \
+ asm ("__mark_"#name"_jump_call") \
+ __attribute__((unused)) = \
+ &&call_label; \
+ volatile static void *__mark_##name##_jump_inline \
+ asm ("__mark_"#name"_jump_inline") \
+ __attribute__((unused)) = \
+ &&inline_label; \
+ MARK_JUMP_CALL_PROTOTYPE(name); \
+ goto *__mark_##name##_jump_over; \
+call_label: \
+ MARK_JUMP_CALL(name, format, ## args); \
+ goto over_label; \
+inline_label: \
+ MARK_JUMP_INLINE(name, format, ## args); \
+over_label: \
+ do {} while(0); \
+ } while(0)

I have to admit I haven't been following all this tracing stuff, but this is pretty opaque. What's it trying to achieve? Hm, OK, I think I see what you're getting at here - see below.

+
+#define MARK(name, format, args...) \
+ do { \
+ __mark_check_format(format, ## args); \
+ MARK_SYM(name); \
+ MARK_JUMP(name, format, ## args); \
+ } while(0)

Does this assume that the symbol injected by MARK_SYM() will label the MARK_JUMP() code? Because it won't.


printk("Installing hook\n");
*target_mark_call = (void*)do_mark1;
saved_over = *target_mark_jump_over;
*target_mark_jump_over = *target_mark_jump_call;

So the point of this is to set up the new function, then update the jumpover to point to it, in a way that's SMP safe? This assumes two things:

1. that your pointer updates are atomic
2. that these writes don't get reordered

1 might be safe, but I don't think its guaranteed for all architectures. 2 is not true without some explicit barriers in there. You'll probably need some in MARK_JUMP too.


return 0;
}

int init_module(void)
{
return mark_install_hook();
}

void cleanup_module(void)
{
printk("Removing hook\n");
*target_mark_jump_over = saved_over;
*target_mark_call = __mark_empty_function;

/* Wait for instrumentation functions to return before quitting */
synchronize_sched();
}

What if multiple people hook onto the same mark? Are you assuming LIFO?

J
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/