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

From: Michael Ellerman
Date: Mon Mar 29 2010 - 18:10:15 EST


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




Attachment: signature.asc
Description: This is a digitally signed message part