From: Michael Ellerman on
On Mon, 2010-03-29 at 23:12 -0700, Yinghai Lu wrote:
> On 03/29/2010 10:26 PM, Benjamin Herrenschmidt wrote:
> > On Mon, 2010-03-29 at 17:03 -0700, Yinghai Lu wrote:
> >>
> >> in short: It could make us to avoid use the range that we are going to
> >> reserve,
> >> when we try to get new position new lmb.reserved.region.
> >
> > I'm not too sure I follow you. For the resizing, I would just basically
> > call a low level variant of alloc (__lmb_alloc ?) that explicitely
> > doesn't honor the total-2 "reserved" entries in the array.
>
> 1. you want to reserve rangeA
> 2. before that will check if region array is big enough,
> 3. if region is not big enough, will call lmb_alloc to get new range.
> lmb_alloc could return rangB that is overlapped with rangeA

So instead you do it the other way.

1. you want to reserve rangeA
2. you reserve rangeA
3. if reserving rangeA consumed a slot in the array then you check if
you have at least two free slots. If not you realloc. You don't need any
special tricks because you have space to lmb_alloc() a new area and move
everything over.

cheers
From: Yinghai Lu on
On 03/29/2010 11:46 PM, Michael Ellerman wrote:
> On Mon, 2010-03-29 at 23:12 -0700, Yinghai Lu wrote:
>> On 03/29/2010 10:26 PM, Benjamin Herrenschmidt wrote:
>>> On Mon, 2010-03-29 at 17:03 -0700, Yinghai Lu wrote:
>>>>
>>>> in short: It could make us to avoid use the range that we are going to
>>>> reserve,
>>>> when we try to get new position new lmb.reserved.region.
>>>
>>> I'm not too sure I follow you. For the resizing, I would just basically
>>> call a low level variant of alloc (__lmb_alloc ?) that explicitely
>>> doesn't honor the total-2 "reserved" entries in the array.
>>
>> 1. you want to reserve rangeA
>> 2. before that will check if region array is big enough,
>> 3. if region is not big enough, will call lmb_alloc to get new range.
>> lmb_alloc could return rangB that is overlapped with rangeA
>
> So instead you do it the other way.
>
> 1. you want to reserve rangeA
> 2. you reserve rangeA
> 3. if reserving rangeA consumed a slot in the array then you check if
> you have at least two free slots. If not you realloc. You don't need any
> special tricks because you have space to lmb_alloc() a new area and move
> everything over.

so that is check it later. should work.

one less find_lmb_area user.

Thanks

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Benjamin Herrenschmidt on
On Mon, 2010-03-29 at 23:12 -0700, Yinghai Lu wrote:
>
> 1. you want to reserve rangeA
> 2. before that will check if region array is big enough,
> 3. if region is not big enough, will call lmb_alloc to get new range.
> lmb_alloc could return rangB that is overlapped with rangeA
> 4. current lmb_alloc only honor limit, and doesn't honor low limit.

I see. This is a direct consequence of you wanting to use find/reserve
instead of alloc tho :-)

This is also easily fixed. Instead of doing the resize "on demand" like
I originally proposed, do it at the end of reserve/alloc. If the number
of free entries is down to 1 or 0, then alloc a new chunk.

Of course, all of that requires that reservations done from FW to take
memory out because it must not be accessed need to be all done before
the first grow of the array, and so the static array must be sized
accordingly. We may want to catch these things too. We don't want to
warn on multiple overlapping lmb_reserve() tho on powerpc...

> another usage is: for temporary buffer, like range array for
> subtraction. we don't need to do free later.

Sorry, doesn't parse.

> > Ie. It should all be one single find/allocation function.
> >
> > In fact, you want to split lmb_find from lmb_reserve, then just make
> > lmb_alloc use the above, I don't want 2 implementations of the same
> > thing (maybe call it __lmb_find to expose the fact that it's a low
> level
> > function to avoid for normal use).
>
> that is some difference between them, and lmb_alloc doesn't honor low
> limit.

