1. 16 11月, 2008 2 次提交
  2. 27 10月, 2008 4 次提交
  3. 14 10月, 2008 5 次提交
    • L
      markers: bit-field is not thread-safe nor smp-safe · 1b7ae37c
      Lai Jiangshan 提交于
      bit-field is not thread-safe nor smp-safe.
      
      struct marker_entry.rcu_pending is not protected by any lock
      in rcu-callback free_old_closure().
      so we must turn it into a safe type.
      
      detail:
      
      I suppose rcu_pending and ptype are store in struct marker_entry.tmp1
      
      free_old_closure() side:           change ptype side:
      
                                      |  load struct marker_entry.tmp1
      --------------------------------|--------------------------------
                                      |  change ptype bit in tmp1
      load struct marker_entry.tmp1   |
      change rcu_pending bit in tmp1  |
      store tmp1                      |
      --------------------------------|--------------------------------
                                      |  store tmp1
      
      now this result equals that free_old_closure() do not change rcu_pending
      bit, bug! This bug will cause redundant rcu_barrier_sched() called.
      not too harmful.
      
      ----- corresponding:
      
      free_old_closure() side:           change ptype side:
      
      load struct marker_entry.tmp1   |
      --------------------------------|--------------------------------
                                      |  load struct marker_entry.tmp1
      change rcu_pending bit in tmp1  |
                                      |  change ptype bit in tmp1
                                      |  store tmp1
      --------------------------------|--------------------------------
      store tmp1                      |
      
      now this result equals that change ptype side do not change ptype
      bit, bug! this bug cause marker_probe_cb() access to invalid memory.
      oops!
      
      see also: http://en.wikipedia.org/wiki/Bit_fieldSigned-off-by: NLai Jiangshan <laijs@cn.fujitsu.com>
      Acked-by: NMathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
      Signed-off-by: NIngo Molnar <mingo@elte.hu>
      1b7ae37c
    • L
      markers: fix unchecked format · 48043bcd
      Lai Jiangshan 提交于
      when the second, third... probe is registered, its format is
      not checked, this patch fixes it.
      Signed-off-by: NLai Jiangshan <laijs@cn.fujitsu.com>
      Acked-by: NMathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
      Signed-off-by: NIngo Molnar <mingo@elte.hu>
      48043bcd
    • M
      markers: re-enable fast batch registration · ed86a590
      Mathieu Desnoyers 提交于
      Lai Jiangshan discovered a reentrancy issue with markers and fixed it by
      adding synchronize_sched() calls at each registration/unregistraiton.
      
      It works, but it removes the ability to do batch
      registration/unregistration and can cause registration of ~100 markers
      to take about 30 seconds on a loaded machine (synchronize_sched() is
      much slower on such workloads).
      
      This patch implements a version of the fix which won't slow down marker batch
      registration/unregistration. It also go back to the original non-synchronized
      reg/unreg.
      Signed-off-by: NMathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
      Signed-off-by: NIngo Molnar <mingo@elte.hu>
      ed86a590
    • M
      markers: fix unregister bug and reenter bug, cleanup · e2d3b75d
      Mathieu Desnoyers 提交于
      Use the new rcu_read_lock_sched/unlock_sched() in marker code around the call
      site instead of preempt_disable/enable(). It helps reviewing the code more
      easily.
      Signed-off-by: NMathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
      Signed-off-by: NIngo Molnar <mingo@elte.hu>
      e2d3b75d
    • L
      markers: fix unregister bug and reenter bug · d74185ed
      Lai Jiangshan 提交于
      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: NLai Jiangshan <laijs@cn.fujitsu.com>
      Cc: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
      Signed-off-by: NIngo Molnar <mingo@elte.hu>
      d74185ed
  4. 31 7月, 2008 1 次提交
  5. 26 7月, 2008 1 次提交
  6. 24 5月, 2008 1 次提交
    • M
      Markers - remove extra format argument · dc102a8f
      Mathieu Desnoyers 提交于
      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: NMathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
      CC: Denys Vlasenko <vda.linux@googlemail.com>
      Signed-off-by: NIngo Molnar <mingo@elte.hu>
      Signed-off-by: NThomas Gleixner <tglx@linutronix.de>
      dc102a8f
  7. 30 4月, 2008 1 次提交
  8. 29 4月, 2008 1 次提交
  9. 03 4月, 2008 1 次提交
    • M
      markers: use synchronize_sched() · 6496968e
      Mathieu Desnoyers 提交于
      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: NMathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
      Acked-by: NPaul E. McKenney <paulmck@linux.vnet.ibm.com>
      Signed-off-by: NAndrew Morton <akpm@linux-foundation.org>
      Signed-off-by: NLinus Torvalds <torvalds@linux-foundation.org>
      6496968e
  10. 25 3月, 2008 2 次提交
  11. 05 3月, 2008 1 次提交
  12. 24 2月, 2008 1 次提交
  13. 14 2月, 2008 1 次提交
    • M
      Linux Kernel Markers: support multiple probes · fb40bd78
      Mathieu Desnoyers 提交于
      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: NMathieu 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: NAndrew Morton <akpm@linux-foundation.org>
      Signed-off-by: NLinus Torvalds <torvalds@linux-foundation.org>
      fb40bd78
  14. 15 11月, 2007 1 次提交
  15. 20 10月, 2007 1 次提交