[lkml]   [2009]   [Feb]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH] workqueue: don't alloc_percpu for single workqueue
Hi, Oleg Nesterov

The new version of this patch has not been accepted.
I think it's due to that I forgot reply this mail.

(comment nesting)

Oleg Nesterov wrote:
> On 01/22, Lai Jiangshan wrote:
>> Oleg Nesterov wrote:
>>> On 01/21, Lai Jiangshan wrote:
>>>> @@ -906,6 +907,13 @@ void destroy_workqueue(struct workqueue_struct *wq)
>>>> const struct cpumask *cpu_map = wq_cpu_map(wq);
>>>> int cpu;
>>>> + if (is_wq_single_threaded(wq)) {
>>>> + cleanup_workqueue_thread(wq->cpu_wq);
>>>> + kfree(wq->cpu_wq);
>>>> + kfree(wq);
>>>> + return;
>>>> + }
>>> again, not sure I understand why this change is needed. Afaics we
>>> only need to use kfree(wq->cpu_wq) instead of free_percpu() if
>>> it is single-threaded.
>> I think this change is needed.
>> In the single thread case, we don't need
>> 1) cpu_maps_update_begin(). --> require cpu_add_remove_lock
>> 2) remove workqueue from the list. (we did not inserted it)
>> It is indeed that there is no bad result occurred when we do these
>> things for single thread. But I think the destroying should not
>> do things more than the creating.
> I disagree.
> Firstly, this path is rare and not time critical, it is better
> to save a couple of bytes from .text.

For non-critical path, I think the prior sequence aim is:

code logic is right
code's readability
save .text size
save cpu cycle

I want to make "code logic is right", so
my patch adds another special case for single thread workqueue.

And in creating site, single thread workqueue are also considered


> But mostly I dislike the fact that we add another special case
> for the single-threaded wqs which is not strictly needed.
> Following your logic we can also change flush_workqueue(), it
> doesn't need for_each_cpu_mask_nr() when single-threaded.
> That said, I agree this is a matter of taste, I won't persist.
> Oleg.

 \ /
  Last update: 2009-02-18 12:23    [from the cache]
©2003-2014 Jasper Spaans. hosted at Digital Ocean