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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Sun, 9 Jan 2011 11:13:44 +0800
From:	Hillf Danton <dhillf@...il.com>
To:	Tejun Heo <tj@...nel.org>
Cc:	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] replace ida in workqueue with page-sized buffer

On Sat, Jan 8, 2011 at 10:01 PM, Tejun Heo <tj@...nel.org> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/