lkml.org 
[lkml]   [2006]   [Nov]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [patch 01/19] hrtimers: state tracking
    From
    Date

    > +/*
    > + * 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...



    > - if (base->cpu_base->curr_timer != timer)
    > + if (!(timer->state & HRTIMER_STATE_CALLBACK))
    > ret = remove_hrtimer(timer, base);

    if there is a hrtimer_is_queued() inline, might as well make a
    hrtimer_is_running() inline as well


    otherwise lookes ok; if you fix these few comments:
    Acked-by: Arjan van de Ven <arjan@linux.intel.com>

    --
    if you want to mail me at work (you don't), use arjan (at) linux.intel.com
    Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org

    -
    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:23    [W:0.028 / U:2.468 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site