linux-kernel - Re: [PATCH 4 of 7] lguest: Config and headers

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:	Sat, 10 Feb 2007 10:41:55 +1100
From:	Rusty Russell <rusty@...tcorp.com.au>
To:	James Morris <jmorris@...ei.org>
Cc:	lkml - Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Andi Kleen <ak@....de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	virtualization <virtualization@...ts.osdl.org>
Subject: Re: [PATCH 4 of 7]  lguest: Config and headers

On Fri, 2007-02-09 at 13:15 -0500, James Morris wrote:
> On Sat, 10 Feb 2007, Rusty Russell wrote:
> 
> > +/* 64k ought to be enough for anybody! */
> > +#define HYPERVISOR_MAP_ORDER 16
> > +#define HYPERVISOR_PAGES ((1 << HYPERVISOR_MAP_ORDER)/PAGE_SIZE)
> 
> I think it'd be better to go back to defining HYPERVISOR_SIZE then derive 
> the map order from that via get_order(), as it should be 4 instead of 16; 
> and this code is now both implying PAGE_SIZE while also using it for 
> calculations. 

Well it was the use of get_order() which triggered Andi's alarm bells,
so I went back to deriving it.  This code is correct, however.

get_order() is one of those classic functions only a kernel coder could
love.  Look how lovingly it has been optimized:

#define get_order(n)							\
(									\
	__builtin_constant_p(n) ?					\
	((n < (1UL << PAGE_SHIFT)) ? 0 : ilog2(n) - PAGE_SHIFT) :	\
	__get_order(n, PAGE_SHIFT)					\
 )

All that time spent, yet no consideration that it should be called
"get_page_order()" or some name which hints that the divide by page size
is happening.  It's even documented in the comment above, so someone
thought it needed explaining.  Too bad they chose to explain it instead
of actually clarifying it. 8(

Cheers,
Rusty.

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