linux-kernel - Re: [patch 01/19] hrtimers: state tracking

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 10 Nov 2006 10:19:48 +0100
From:	Arjan van de Ven <arjan@...radead.org>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	Andrew Morton <akpm@...l.org>, LKML <linux-kernel@...r.kernel.org>,
	Ingo Molnar <mingo@...e.hu>, Len Brown <lenb@...nel.org>,
	John Stultz <johnstul@...ibm.com>, Andi Kleen <ak@...e.de>,
	Roman Zippel <zippel@...ux-m68k.org>
Subject: Re: [patch 01/19] hrtimers: state tracking


> +/*
> + * 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@...ux.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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/