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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