linux-kernel - Re: [PATCH] highres: do not run the TIMER_SOFTIRQ after switching to highres mode

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: <Pine.LNX.4.64.0703060935310.5963@woody.linux-foundation.org>
Date:	Tue, 6 Mar 2007 09:41:33 -0800 (PST)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Thomas Gleixner <tglx@...utronix.de>
cc:	Ingo Molnar <mingo@...e.hu>,
	"Michael S. Tsirkin" <mst@...lanox.co.il>,
	Adrian Bunk <bunk@...sta.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Michal Piotrowski <michal.k.k.piotrowski@...il.com>,
	Emil Karlson <jkarlson@...hut.fi>,
	Soeren Sonnenburg <kernel@....de>, Len Brown <lenb@...nel.org>
Subject: Re: [PATCH] highres: do not run the TIMER_SOFTIRQ after switching
 to highres mode

On Tue, 6 Mar 2007, Thomas Gleixner wrote:
> 
> /me pleads guilty !
> 
> Replacement patch below.

Well, I already applied the original one that came through Andrew, so I 
really just wanted to note the coding style in general, and your fixed 
patch no longer applied ;)

Also, you do end up testing for "not error" with a

>  	if (tick_check_oneshot_change(!hrtimer_is_hres_enabled()))
> -		hrtimer_switch_to_hres();
> +		if (!hrtimer_switch_to_hres())
> +			return;

which I guess is ok, if only because we simply don't care about what the 
exact error was. But it means that this particular code sequence ends up 
having the same problem (which is still fewer places than the original 
patch, so we're good).

I personally hate the

	if (hrtimer_switch_to_hres() == SUCCESS)
		return;

kind of syntax (it's just too long, and it's *not* obvious at all that 
SUCCESS is zero and that this is a "negative error or zero" kind of 
function, so it's actually *worse* than just doing what you did, but some 
projects seem to have that kind of approach.

We could encourage people to do

	if (hrtimer_switch_to_hres() >= 0)
		return;

which is fairly obviously a "success" case for a negative error value, but 
I'm not sure the extra typing really is worth it. Does anybody have any 
smart ideas that people might even be ok with following (just making 
things more cumbersome is anti-productive, so I don't want to have some 
stupid rule that everybody really hates)?

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