[Patch] CPM allocation mechanism in 2.6.13

[Clement Fabien]

Hello

I found out that the rh_alloc mechanism used in 2.6.xx does not always
provide aligned blocks. 
I'm using a 8275 and try to start the SAR driver of Alex Zeffert.

Here is a trace of allocations at linux startup of the code that gives
me problems:

<...>
rh_alloc Required size = 8, align = 16
rh_alloc Block available, blk->size = 00003f40 - Start = 000000c0
rh_alloc Alloc size = 16, align = 16 - Start = 000000c0

rh_alloc Required size = 6144, align = 32 
rh_alloc Block available, blk->size = 00003f30 - Start = 000000d0 
rh_alloc Alloc size = 6144, align = 32 - Start = 000000d0 
(!!!! WRONG ALIGNMENT !!!!)

rh_alloc Required size = 264, align = 64 
rh_alloc Block available, blk->size = 00002730 - Start = 000018d0
rh_alloc Alloc size = 320, align = 64 - Start = 000018d0  
(!!!! WRONG ALIGNMENT !!!!)

rh_alloc Required size = 64, align = 16
rh_alloc Block available, blk->size = 000025f0 - Start = 00001a10
rh_alloc Alloc size = 64, align = 16 - Start = 00001a10
<...>

If we observe rh_alloc (arch/ppc/lib/rheap.c) function

The following line only ensures that there is enough space to perform
alignment:
1.	size = (size + (info->alignment - 1)) & ~(info->alignment - 1);

But with the block start modification mechanism you cannot ensure that
the next block will be aligned : 
2.   	blk->start = (int8_t *)blk->start + size;

The allocation mechanism must take into account the starting address, as
it was in Linux 2.4 following code (3. extracted from commproc.c)
3.     off = ((p->start_addr + align_mask) & (~align_mask)) -
p->start_addr;

I've had an initial discussion with Pantelis who suggests that alignment
shall be unique for this heap: 

>> Excuse me, how do you modify the rheap's alignment at runtime?
>> The allocators prototype is:

>> void *rh_alloc(rh_info_t * info, int size, const char *owner)

>> I don't see an alignment parameter.

>> If you are modifying info->alignment by hand before calling you can't

>> possibly expect this to work.

>> The alignment is fixed after the creation of the rheap.
>> If you need specific alignment you do it like this.

>> u8 *p = rh_alloc(info, size + (align - 1), "owner");

>> p = (void *)((unsigned long)p + (align - 1) & ~(align - 1));

So I guess the problem is in the usage of the function in kernel: it is
initialized with 1 byte alignment (in cpm2_dpinit in
arch/ppc/syslib/cpm2_common.c), then the cpm_dpalloc manipulates the
alignment like this:

4.	cpm_dpmem_info.alignment = align;
	start = rh_alloc(&cpm_dpmem_info, size, "commproc");


I do not have an exact idea whether to modify cpm_dpalloc or rh_alloc.
Can someone provide inputs on this subject?

Please see also the patch I suggest to correct the rh_alloc function

Regards
Fabien Clement

-------------- next part --------------
A non-text attachment was scrubbed...
Name: rheap.patch
Type: application/octet-stream
Size: 1863 bytes
Desc: rheap.patch
Url : http://ozlabs.org/pipermail/linuxppc-embedded/attachments/20051124/77815e22/attachment.obj