Re: [patch 02/41] cpu alloc: The allocator

Mike Travis
2008-06-04T14:48:48Z

Andrew Morton wrote:
...
>> +/*
>> + * Mark an object as used in the cpu_alloc_map
>> + *
>> + * Must hold cpu_alloc_map_lock
>> + */
>> +static void set_map(int start, int length)
>> +{
>> + while (length-- > 0)
>> + __set_bit(start++, cpu_alloc_map);
>> +}
>
> bitmap_fill()?
>
>> +/*
>> + * Mark an area as freed.
>> + *
>> + * Must hold cpu_alloc_map_lock
>> + */
>> +static void clear_map(int start, int length)
>> +{
>> + while (length-- > 0)
>> + __clear_bit(start++, cpu_alloc_map);
>> +}
>
> bitmap_zero()?
...
>> +void *cpu_alloc(unsigned long size, gfp_t gfpflags, unsigned long align)
>> +{
>> + unsigned long start;
>> + int units = size_to_units(size);
>> + void *ptr;
>> + int first;
>> + unsigned long flags;
>> +
>> + if (!size)
>> + return ZERO_SIZE_PTR;
>
> OK, so we reuse ZERO_SIZE_PTR from kmalloc.
>
>> + spin_lock_irqsave(&cpu_alloc_map_lock, flags);
>> +
>> + first = 1;
>> + start = first_free;
>> +
>> + for ( ; ; ) {
>> +
>> + start = find_next_zero_bit(cpu_alloc_map, UNITS, start);
>> + if (start >= UNITS)
>> + goto out_of_memory;
>> +
>> + if (first)
>> + first_free = start;
>> +
>> + /*
>> + * Check alignment and that there is enough space after
>> + * the starting unit.
>> + */
>> + if (start % (align / UNIT_SIZE) == 0 &&
>> + find_next_bit(cpu_alloc_map, UNITS, start + 1)
>> + >= start + units)
>> + break;
>> + start++;
>> + first = 0;
>> + }
>
> This is kinda bitmap_find_free_region(), only bitmap_find_free_region()
> isn't quite strong enough.
>
> Generally I think it would have been better if you had added new
> primitives to the bitmap library (or enhanced existing ones) and used
> them here, rather than implementing private functionality.

Hi Andrew,

I've sort of inherited this now...

So are you suggesting that we add new bitmap primitives to:

bitmap_fill_offset(bitmap, start, nbits) /* start at bitmap[start] */
bitmap_zero_offset(bitmap, start, nbits)
bitmap_find_free_area(bitmap, nbits, size, align) /* size not order */

Thanks,
Mike