Messages in this thread | | | Date | Fri, 10 Nov 2006 01:40:53 -0800 | From | Andrew Morton <> | Subject | Re: [patch 01/19] hrtimers: state tracking |
| |
On Fri, 10 Nov 2006 10:19:48 +0100 Arjan van de Ven <arjan@infradead.org> wrote:
> > > +/* > > + * Bit values to track state of the timer > > + * > > + * Possible states: > > + * > > + * 0x00 inactive > > + * 0x01 enqueued into rbtree > > + * 0x02 callback function running > > + * 0x03 callback function running and enqueued > > + * (was requeued on another CPU) > > + * > > + * The "callback function running and enqueued" status is only possible on > > + * SMP. It happens for example when a posix timer expired and the callback > > + * queued a signal. Between dropping the lock which protects the posix timer > > + * and reacquiring the base lock of the hrtimer, another CPU can deliver the > > + * signal and rearm the timer. We have to preserve the callback running state, > > + * as otherwise the timer could be removed before the softirq code finishes the > > + * the handling of the timer. > > + * > > + * The HRTIMER_STATE_ENQUEUE bit is always or'ed to the current state to > > + * preserve the HRTIMER_STATE_CALLBACK bit in the above scenario. > > + * > > + * All state transitions are protected by cpu_base->lock. > > + */ > > +#define HRTIMER_STATE_INACTIVE 0x00 > > +#define HRTIMER_STATE_ENQUEUED 0x01 > > +#define HRTIMER_STATE_CALLBACK 0x02 > > where is the define for 0x03? > > > > > +static inline int hrtimer_is_queued(struct hrtimer *timer) > > +{ > > + return timer->state != HRTIMER_STATE_INACTIVE && > > + timer->state != HRTIMER_STATE_CALLBACK; > > +} > > the state things are either bits or they're not. If they're bits, you > probably want to make this a bitcheck instead... > > rb_insert_color(&timer->node, &base->active); > > + /* > > + * HRTIMER_STATE_ENQUEUED is or'ed to the current state to preserve the > > + * state of a possibly running callback. > > + */ > > + timer->state |= HRTIMER_STATE_ENQUEUED; > > ok so it IS a bit thing, see comment about hrtimer_is_queued() not being > a bit check then... >
eek. I exhaustively went over that confusion in my initial (and lengthy) review of these patches.
I don't think we ever saw a point-by-point reply. What got lost? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |