[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    [W:0.022 / U:5.980 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site