debuggers.hg

changeset 22730:0e49e2590462

timer: Ensure that CPU field of a timer is read safely when lock-free.

Firstly, all updates must use atomic_write16(), and lock-free reads
must use atomic_read16(). Secondly, we ensure ->cpu is the only field
accessed without a lock. This requires us to place a special sentinel
value in that field when a timer is killed, to avoid needing to read
->status outside a locked critical section.

Signed-off-by: Keir Fraser <keir@xen.org>
author Keir Fraser <keir@xen.org>
date Sat Jan 08 10:09:44 2011 +0000 (2011-01-08)
parents 533d6e5c0099
children 1a64415c959f
files xen/common/timer.c xen/include/xen/timer.h
line diff
     1.1 --- a/xen/common/timer.c	Sat Jan 08 10:05:55 2011 +0000
     1.2 +++ b/xen/common/timer.c	Sat Jan 08 10:09:44 2011 +0000
     1.3 @@ -23,6 +23,7 @@
     1.4  #include <xen/symbols.h>
     1.5  #include <asm/system.h>
     1.6  #include <asm/desc.h>
     1.7 +#include <asm/atomic.h>
     1.8  
     1.9  /* We program the time hardware this far behind the closest deadline. */
    1.10  static unsigned int timer_slop __read_mostly = 50000; /* 50 us */
    1.11 @@ -238,15 +239,14 @@ static inline bool_t timer_lock(struct t
    1.12  
    1.13      for ( ; ; )
    1.14      {
    1.15 -        cpu = timer->cpu;
    1.16 -        if ( unlikely(timer->status == TIMER_STATUS_killed) )
    1.17 +        cpu = atomic_read16(&timer->cpu);
    1.18 +        if ( unlikely(cpu == TIMER_CPU_status_killed) )
    1.19          {
    1.20              rcu_read_unlock(&timer_cpu_read_lock);
    1.21              return 0;
    1.22          }
    1.23          spin_lock(&per_cpu(timers, cpu).lock);
    1.24 -        if ( likely(timer->cpu == cpu) &&
    1.25 -             likely(timer->status != TIMER_STATUS_killed) )
    1.26 +        if ( likely(timer->cpu == cpu) )
    1.27              break;
    1.28          spin_unlock(&per_cpu(timers, cpu).lock);
    1.29      }
    1.30 @@ -292,7 +292,7 @@ void init_timer(
    1.31      memset(timer, 0, sizeof(*timer));
    1.32      timer->function = function;
    1.33      timer->data = data;
    1.34 -    timer->cpu = cpu;
    1.35 +    atomic_write16(&timer->cpu, cpu);
    1.36      timer->status = TIMER_STATUS_inactive;
    1.37      if ( !timer_lock_irqsave(timer, flags) )
    1.38          BUG();
    1.39 @@ -335,7 +335,7 @@ void stop_timer(struct timer *timer)
    1.40  
    1.41  void migrate_timer(struct timer *timer, unsigned int new_cpu)
    1.42  {
    1.43 -    int old_cpu;
    1.44 +    unsigned int old_cpu;
    1.45      bool_t active;
    1.46      unsigned long flags;
    1.47  
    1.48 @@ -343,8 +343,8 @@ void migrate_timer(struct timer *timer, 
    1.49  
    1.50      for ( ; ; )
    1.51      {
    1.52 -        if ( ((old_cpu = timer->cpu) == new_cpu) ||
    1.53 -             unlikely(timer->status == TIMER_STATUS_killed) )
    1.54 +        old_cpu = atomic_read16(&timer->cpu);
    1.55 +        if ( (old_cpu == new_cpu) || (old_cpu == TIMER_CPU_status_killed) )
    1.56          {
    1.57              rcu_read_unlock(&timer_cpu_read_lock);
    1.58              return;
    1.59 @@ -361,8 +361,7 @@ void migrate_timer(struct timer *timer, 
    1.60              spin_lock(&per_cpu(timers, old_cpu).lock);
    1.61          }
    1.62  
    1.63 -        if ( likely(timer->cpu == old_cpu) &&
    1.64 -             likely(timer->status != TIMER_STATUS_killed) )
    1.65 +        if ( likely(timer->cpu == old_cpu) )
    1.66               break;
    1.67  
    1.68          spin_unlock(&per_cpu(timers, old_cpu).lock);
    1.69 @@ -376,7 +375,7 @@ void migrate_timer(struct timer *timer, 
    1.70          deactivate_timer(timer);
    1.71  
    1.72      list_del(&timer->inactive);
    1.73 -    timer->cpu = new_cpu;
    1.74 +    atomic_write16(&timer->cpu, new_cpu);
    1.75      list_add(&timer->inactive, &per_cpu(timers, new_cpu).inactive);
    1.76  
    1.77      if ( active )
    1.78 @@ -389,7 +388,7 @@ void migrate_timer(struct timer *timer, 
    1.79  
    1.80  void kill_timer(struct timer *timer)
    1.81  {
    1.82 -    int           cpu;
    1.83 +    unsigned int old_cpu, cpu;
    1.84      unsigned long flags;
    1.85  
    1.86      BUG_ON(this_cpu(timers).running == timer);
    1.87 @@ -402,8 +401,10 @@ void kill_timer(struct timer *timer)
    1.88  
    1.89      list_del(&timer->inactive);
    1.90      timer->status = TIMER_STATUS_killed;
    1.91 +    old_cpu = timer->cpu;
    1.92 +    atomic_write16(&timer->cpu, TIMER_CPU_status_killed);
    1.93  
    1.94 -    timer_unlock_irqrestore(timer, flags);
    1.95 +    spin_unlock_irqrestore(&per_cpu(timers, old_cpu).lock, flags);
    1.96  
    1.97      for_each_online_cpu ( cpu )
    1.98          while ( per_cpu(timers, cpu).running == timer )
    1.99 @@ -561,7 +562,7 @@ static void migrate_timers_from_cpu(unsi
   1.100      while ( (t = GET_HEAP_SIZE(ts->heap) ? ts->heap[1] : ts->list) != NULL )
   1.101      {
   1.102          remove_entry(t);
   1.103 -        t->cpu = 0;
   1.104 +        atomic_write16(&t->cpu, 0);
   1.105          notify |= add_entry(t);
   1.106      }
   1.107  
   1.108 @@ -569,7 +570,7 @@ static void migrate_timers_from_cpu(unsi
   1.109      {
   1.110          t = list_entry(ts->inactive.next, struct timer, inactive);
   1.111          list_del(&t->inactive);
   1.112 -        t->cpu = 0;
   1.113 +        atomic_write16(&t->cpu, 0);
   1.114          list_add(&t->inactive, &per_cpu(timers, 0).inactive);
   1.115      }
   1.116  
     2.1 --- a/xen/include/xen/timer.h	Sat Jan 08 10:05:55 2011 +0000
     2.2 +++ b/xen/include/xen/timer.h	Sat Jan 08 10:09:44 2011 +0000
     2.3 @@ -32,6 +32,7 @@ struct timer {
     2.4      void *data;
     2.5  
     2.6      /* CPU on which this timer will be installed and executed. */
     2.7 +#define TIMER_CPU_status_killed 0xffffu /* Timer is TIMER_STATUS_killed */
     2.8      uint16_t cpu;
     2.9  
    2.10      /* Timer status. */