linux-kernel - Re: [v4 RFC PATCH 4/4] timers: logic to move non pinned timers

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.LFD.2.00.0904061732020.21577@localhost.localdomain>
Date:	Mon, 6 Apr 2009 17:35:09 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Arun R Bharadwaj <arun@...ux.vnet.ibm.com>
cc:	linux-kernel@...r.kernel.org, linux-pm@...ts.linux-foundation.org,
	a.p.zijlstra@...llo.nl, ego@...ibm.com, mingo@...e.hu,
	andi@...stfloor.org, venkatesh.pallipadi@...el.com,
	vatsa@...ux.vnet.ibm.com, arjan@...radead.org,
	svaidy@...ux.vnet.ibm.com
Subject: Re: [v4 RFC PATCH 4/4] timers: logic to move non pinned timers

Arun,

On Mon, 6 Apr 2009, Arun R Bharadwaj wrote:
> > > -	new_cpu_base = &__get_cpu_var(hrtimer_bases);
> > > +again:
> > > +	new_cpu_base = &per_cpu(hrtimer_bases, cpu);
> > >  	new_base = &new_cpu_base->clock_base[base->index];
> > >  
> > >  	if (base != new_base) {
> > > @@ -220,6 +242,32 @@ switch_hrtimer_base(struct hrtimer *time
> > >  		spin_unlock(&base->cpu_base->lock);
> > >  		spin_lock(&new_base->cpu_base->lock);
> > >  		timer->base = new_base;
> > > +
> > > +		if (cpu == preferred_cpu) {
> > > +			/* Calculate clock monotonic expiry time */
> > > +			ktime_t expires = ktime_sub(hrtimer_get_expires(timer),
> > > +							new_base->offset);
> > > +
> > > +			/*
> > > +			 * Get the next event on target cpu from the
> > > +			 * clock events layer.
> > > +			 * This covers the highres=off nohz=on case as well.
> > > +			 */
> > > +			ktime_t next = clockevents_get_next_event(cpu);
> > > +
> > > +			ktime_t delta = ktime_sub(expires, next);
> > > +
> > > +			/*
> > > +			 * We do not migrate the timer when it is expiring
> > > +			 * before the next event on the target cpu because
> > > +			 * we cannot reprogram the target cpu hardware and
> > > +			 * we would cause it to fire late.
> > > +			 */
> > > +			if (delta.tv64 < 0) {
> > > +				cpu = smp_processor_id();
> > 
> >   You are missing a small but fatal detail here: You hold
> >   new_base->cpu_base->lock. So you need to do:
> >
> 
> I just moved the if block.. if (cpu==preferred_cpu) above the base
> locking part to avoid the extra unlocking.

That's not a good idea. You want to look at the next event with the
base lock of the other cpu held. That prevents that expires the first
pending timer right after you checked next_event and before you queue
your timer, which then might become the first timer to expire but you
can't reprogram the clock event device on the other cpu.

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/