lkml.org 
[lkml]   [2006]   [Nov]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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/

\
 
 \ /
  Last update: 2006-11-10 10:47    [W:0.061 / U:0.472 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site