linux-kernel - Re: [PATCH v2 3/3] sched: increase SCHED_LOAD

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: <20110518202646.GA14141@elte.hu>
Date:	Wed, 18 May 2011 22:26:46 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Nikhil Rao <ncrao@...gle.com>
Cc:	Peter Zijlstra <peterz@...radead.org>,
	Mike Galbraith <efault@....de>, linux-kernel@...r.kernel.org,
	"Nikunj A. Dadhania" <nikunj@...ux.vnet.ibm.com>,
	Srivatsa Vaddagiri <vatsa@...ux.vnet.ibm.com>,
	Stephan Barwolf <stephan.baerwolf@...ilmenau.de>
Subject: Re: [PATCH v2 3/3] sched: increase SCHED_LOAD_SCALE resolution

* Nikhil Rao <ncrao@...gle.com> wrote:

> +#if BITS_PER_LONG > 32
> +# define SCHED_LOAD_RESOLUTION	10
> +# define scale_load(w)		(w << SCHED_LOAD_RESOLUTION)
> +# define scale_load_down(w)	(w >> SCHED_LOAD_RESOLUTION)
> +#else
> +# define SCHED_LOAD_RESOLUTION	0
> +# define scale_load(w)		(w)
> +# define scale_load_down(w)	(w)
> +#endif

We want (w) in the other definitions as well, to protect potential operators 
with lower precedence than <<. (Roughly half of the C operators are such so 
it's a real issue, should anyone use these macros with operators.)

> +#define SCHED_LOAD_SHIFT	(10 + (SCHED_LOAD_RESOLUTION))

that () is not needed actually, if you look at the definition of 
SCHED_LOAD_RESOLUTION.

So you could move the superfluous () from here up to the two definitions above 
and thus no parentheses would be hurt during the making of this patch.

> +		if (BITS_PER_LONG > 32 && unlikely(w >= WMULT_CONST))
>  			lw->inv_weight = 1;
> +		else if (unlikely(!w))
> +			lw->inv_weight = WMULT_CONST;
>  		else
> +			lw->inv_weight = WMULT_CONST / w;

Ok, i just noticed that you made use of BITS_PER_LONG here too.

It's better to put that into a helper define, something like 
SCHED_LOAD_HIGHRES, which could thus be used like this:

		if (SCHED_LOAD_HIGHRES && unlikely(w >= WMULT_CONST))

then, should anyone want to tweak the condition for SCHED_LOAD_HIGHRES, it 
could be done in a single place. It would also self-document.

Thanks,

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