[lkml]   [2007]   [May]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH] make-cancel_rearming_delayed_work-reliable-fix
    On 05/07, Jarek Poplawski wrote:
    > There is a lot of new things in the final version of this
    > patch. I guess, there was no such problem in the previous
    > version.

    No, this is basically the same patch + re-check-cwq-after-lock,
    the latter is mostly needed to prevent racing with CPU-hotplug.

    > I can also see you have new doubts about usefulness, which
    > I cannot understand:
    > - even if there are some slowdowns, where does it matter?
    > - the "old" method uses only one method of cancelling, i.e.
    > del_timer, not trying to stop requeuing or to remove from
    > the queue; it seems to be effective only with long delayed
    > timers, and its real problems are probably mostly invisible.

    The slowdown is small, changelog mentions it just to be "fair".

    I am not happy with the complication this patch adds, mostly
    I hate this smb_wmb() in insert_work(). I have an idea how to
    remove it later, but this needs another patch not related to

    > BTW, I'm still not convinced all additions are needed:
    > the "old" cancel_rearming_ doesn't care about checking
    > or waiting on anything after del_timer positive.

    It would be very strange to do wait_on_work() only in case
    when del_timer() failed. This way we still need to do
    cancel_work_sync() after cancel_rearming_delayed_work(),
    but only when del_timer() failed, ugly. Note also that
    wait_on_work() does not sleep if work->func() is not running.

    Also, consider this callback:

    void work_handler(struct work_struct *w)
    struct delayed_work dw = container_of(...);

    queue_delayed_work(dw, delay);

    // <------------- cancel_rearming_delayed_work()

    queue_delayed_work(dw, another_delay);

    Yes, this is strange and ugly. But correct! The current version
    (before this patch) can't cancel this delayed_work. The new
    implementation works correctly. So I think it is far better to
    do wait_on_work() unconditionally.

    > PS: I'll try to check this all in the evening and will
    > write tomorrow, if found something interesting.

    Yes, please!


    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2007-05-07 13:31    [W:0.020 / U:63.172 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site