Re: [PATCH] replace ida in workqueue with page-sized buffer

[Hillf Danton]

On Sat, Jan 8, 2011 at 10:01 PM, Tejun Heo <tj@xxxxxxxxxx> wrote:
> Hello,
>
> On Sat, Dec 18, 2010 at 09:10:18PM +0800, Hillf Danton wrote:
>> The id of worker of global work queue is monitored with kernel library, ida.
>>
>> The ida in global work queue is redefined to be page-sized buffer,
>> which is allocated when initializing workqueue.
>>
>> The new id allocator could monitor ids in the rage from 0 to INT_MAX.
>> The buffer is used to store freed ids, and if the peak number of ids,
>> in workqueue as whole, is not out of the capacity of the allocated
>> buffer, there is no more allocation of buffer after initializing.
>>
>> The advantage looks that the new allocator runs faster.
>
> I'm confused about why this is necessary. ÂIt's not like workqueue ID
> alloc/dealloc is hot path to begin with (they're quite lazy) and
> during worker creation / destruction ida is a quite insignificant

Great point. Since quite lazy, the patch is quite inapplicable.

> part, so what is it trying to optimize? ÂIt's not like ida allocator
> is slow to begin with.
>
> Also, the allocator looks quite dubious.
>
> * What happens if more entries than what can fit in a page are freed?
> ÂIt'll just lose track of it.
>

Losing track of allocated ids could happen only if allocation is not
quite lazy, and it could be fixed by extending the page buffer, though
not included in the delivered patch.

> * ida guarantees that always the lowest available slot is allocated.
> ÂThe new allocator doesn't.
>

Since the allocated ids are then hashed, the lowest available looks
unnecessary in workqueue, which simplifies allocation.

Thanks a lot for reviewing the patch.
Hillf
--
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/