1. 23 May, 2008 1 commit
    • Mathieu Desnoyers's avatar
      Markers - remove extra format argument · dc102a8f
      Mathieu Desnoyers authored
      
      Denys Vlasenko <vda.linux@googlemail.com> :
      
      > Not in this patch, but I noticed:
      >
      > #define __trace_mark(name, call_private, format, args...)               \
      >         do {                                                            \
      >                 static const char __mstrtab_##name[]                    \
      >                 __attribute__((section("__markers_strings")))           \
      >                 = #name "\0" format;                                    \
      >                 static struct marker __mark_##name                      \
      >                 __attribute__((section("__markers"), aligned(8))) =     \
      >                 { __mstrtab_##name, &__mstrtab_##name[sizeof(#name)],   \
      >                 0, 0, marker_probe_cb,                                  \
      >                 { __mark_empty_function, NULL}, NULL };                 \
      >                 __mark_check_format(format, ## args);                   \
      >                 if (unlikely(__mark_##name.state)) {                    \
      >                         (*__mark_##name.call)                           \
      >                                 (&__mark_##name, call_private,          \
      >                                 format, ## args);                       \
      >                 }                                                       \
      >         } while (0)
      >
      > In this call:
      >
      >                         (*__mark_##name.call)                           \
      >                                 (&__mark_##name, call_private,          \
      >                                 format, ## args);                       \
      >
      > you make gcc allocate duplicate format string. You can use
      > &__mstrtab_##name[sizeof(#name)] instead since it holds the same string,
      > or drop ", format," above and "const char *fmt" from here:
      >
      >         void (*call)(const struct marker *mdata,        /* Probe wrapper */
      >                 void *call_private, const char *fmt, ...);
      >
      > since mdata->format is the same and all callees which need it can take it there.
      
      Very good point. I actually thought about dropping it, since it would
      remove an unnecessary argument from the stack. And actually, since I now
      have the marker_probe_cb sitting between the marker site and the
      callbacks, there is no API change required. Thanks :)
      
      Mathieu
      Signed-off-by: default avatarMathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
      CC: Denys Vlasenko <vda.linux@googlemail.com>
      Signed-off-by: default avatarIngo Molnar <mingo@elte.hu>
      Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
      dc102a8f
  2. 30 Apr, 2008 1 commit
  3. 29 Apr, 2008 1 commit
  4. 02 Apr, 2008 1 commit
    • Mathieu Desnoyers's avatar
      markers: use synchronize_sched() · 6496968e
      Mathieu Desnoyers authored
      
      Markers do not mix well with CONFIG_PREEMPT_RCU because it uses
      preempt_disable/enable() and not rcu_read_lock/unlock for minimal
      intrusiveness.  We would need call_sched and sched_barrier primitives.
      
      Currently, the modification (connection and disconnection) of probes
      from markers requires changes to the data structure done in RCU-style :
      a new data structure is created, the pointer is changed atomically, a
      quiescent state is reached and then the old data structure is freed.
      
      The quiescent state is reached once all the currently running
      preempt_disable regions are done running.  We use the call_rcu mechanism
      to execute kfree() after such quiescent state has been reached.
      However, the new CONFIG_PREEMPT_RCU version of call_rcu and rcu_barrier
      does not guarantee that all preempt_disable code regions have finished,
      hence the race.
      
      The "proper" way to do this is to use rcu_read_lock/unlock, but we don't
      want to use it to minimize intrusiveness on the traced system.  (we do
      not want the marker code to call into much of the OS code, because it
      would quickly restrict what can and cannot be instrumented, such as the
      scheduler).
      
      The temporary fix, until we get call_rcu_sched and rcu_barrier_sched in
      mainline, is to use synchronize_sched before each call_rcu calls, so we
      wait for the quiescent state in the system call code path.  It will slow
      down batch marker enable/disable, but will make sure the race is gone.
      Signed-off-by: default avatarMathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
      Acked-by: default avatarPaul E. McKenney <paulmck@linux.vnet.ibm.com>
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      6496968e
  5. 25 Mar, 2008 2 commits
  6. 05 Mar, 2008 1 commit
  7. 24 Feb, 2008 1 commit
  8. 14 Feb, 2008 1 commit
    • Mathieu Desnoyers's avatar
      Linux Kernel Markers: support multiple probes · fb40bd78
      Mathieu Desnoyers authored
      
      RCU style multiple probes support for the Linux Kernel Markers.  Common case
      (one probe) is still fast and does not require dynamic allocation or a
      supplementary pointer dereference on the fast path.
      
      - Move preempt disable from the marker site to the callback.
      
      Since we now have an internal callback, move the preempt disable/enable to the
      callback instead of the marker site.
      
      Since the callback change is done asynchronously (passing from a handler that
      supports arguments to a handler that does not setup the arguments is no
      arguments are passed), we can safely update it even if it is outside the
      preempt disable section.
      
      - Move probe arm to probe connection. Now, a connected probe is automatically
        armed.
      
      Remove MARK_MAX_FORMAT_LEN, unused.
      
      This patch modifies the Linux Kernel Markers API : it removes the probe
      "arm/disarm" and changes the probe function prototype : it now expects a
      va_list * instead of a "...".
      
      If we want to have more than one probe connected to a marker at a given
      time (LTTng, or blktrace, ssytemtap) then we need this patch. Without it,
      connecting a second probe handler to a marker will fail.
      
      It allow us, for instance, to do interesting combinations :
      
      Do standard tracing with LTTng and, eventually, to compute statistics
      with SystemTAP, or to have a special trigger on an event that would call
      a systemtap script which would stop flight recorder tracing.
      Signed-off-by: default avatarMathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
      Cc: Christoph Hellwig <hch@infradead.org>
      Cc: Mike Mason <mmlnx@us.ibm.com>
      Cc: Dipankar Sarma <dipankar@in.ibm.com>
      Cc: David Smith <dsmith@redhat.com>
      Cc: "Paul E. McKenney" <paulmck@us.ibm.com>
      Cc: "Frank Ch. Eigler" <fche@redhat.com>
      Cc: Steven Rostedt <rostedt@goodmis.org>
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      fb40bd78
  9. 15 Nov, 2007 1 commit
  10. 19 Oct, 2007 1 commit