lkml.org 
[lkml]   [2006]   [Sep]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] Linux Markers 0.4 (+dynamic probe loader) for 2.6.17
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@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-09-21 02:17    [W:0.080 / U:0.444 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site