linux-kernel - Re: [PATCH V2 2/2] tick/cpuidle: Initialize hrtimer mode of broadcast

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]

Message-ID: <alpine.DEB.2.02.1402041101330.24986@ionos.tec.linutronix.de>
Date:	Tue, 4 Feb 2014 11:18:02 +0100 (CET)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Preeti U Murthy <preeti@...ux.vnet.ibm.com>
cc:	daniel.lezcano@...aro.org, peterz@...radead.org,
	fweisbec@...il.com, galak@...nel.crashing.org,
	paul.gortmaker@...driver.com, paulus@...ba.org, mingo@...nel.org,
	mikey@...ling.org, shangw@...ux.vnet.ibm.com,
	rafael.j.wysocki@...el.com, agraf@...e.de,
	benh@...nel.crashing.org, paulmck@...ux.vnet.ibm.com,
	arnd@...db.de, linux-pm@...r.kernel.org, rostedt@...dmis.org,
	michael@...erman.id.au, john.stultz@...aro.org, anton@...ba.org,
	chenhui.zhao@...escale.com, deepthi@...ux.vnet.ibm.com,
	r58472@...escale.com, geoff@...radead.org,
	linux-kernel@...r.kernel.org, srivatsa.bhat@...ux.vnet.ibm.com,
	schwidefsky@...ibm.com, svaidy@...ux.vnet.ibm.com,
	linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH V2 2/2] tick/cpuidle: Initialize hrtimer mode of
 broadcast

> +++ b/kernel/time/tick-broadcast-hrtimer.c
> +/*
> + * This is called from the guts of the broadcast code when the cpu
> + * which is about to enter idle has the earliest broadcast timer event.
> + */
> +static int bc_set_next(ktime_t expires, struct clock_event_device *bc)
> +{
> +	ktime_t now, interval;
> +	/*
> +	 * We try to cancel the timer first. If the callback is on
> +	 * flight on some other cpu then we let it handle it. If we
> +	 * were able to cancel the timer nothing can rearm it as we
> +	 * own broadcast_lock.
> +	 *
> +	 * However if we are called from the hrtimer interrupt handler
> +	 * itself, reprogram it.
> +	 */
> +	if (hrtimer_try_to_cancel(&bctimer) >= 0) {
> +		hrtimer_start(&bctimer, expires, HRTIMER_MODE_ABS_PINNED);
> +		/* Bind the "device" to the cpu */
> +		bc->bound_on = smp_processor_id();
> +	} else if (bc->bound_on == smp_processor_id()) {

This part really wants a proper comment. It took me a while to figure
out why this is correct and what the call chain is.


> +		now = ktime_get();
> +		interval = ktime_sub(expires, now);
> +		hrtimer_forward_now(&bctimer, interval);

We are in the event handler called from bc_handler() and expires is
absolute time. So what's wrong with calling
hrtimer_set_expires(&bctimer, expires)?

> +static enum hrtimer_restart bc_handler(struct hrtimer *t)
> +{
> +	ce_broadcast_hrtimer.event_handler(&ce_broadcast_hrtimer);
> +	return HRTIMER_RESTART;

We probably want to check whether the timer needs to be restarted at
all.

	if (ce_broadcast_timer.next_event.tv64 == KTIME_MAX)
	   return HRTIMER_NORESTART;

	return HRTIMER_RESTART;

Hmm?

Thanks,

	tglx
--
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/