linux-kernel - Re: [PATCH -v9 00/31] use lmb with x86

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: <1269900605.2286.2.camel@concordia>
Date:	Tue, 30 Mar 2010 09:10:05 +1100
From:	Michael Ellerman <michael@...erman.id.au>
To:	Yinghai Lu <yinghai@...nel.org>
Cc:	Ingo Molnar <mingo@...e.hu>, Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	David Miller <davem@...emloft.net>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Johannes Weiner <hannes@...xchg.org>,
	linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org
Subject: Re: [PATCH -v9 00/31] use lmb with x86

On Mon, 2010-03-29 at 09:52 -0700, Yinghai Lu wrote:
> On 03/29/2010 05:22 AM, Michael Ellerman wrote:
> > On Sun, 2010-03-28 at 19:42 -0700, Yinghai Lu wrote:
> >> the new lmb could be used to early_res in x86.
> >>
> >> Suggested by: David, Ben, and Thomas
> >>
> >> First three patches should go into 2.6.34
> >>
> >> -v6: change sequence as requested by Thomas
> >> -v7: seperate them to more patches
> >> -v8: add boundary checking to make sure not free partial page.
> >> -v9: use lmb_debug to control print out of reserve_lmb.
> >>      add e820 clean up, and e820 become __initdata
> > 
> > Bike shedding perhaps, but can you maintain the naming convention, ie.
> > lmb_xxx() rather than xxx_lmb(). Neither is necessarily better, but all
> > the existing functions use the lmb_xxx() style.
> > 
> 
> so you want
> 
> find_lmb_area ==> lmb_find_area
> reserve_lmb ==> lmb_reserve
> free_lmb ==> lmb_free
> 
> first one is ok, 
> 
> but next two we already have lmb_reserved and lmb_free without checking and increasing the size of region array.

That was the point of my other mail. We now have two lmb APIs, one which
checks if the array will overflow and one which doesn't. That seems like
a bad idea. Having one called lmb_free() and one called free_lmb() is
definitely a bad idea, because it's completely non obvious which one
caters for overflow.

cheers





Download attachment "signature.asc" of type "application/pgp-signature" (198 bytes)