Messages in this thread | | | Subject | Re: Uninline kcalloc | From | Xi Wang <> | Date | Thu, 16 Feb 2012 13:32:45 -0500 |
| |
On Feb 16, 2012, at 9:51 AM, Christoph Lameter wrote: > Then there is > > vzalloc > kzalloc > vmalloc_32 > alloc_bootmem (MAXORDER limit may not work) > alloc_remap > > ... > > This would also work for special subsystem allocations like > > usb_alloc_coherent > dm_vcalloc > devres_alloc > > .... > > > The use of a function or macro makes the overflow check much more > universal and allows these array allocations to occur with different > allocation functions throughout the kernel.
No, it does NOT. It can be easily misued to introduce more bugs.
1) Should calculate_array_size() return 0 on overflow, as you orginally proposed?
No, as Dan, Pekka, and some others already pointed out.
2) Should calculate_array_size() return something like KMALLOC_MAX_SIZE + 1?
No, because you intended to use it with other allocators such as vmalloc().
3) Should calculate_array_size() return ULONG_MAX/SIZE_MAX/-1?
No! Consider devres_alloc() you mentioned. Then you do
devres_alloc(..., calculate_array_size(n, size), ...).
It internally invokes kmalloc() with allocation size:
sizeof(struct devres) + calculate_array_size(n, size).
When n * size overflows, calculate_array_size() returns ULONG_MAX, and the allocation size wraps around to a small integer!
I like the idea of "do not add an allocator unless necessary". However, "universal" calculate_array_size() just doesn't work, unless you can find the correct semantics or limit its use. It can be easily misused and bring more trouble than it's worth.
- xi
| |