On Thu, Nov 17, 2016 at 5:32 AM, <caifeng.zhu@xxxxxxxxxxx> wrote:
> Currently dout control is based on module and trace level.
> In practice, we find that it is much easier to control dout
> by specifying module, file, function and line, as done by
> kernel rbd/cephfs client module. The paragraphs below try to
> propose a way to implement similar control at the ceph server
> side.
>
> As the kernel clients, we can define control points (called
> doutpoints later) for dout in the macro dout_impl and collect
> them into a seperate ELF section, like below:
>
> #define dout_impl(cct, sub, v) \
> do { \
> - if (cct->_conf->subsys.should_gather(sub, v)) { \
> + static doutpoint_t *_dop; \
> + DOUTPOINT(TOSTR(sub), __FILE__, __func__, __LINE__); \
> + \
> + if (cct->_conf->subsys.should_gather(sub, v) || \
> + doutpoint_active(_dop)) { \
>
> In the above code, DOUTPOINT will define a doutpoint and ask linker
> to collect it into an ELF section, such as "set_doutpoint". Instead of
> adding __attribute__((section("set_doutpoint"))) to variables, we resort
> to inline ASM like below:
>
> +typedef struct doutpoint {
> + const char * dop_module; // module
> + const char * dop_file; // file name
> + const char * dop_func; // function name
> + int dop_line; // line no
> + volatile int dop_state; // state 0:off, 1:on
> +} doutpoint_t;
>
> +#define TOSTR(a) #a
> +#define DOUTPOINT(module, file, func, line) \
> +do { \
> + __asm__(".pushsection set_doutpoint, \"aw\", @progbits" "\n" \
> + ".align 32" "\n" \
> + "416:" "\n" \
> + ".quad %c1" "\n" \
> + ".quad %c2" "\n" \
> + ".quad %c3" "\n" \
> + ".long %c4" "\n" \
> + ".long 0" "\n" \
> + ".popsection" "\n" \
> + "leaq 416b(%%rip), %0" "\n" \
> + : "=r"(_dop) \
> + : "i"(module), "i"(file), "i"(func), "i"(line)); \
> +} while(0)
>
> The reason for inline ASM is that: in C++, there are inline and template
> functions. static variables defined in these functions will be automatically
> added attributes by the compiler. thus it impossible to collect all doutpoints
> into one section.
>
> There is another problem to consider: collect doutpoints of shared libs into
> the main executable. We solve the problem by defining a constructor function
> and a weak function. The contructor function will automatically register doupoints
> of shared libs into the main executable. The weak function is to be overriden by
> the main executable, providing where to register doutponits.
>
> //
> // in common/dout.h
> //
> +typedef struct doutsection {
> + doutpoint_t * start;
> + doutpoint_t * stop;
> +} doutsection_t;
>
> //
> // in common/dout.cc
> //
> +extern doutpoint_t __start_set_doutpoint[]
> + __attribute__((weak, visibility("hidden")));
> +extern doutpoint_t __stop_set_doutpoint[]
> + __attribute__((weak, visibility("hidden")));
> +
> +extern void doutsection_global(doutsection_t **s, int *n)
> + __attribute__((weak));
> +
> +
> +static void
> +doutsection_array(doutsection_t **s, int *n)
> +{
> + static doutsection_t _sections[10];
> +
> + /*
> + * If doutsection_global is defined, provide
> + * from the global one; otherwise, fall back
> + * to local ones.
> + */
> + if (doutsection_global)
> + doutsection_global(s, n);
> + else {
> + *s = _sections;
> + *n = 10;
> + }
> +}
> +
> +/*
> + * The 'contructor' attribute means the function will be
> + * called after dynamic linking for the library or before
> + * entering into main for the executable.
> + */
> +void __attribute__((constructor))
> +doutpoint_register(void)
> +{
> + int i, n;
> + doutsection_t *s;
> +
> + doutsection_array(&s, &n);
> + for (i = 0; i < n; i++, s++) {
> + if (!s->start) {
> + s->start = __start_set_doutpoint;
> + s->stop = __stop_set_doutpoint;
> + return;
> + }
> + }
> +}
>
> //
> // in global/global_init.cc
> //
> void
> doutsection_global(doutsection_t **s, int *n)
> {
> static doutsection_t _sections[10];
>
> *s = _sections;
> *n = 10;
> }
>
> There are some shortcomings of the proposal: mainly archtecture
> dependency and compiler dependency. DOUTPOINT exploits one x86
> instruction; weak symbol support may be different on different
> compilers.
>
> Glad and eager to hear your opinions!
> If it is useful, I'll post the whole patch.
As someone who has experience with kernel dynamic_debug mechanism,
I wish we had a subsystem/level mechanism in the kernel client instead.
While sometimes handy, enabling only a few selected douts doesn't come
useful that often - more frequently the dout you really need isn't
there and you end up fiddling with "perf probe" and others, longing for
a good scriptable dynamic tracer.
It looks like you are proposing this as an addition to the existing
module/level scheme and not as a replacement though, so it's just an
observation...
Thanks,
Ilya
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
