Re: [PATCH 17/43] workqueue: update cwq alignement

[Tejun Heo]

Hello,

On 03/01/2010 02:12 AM, Oleg Nesterov wrote:
> On 02/26, Tejun Heo wrote:
>>
>> +static struct cpu_workqueue_struct *alloc_cwqs(void)
>> +{
>> +	const size_t size = sizeof(struct cpu_workqueue_struct);
>> +	const size_t align = 1 << WORK_STRUCT_FLAG_BITS;
>> +	struct cpu_workqueue_struct *cwqs;
>> +#ifndef CONFIG_SMP
>> +	void *ptr;
>> +
>> +	/*
>> +	 * On UP, percpu allocator doesn't honor alignment parameter
>> +	 * and simply uses arch-dependent default.  Allocate enough
>> +	 * room to align cwq and put an extra pointer at the end
>> +	 * pointing back to the originally allocated pointer which
>> +	 * will be used for free.
>> +	 */
>> +	ptr = __alloc_percpu(size + align + sizeof(void *), 1);
>> +	cwqs = PTR_ALIGN(ptr, align);
>> +	*(void **)per_cpu_ptr(cwqs + 1, 0) = ptr;
>> +#else
> 
> Nice trick, but perhaps it would be more simple/clear to just add
> "void *my_memory" into cpu_workqueue_struct, under !CONFIG_SMP ?

I agree that it's ugly but wanted to contain it inside
alloc/free_cwqs() as this is something which should be handled by the
allocator not the workqueue code.  The right thing to do would be
adding alignment code to UP percpu allocator.  I'll leave the ugly
code as it is for now but add big fat FIXME: there and will update
percpu allocator code later and then remove this ugliness.

Thank you.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/