[rtems commit] rbheap: New files

[Gedare Bloom]

On Sun, Apr 15, 2012 at 6:56 AM, Sebastian Huber
<sebastian.huber at embedded-brains.de> wrote:
> On 14/04/12 18:20, Gedare Bloom wrote:
>>
>> I have a couple concerns.
>>
>> First is the naming "rbheap";. I see it is implementing a memory
>> manager using the rbtree and a couple chain controls. It's not really
>> a heap so much as a custom free page allocator.
>
>
> From my point of view everything that implements the malloc() and free()
> interface is a heap.  Why do you think this is not a heap?
>
Sorry I was confusing terms between a heap data structure versus a
heap space of memory. rbheap should be fine on further thought. It
might be nice to formalize/standardize this interface. Perhaps even
implement this rbheap as an alternative Heap Control.

>
>> Second is the code placement. I'm not convinced this is best to just
>> shim into the sapi layer.
>
>
> Where else would you place this?
>
I'm not sure. sapi probably is the most appropriate place that we have
currently. I am not happy with the sapi directory generally as it is
quite vague how we
should be using it / what should be implemented in it.

I'm seeing a pattern between the Region Manager, Partition Manager,
and your rbheap manager. They all offer quite similar functionality
except for how they implement the memory pool. I wonder if we can
unify them all in one Supercore Manager that uses a pluggable memory
pool implementation. Region and Partition would still need to have a
thin wrapper in the classic api, and the rbheap can be in the sapi
since it is not part of any standardized features.

In my view the classic API is not consistent with a standard (orkid
seems like it was doa unless I'm missing something) and we should
consider it to be the RTEMS api and merge the sapi into it and move
forward with implementing any RTEMS-specific non-standard APIs at that
layer. This point should be discussed separately / elsewhere however.

>
>> The functionality is reminiscent of the
>> Region Manager. What functionality is missing that this code is unable
>> to use existing classic or score services to achieve? I have a feeling
>> we could generalize the approach taken here by implementing an Arena
>> Manager somewhere.
>
>
> The region manager stores administration data in the managed memory area.
>  The rbheap separates this.  You can use it to implement a task stack
> allocator where each task has only access to is own task stack.
>
>
Let me see if I understand. With the rbheap you can use one rbheap to
allocate n task stacks, and the administrative data for the stack
allocation will reside in the rbheap_page structure. So you can have
precise definitions of which memory the task needs to access and a
blown task stack will not directly access the administrative data.

With the region manager you cannot use one region to allocate n task
stacks because the administrative data (when using a Heap Control)
resides within the heap so a blown task stack can overwrite the heap
metadata to cause corruption elsewhere. You would need n regions to
deal with n separate task stacks--quite a waste.

The utility in the implementation of a memory pool using the rbheap
approach is that the administrative data resides externally to the
memory pool, which differs from a traditional heap implementation.

>> Third is the use of the word "pages". Unless we are referring to
>> fixed-size ranges of virtual memory I think we should avoid this word
>> since it has a fairly common technical meaning and connotation as an
>> OS concept.
>
>
> The region manager and the first-fit heap code uses also this notation.
>  What do you think about "chunk" instead of "page"?
>
I like chunk better than page. The region manager actually uses page,
segment, and block. In the region manager the page size refers only to
the initial amount of memory that is allocated for the region manager
to use. The region manager refers to allocated chunks as segments
which might be ok though it suffers from a similar double-meaning as
page. The partition manager uses buffer and blocks, which are
overloaded terms in RTEMS since we use those in the fs layer. We
should probably unify all this terminology.

>
>> Fourth is the lack of documentation. This is an addition to the public
>> API and should be reflected somewhere in docu and the release notes.
>
>
> Ok.
>
>
>> Fifth is the lack of review before committing;)
>
>
> Sorry, we can do the review now.
>
> --
> Sebastian Huber, embedded brains GmbH
>
> Address : Obere Lagerstr. 30, D-82178 Puchheim, Germany
> Phone   : +49 89 18 90 80 79-6
> Fax     : +49 89 18 90 80 79-9
> E-Mail  : sebastian.huber at embedded-brains.de
> PGP     : Public key available on request.
>
> Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
>
> _______________________________________________
> rtems-devel mailing list
> rtems-devel at rtems.org
> http://www.rtems.org/mailman/listinfo/rtems-devel