If you want a low and a high limit, then add the low limit to
lmb_alloc_base(), it's easy to fix all callers, there aren't many, and
make not one but two defaults for lmb_alloc(), one for the low limit and
one for the high limit. Problem solved.

> we can provide
> lmb_find_area
> lmb_reserve_area
> lmb_free_area
>
> and use lmb_find_area + lmb_reserve_area to get one lmb_alloc()

I still don't understand why you insist on using find + reserve instead
of alloc in x86 land. Can you give me a proper explanation as to why
that is needed since it seems to be causing problems, and so far and I
don't see what it solves.

> x86 sometime is using find_lmb_area to find big area, and use those
> area and later reserve actually used area.

That's very wrong. If you use something, alloc/reserve it. You can
always free it later.

> you could use lmb_alloc, and later lmb_free not used. that is equal to
> lmb_find + lmb_reserve + lmb_free ...

Sure, then just do alloc + later free.

Cheers,
Ben.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Yinghai Lu on
On 03/30/2010 02:30 PM, Benjamin Herrenschmidt wrote:
> On Mon, 2010-03-29 at 23:12 -0700, Yinghai Lu wrote:
>>
>> 1. you want to reserve rangeA
>> 2. before that will check if region array is big enough,
>> 3. if region is not big enough, will call lmb_alloc to get new range.
>> lmb_alloc could return rangB that is overlapped with rangeA
>> 4. current lmb_alloc only honor limit, and doesn't honor low limit.
>
> I see. This is a direct consequence of you wanting to use find/reserve
> instead of alloc tho :-)
>
> This is also easily fixed. Instead of doing the resize "on demand" like
> I originally proposed, do it at the end of reserve/alloc. If the number
> of free entries is down to 1 or 0, then alloc a new chunk.

already done. last night, Michael point that out.

>
> Of course, all of that requires that reservations done from FW to take
> memory out because it must not be accessed need to be all done before
> the first grow of the array, and so the static array must be sized
> accordingly. We may want to catch these things too. We don't want to
> warn on multiple overlapping lmb_reserve() tho on powerpc...
>
>> another usage is: for temporary buffer, like range array for
>> subtraction. we don't need to do free later.
>
> Sorry, doesn't parse.
never mind.
>
>>> Ie. It should all be one single find/allocation function.
>>>
>>> In fact, you want to split lmb_find from lmb_reserve, then just make
>>> lmb_alloc use the above, I don't want 2 implementations of the same
>>> thing (maybe call it __lmb_find to expose the fact that it's a low
>> level
>>> function to avoid for normal use).
>>
>> that is some difference between them, and lmb_alloc doesn't honor low
>> limit.
>
> If you want a low and a high limit, then add the low limit to
> lmb_alloc_base(), it's easy to fix all callers, there aren't many, and
> make not one but two defaults for lmb_alloc(), one for the low limit and
> one for the high limit. Problem solved.

make 32 bit x86 can work from high to low now.

>
>> we can provide
>> lmb_find_area
>> lmb_reserve_area
>> lmb_free_area
>>
>> and use lmb_find_area + lmb_reserve_area to get one lmb_alloc()
>
> I still don't understand why you insist on using find + reserve instead
> of alloc in x86 land. Can you give me a proper explanation as to why
> that is needed since it seems to be causing problems, and so far and I
> don't see what it solves.
>
>> x86 sometime is using find_lmb_area to find big area, and use those
>> area and later reserve actually used area.
>
> That's very wrong. If you use something, alloc/reserve it. You can
> always free it later.
>
>> you could use lmb_alloc, and later lmb_free not used. that is equal to
>> lmb_find + lmb_reserve + lmb_free ...
>
> Sure, then just do alloc + later free.

ok, I will replace that later after it get stable.
but at first will expose the find_lmb_area.

will send out -v11, please check that version.

Thanks

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/