[Xenomai-core] [PATCH] fix xnheap_alloc rounding

[Xenomai-core] [PATCH] fix xnheap_alloc rounding

[Jan Kiszka]

[-- Attachment #1.1: Type: text/plain, Size: 1024 bytes --]

Hi,

playing a stupid rt_heap user (actually I didn't just play this...), I
stumbled over this undocumented oddity:

rt_heap_create(&heap, name, 10000, H_PRIO|H_MAPPABLE);
rt_heap_alloc(&heap, 10000, TM_NONBLOCK, &ptr);

Creation is successful, allocation fails. The reason: while during
creation the net heap size is rounded down to page boundaries, the
allocation of memory > PAGE_SIZE is rounded up. One could add H_SINGLE
to the flags, but this may even result in allocating less memory than
the user expected, causing severe problems later.

How to resolve this best? I thought about rounding twice in
rt_head_create (one time the net size, the second time including the
overhead), but this encodes characteristics of the underlying heap
allocator into the skin (I have a generic heap allocator framework in
mind for 2.3). So I decided to do this rounding in xnheap_overhead()
instead, see attached patch. Hope I didn't skewed up any calculation. At
least the scenario above now works fine.

Jan

[-- Attachment #1.2: xnheap_overhead-fix-rounding.patch --]
[-- Type: text/plain, Size: 927 bytes --]

Index: include/nucleus/heap.h
===================================================================
--- include/nucleus/heap.h	(revision 1322)
+++ include/nucleus/heap.h	(working copy)
@@ -109,8 +109,14 @@ extern xnheap_t kheap;
 #define xnheap_used_mem(heap)        ((heap)->ubytes)
 #define xnheap_max_contiguous(heap)  ((heap)->maxcont)
 #define xnheap_overhead(hsize,psize) \
-((sizeof(xnextent_t) + (((hsize) - sizeof(xnextent_t)) / (psize)) + \
- XNHEAP_MINALIGNSZ - 1) & ~(XNHEAP_MINALIGNSZ - 1))
+({ \
+    u_long rounded_hsize = (hsize + psize - 1) & ~(psize - 1); \
+    u_long overhead = ((sizeof(xnextent_t) + \
+	(((rounded_hsize) - sizeof(xnextent_t)) / (psize)) + \
+	XNHEAP_MINALIGNSZ - 1) & ~(XNHEAP_MINALIGNSZ - 1)); \
+    overhead += rounded_hsize - hsize; \
+    overhead; \
+})
 
 #define xnmalloc(size)     xnheap_alloc(&kheap,size)
 #define xnfree(ptr)        xnheap_free(&kheap,ptr)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

Re: [Xenomai-core] [PATCH] fix xnheap_alloc rounding

[Gilles Chanteperdrix]

Jan Kiszka wrote:
 > Hi,
 > 
 > playing a stupid rt_heap user (actually I didn't just play this...), I
 > stumbled over this undocumented oddity:
 > 
 > rt_heap_create(&heap, name, 10000, H_PRIO|H_MAPPABLE);
 > rt_heap_alloc(&heap, 10000, TM_NONBLOCK, &ptr);
 > 
 > Creation is successful, allocation fails. The reason: while during
 > creation the net heap size is rounded down to page boundaries, the
 > allocation of memory > PAGE_SIZE is rounded up. One could add H_SINGLE
 > to the flags, but this may even result in allocating less memory than
 > the user expected, causing severe problems later.
 > 
 > How to resolve this best? I thought about rounding twice in
 > rt_head_create (one time the net size, the second time including the
 > overhead), but this encodes characteristics of the underlying heap
 > allocator into the skin (I have a generic heap allocator framework in
 > mind for 2.3). So I decided to do this rounding in xnheap_overhead()
 > instead, see attached patch. Hope I didn't skewed up any calculation. At
 > least the scenario above now works fine.

The problem I see with this patch is that you are counting 
(rounded_hsize - hsize) as part of the overhead, whereas you would like to
count it as free space. Would not it make more sense to do the rounding
in xnheap_init_mapped ?

-- 


					    Gilles Chanteperdrix.

Re: [Xenomai-core] [PATCH] fix xnheap_alloc rounding

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1550 bytes --]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>  > Hi,
>  > 
>  > playing a stupid rt_heap user (actually I didn't just play this...), I
>  > stumbled over this undocumented oddity:
>  > 
>  > rt_heap_create(&heap, name, 10000, H_PRIO|H_MAPPABLE);
>  > rt_heap_alloc(&heap, 10000, TM_NONBLOCK, &ptr);
>  > 
>  > Creation is successful, allocation fails. The reason: while during
>  > creation the net heap size is rounded down to page boundaries, the
>  > allocation of memory > PAGE_SIZE is rounded up. One could add H_SINGLE
>  > to the flags, but this may even result in allocating less memory than
>  > the user expected, causing severe problems later.
>  > 
>  > How to resolve this best? I thought about rounding twice in
>  > rt_head_create (one time the net size, the second time including the
>  > overhead), but this encodes characteristics of the underlying heap
>  > allocator into the skin (I have a generic heap allocator framework in
>  > mind for 2.3). So I decided to do this rounding in xnheap_overhead()
>  > instead, see attached patch. Hope I didn't skewed up any calculation. At
>  > least the scenario above now works fine.
> 
> The problem I see with this patch is that you are counting 
> (rounded_hsize - hsize) as part of the overhead, whereas you would like to
> count it as free space. Would not it make more sense to do the rounding
> in xnheap_init_mapped ?
> 

I was thinking of the open-coded

allocate(heap_size + xnheap_overhead)
xnheap_init()

in many skins as well.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

Re: [Xenomai-core] [PATCH] fix xnheap_alloc rounding

[Philippe Gerum]

On Tue, 2006-07-11 at 16:02 +0200, Jan Kiszka wrote:
> Hi,
> 
> playing a stupid rt_heap user (actually I didn't just play this...), I
> stumbled over this undocumented oddity:
> 
> rt_heap_create(&heap, name, 10000, H_PRIO|H_MAPPABLE);
> rt_heap_alloc(&heap, 10000, TM_NONBLOCK, &ptr);
> 
> Creation is successful, allocation fails. The reason: while during
> creation the net heap size is rounded down to page boundaries, the
> allocation of memory > PAGE_SIZE is rounded up. One could add H_SINGLE
> to the flags, but this may even result in allocating less memory than
> the user expected, causing severe problems later.
> 
> How to resolve this best? I thought about rounding twice in
> rt_head_create (one time the net size, the second time including the
> overhead), but this encodes characteristics of the underlying heap
> allocator into the skin (I have a generic heap allocator framework in
> mind for 2.3).

The nucleus heap expects the caller to pass it the memory which is going
to be used to fulfill allocation requests, so there is already a builtin
dependency from any client heap manager on the nucleus implementation.
Double rounding is therefore the way to go, since callers need to be
fixed so that they provide enough memory to the xnheap manager when
specializing its behaviour.

>  So I decided to do this rounding in xnheap_overhead()
> instead, see attached patch. Hope I didn't skewed up any calculation. At
> least the scenario above now works fine.
> 
> Jan
> plain text document attachment (xnheap_overhead-fix-rounding.patch)
> Index: include/nucleus/heap.h
> ===================================================================
> --- include/nucleus/heap.h	(revision 1322)
> +++ include/nucleus/heap.h	(working copy)
> @@ -109,8 +109,14 @@ extern xnheap_t kheap;
>  #define xnheap_used_mem(heap)        ((heap)->ubytes)
>  #define xnheap_max_contiguous(heap)  ((heap)->maxcont)
>  #define xnheap_overhead(hsize,psize) \
> -((sizeof(xnextent_t) + (((hsize) - sizeof(xnextent_t)) / (psize)) + \
> - XNHEAP_MINALIGNSZ - 1) & ~(XNHEAP_MINALIGNSZ - 1))
> +({ \
> +    u_long rounded_hsize = (hsize + psize - 1) & ~(psize - 1); \
> +    u_long overhead = ((sizeof(xnextent_t) + \
> +	(((rounded_hsize) - sizeof(xnextent_t)) / (psize)) + \
> +	XNHEAP_MINALIGNSZ - 1) & ~(XNHEAP_MINALIGNSZ - 1)); \
> +    overhead += rounded_hsize - hsize; \
> +    overhead; \
> +})
>  
>  #define xnmalloc(size)     xnheap_alloc(&kheap,size)
>  #define xnfree(ptr)        xnheap_free(&kheap,ptr)
> _______________________________________________
> Xenomai-core mailing list
> Xenomai-core@domain.hid
> https://mail.gna.org/listinfo/xenomai-core
-- 
Philippe.

Re: [Xenomai-core] [PATCH] fix xnheap_alloc rounding

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 3249 bytes --]

Philippe Gerum wrote:
> On Tue, 2006-07-11 at 16:02 +0200, Jan Kiszka wrote:
>> Hi,
>>
>> playing a stupid rt_heap user (actually I didn't just play this...), I
>> stumbled over this undocumented oddity:
>>
>> rt_heap_create(&heap, name, 10000, H_PRIO|H_MAPPABLE);
>> rt_heap_alloc(&heap, 10000, TM_NONBLOCK, &ptr);
>>
>> Creation is successful, allocation fails. The reason: while during
>> creation the net heap size is rounded down to page boundaries, the
>> allocation of memory > PAGE_SIZE is rounded up. One could add H_SINGLE
>> to the flags, but this may even result in allocating less memory than
>> the user expected, causing severe problems later.
>>
>> How to resolve this best? I thought about rounding twice in
>> rt_head_create (one time the net size, the second time including the
>> overhead), but this encodes characteristics of the underlying heap
>> allocator into the skin (I have a generic heap allocator framework in
>> mind for 2.3).
> 
> The nucleus heap expects the caller to pass it the memory which is going
> to be used to fulfill allocation requests, so there is already a builtin
> dependency from any client heap manager on the nucleus implementation.
> Double rounding is therefore the way to go, since callers need to be
> fixed so that they provide enough memory to the xnheap manager when
> specializing its behaviour.

If this means to do the rounding in the clients without the help of some
well-defined xnheap-API, then I disagree. I have an alternative nucleus
heap manager in mind for the future that will have a different rounding
strategy for allocations. So we should cleanly hide such property behind
a xnheap service - either in the overhead calculation or in some
additional rounding service. But I do not see a need for splitting this
up yet.

> 
>>  So I decided to do this rounding in xnheap_overhead()
>> instead, see attached patch. Hope I didn't skewed up any calculation. At
>> least the scenario above now works fine.
>>
>> Jan
>> plain text document attachment (xnheap_overhead-fix-rounding.patch)
>> Index: include/nucleus/heap.h
>> ===================================================================
>> --- include/nucleus/heap.h	(revision 1322)
>> +++ include/nucleus/heap.h	(working copy)
>> @@ -109,8 +109,14 @@ extern xnheap_t kheap;
>>  #define xnheap_used_mem(heap)        ((heap)->ubytes)
>>  #define xnheap_max_contiguous(heap)  ((heap)->maxcont)
>>  #define xnheap_overhead(hsize,psize) \
>> -((sizeof(xnextent_t) + (((hsize) - sizeof(xnextent_t)) / (psize)) + \
>> - XNHEAP_MINALIGNSZ - 1) & ~(XNHEAP_MINALIGNSZ - 1))
>> +({ \
>> +    u_long rounded_hsize = (hsize + psize - 1) & ~(psize - 1); \
>> +    u_long overhead = ((sizeof(xnextent_t) + \
>> +	(((rounded_hsize) - sizeof(xnextent_t)) / (psize)) + \
>> +	XNHEAP_MINALIGNSZ - 1) & ~(XNHEAP_MINALIGNSZ - 1)); \
>> +    overhead += rounded_hsize - hsize; \
>> +    overhead; \
>> +})
>>  
>>  #define xnmalloc(size)     xnheap_alloc(&kheap,size)
>>  #define xnfree(ptr)        xnheap_free(&kheap,ptr)
>> _______________________________________________
>> Xenomai-core mailing list
>> Xenomai-core@domain.hid
>> https://mail.gna.org/listinfo/xenomai-core



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

Re: [Xenomai-core] [PATCH] fix xnheap_alloc rounding

[Philippe Gerum]

On Sat, 2006-07-15 at 10:07 +0200, Jan Kiszka wrote:
> Philippe Gerum wrote:
> > On Tue, 2006-07-11 at 16:02 +0200, Jan Kiszka wrote:
> >> Hi,
> >>
> >> playing a stupid rt_heap user (actually I didn't just play this...), I
> >> stumbled over this undocumented oddity:
> >>
> >> rt_heap_create(&heap, name, 10000, H_PRIO|H_MAPPABLE);
> >> rt_heap_alloc(&heap, 10000, TM_NONBLOCK, &ptr);
> >>
> >> Creation is successful, allocation fails. The reason: while during
> >> creation the net heap size is rounded down to page boundaries, the
> >> allocation of memory > PAGE_SIZE is rounded up. One could add H_SINGLE
> >> to the flags, but this may even result in allocating less memory than
> >> the user expected, causing severe problems later.
> >>
> >> How to resolve this best? I thought about rounding twice in
> >> rt_head_create (one time the net size, the second time including the
> >> overhead), but this encodes characteristics of the underlying heap
> >> allocator into the skin (I have a generic heap allocator framework in
> >> mind for 2.3).
> > 
> > The nucleus heap expects the caller to pass it the memory which is going
> > to be used to fulfill allocation requests, so there is already a builtin
> > dependency from any client heap manager on the nucleus implementation.
> > Double rounding is therefore the way to go, since callers need to be
> > fixed so that they provide enough memory to the xnheap manager when
> > specializing its behaviour.
> 
> If this means to do the rounding in the clients without the help of some
> well-defined xnheap-API, then I disagree. I have an alternative nucleus
> heap manager in mind for the future that will have a different rounding
> strategy for allocations. So we should cleanly hide such property behind
> a xnheap service - either in the overhead calculation or in some
> additional rounding service. But I do not see a need for splitting this
> up yet.

xnheap_overhead() needs to remain that way, i.e. compute the exact
number of additional bytes used for housekeeping purposes by the heap
manager. Fixing the clients by providing them the required support to
perform proper rounding without resorting to open-coded calculations is
another issue, and is indeed the way to go. I'll handle that.

> 
> > 
> >>  So I decided to do this rounding in xnheap_overhead()
> >> instead, see attached patch. Hope I didn't skewed up any calculation. At
> >> least the scenario above now works fine.
> >>
> >> Jan
> >> plain text document attachment (xnheap_overhead-fix-rounding.patch)
> >> Index: include/nucleus/heap.h
> >> ===================================================================
> >> --- include/nucleus/heap.h	(revision 1322)
> >> +++ include/nucleus/heap.h	(working copy)
> >> @@ -109,8 +109,14 @@ extern xnheap_t kheap;
> >>  #define xnheap_used_mem(heap)        ((heap)->ubytes)
> >>  #define xnheap_max_contiguous(heap)  ((heap)->maxcont)
> >>  #define xnheap_overhead(hsize,psize) \
> >> -((sizeof(xnextent_t) + (((hsize) - sizeof(xnextent_t)) / (psize)) + \
> >> - XNHEAP_MINALIGNSZ - 1) & ~(XNHEAP_MINALIGNSZ - 1))
> >> +({ \
> >> +    u_long rounded_hsize = (hsize + psize - 1) & ~(psize - 1); \
> >> +    u_long overhead = ((sizeof(xnextent_t) + \
> >> +	(((rounded_hsize) - sizeof(xnextent_t)) / (psize)) + \
> >> +	XNHEAP_MINALIGNSZ - 1) & ~(XNHEAP_MINALIGNSZ - 1)); \
> >> +    overhead += rounded_hsize - hsize; \
> >> +    overhead; \
> >> +})
> >>  
> >>  #define xnmalloc(size)     xnheap_alloc(&kheap,size)
> >>  #define xnfree(ptr)        xnheap_free(&kheap,ptr)
> >> _______________________________________________
> >> Xenomai-core mailing list
> >> Xenomai-core@domain.hid
> >> https://mail.gna.org/listinfo/xenomai-core
> 
> 
-- 
Philippe.