linux-kernel - Re: [RFC 1/7] hrtimer: Warn if hrtimer

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.10.1407100141090.4357@nanos>
Date:	Thu, 10 Jul 2014 01:58:16 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Frederic Weisbecker <fweisbec@...il.com>
cc:	Viresh Kumar <viresh.kumar@...aro.org>,
	linaro-kernel@...ts.linaro.org, linux-kernel@...r.kernel.org,
	arvind.chauhan@....com, preeti@...ux.vnet.ibm.com,
	khilman@...aro.org
Subject: Re: [RFC 1/7] hrtimer: Warn if hrtimer_start*() failed to enqueue
 hrtimer

On Thu, 10 Jul 2014, Frederic Weisbecker wrote:

> On Wed, Jul 09, 2014 at 12:25:33PM +0530, Viresh Kumar wrote:
> > hrtimer_start*() family never fails to enqueue a hrtimer to a clock-base. The
> > only special case is when the hrtimer was in past. If it is getting enqueued to
> > local CPUs's clock-base, we raise a softirq and exit, else we handle that on
> > next interrupt on remote CPU.
> > 
> > At several places in kernel we check if hrtimer is enqueued properly with
> > hrtimer_active(). This isn't required and can be dropped.
> > 
> > Before fixing that, lets make sure hrtimer is always enqueued properly by adding
> > 
> > 	WARN_ON_ONCE(!hrtimer_active(timer));
> > 
> > towards the end of __hrtimer_start_range_ns().
> > 
> > Suggested-by: Frederic Weisbecker <fweisbec@...il.com>
> > Signed-off-by: Viresh Kumar <viresh.kumar@...aro.org>
> > ---
> >  kernel/hrtimer.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
> > index 3ab2899..cf40209 100644
> > --- a/kernel/hrtimer.c
> > +++ b/kernel/hrtimer.c
> > @@ -1037,6 +1037,8 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
> >  
> >  	unlock_hrtimer_base(timer, &flags);
> >  
> > +	/* Make sure timer is enqueued */
> > +	WARN_ON_ONCE(!hrtimer_active(timer));
> 
> Hmm, after reading Thomas reply, I think it's possible that the hrtimer expires
> right after we unlock it and, if we are unlucky enough, before the hrtimer_active()
> check.
> 
> In this case we might hit a false positive.

Haha, I didn't even go down to this patch after reading 0/N. I knew
right there that it's going to be pointless shite.

But now that you point me to it, it's even worse. It's not only
pointless shite it's actively wrong and outright stupid for someone
who tries to "work" on this code for a couple of month now.

Viresh, I'm really tired of this. Stop touching code you do not
understand. I warned you more than once and now you really reached the
level of complete incompetence. Welcome to my killfile.

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/