An error occurred fetching the project authors.
  1. 14 Oct, 2008 1 commit
    • Lai Jiangshan's avatar
      markers: fix unregister bug and reenter bug · d74185ed
      Lai Jiangshan authored
      unregister bug:
      
      codes using makers are typically calling marker_probe_unregister()
      and then destroying the data that marker_probe_func needs(or
      unloading this module). This is bug when the corresponding
      marker_probe_func is still running(on other cpus),
      it is using the destroying/ed data.
      
      we should call synchronize_sched() after marker_update_probes().
      
      reenter bug:
      
      marker_probe_register(), marker_probe_unregister() and
      marker_probe_unregister_private_data() are not reentrant safe
      functions. these 3 functions release markers_mutex and then
      require it again and do "entry->oldptr = old; ...", but entry->oldptr
      maybe is using now for these 3 functions may reenter when markers_mutex
      is released.
      
      we use synchronize_sched() instead of call_rcu_sched() to fix
      this bug. actually we can do:
      "
      if (entry->rcu_pending)
      		rcu_barrier_sched();
      "
      after require markers_mutex again. but synchronize_sched()
      is better and simpler. For these 3 functions are not critical path.
      Signed-off-by: default avatarLai Jiangshan <laijs@cn.fujitsu.com>
      Cc: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
      Signed-off-by: default avatarIngo Molnar <mingo@elte.hu>
      d74185ed
  2. 30 Jul, 2008 1 commit
  3. 25 Jul, 2008 1 commit
  4. 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
  5. 30 Apr, 2008 1 commit
  6. 29 Apr, 2008 1 commit
  7. 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
  8. 25 Mar, 2008 2 commits
  9. 05 Mar, 2008 1 commit
  10. 24 Feb, 2008 1 commit
  11. 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
  12. 15 Nov, 2007 1 commit
  13. 19 Oct, 2007 1 commit