Clement Fabien wrote: > 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 >
I'm on it. Regards Pantelis
