• Tejun Heo's avatar
    workqueue: mod_delayed_work_on() shouldn't queue timer on 0 delay · 8852aac2
    Tejun Heo authored
    8376fe22 ("workqueue: implement mod_delayed_work[_on]()")
    implemented mod_delayed_work[_on]() using the improved
    try_to_grab_pending().  The function is later used, among others, to
    replace [__]candel_delayed_work() + queue_delayed_work() combinations.
    
    Unfortunately, a delayed_work item w/ zero @delay is handled slightly
    differently by mod_delayed_work_on() compared to
    queue_delayed_work_on().  The latter skips timer altogether and
    directly queues it using queue_work_on() while the former schedules
    timer which will expire on the closest tick.  This means, when @delay
    is zero, that [__]cancel_delayed_work() + queue_delayed_work_on()
    makes the target item immediately executable while
    mod_delayed_work_on() may induce delay of upto a full tick.
    
    This somewhat subtle difference breaks some of the converted users.
    e.g. block queue plugging uses delayed_work for deferred processing
    and uses mod_delayed_work_on() when the queue needs to be immediately
    unplugged.  The above problem manifested as noticeably higher number
    of context switches under certain circumstances.
    
    The difference in behavior was caused by missing special case handling
    for 0 delay in mod_delayed_work_on() compared to
    queue_delayed_work_on().  Joonsoo Kim posted a patch to add it -
    ("workqueue: optimize mod_delayed_work_on() when @delay == 0")[1].
    The patch was queued for 3.8 but it was described as optimization and
    I missed that it was a correctness issue.
    
    As both queue_delayed_work_on() and mod_delayed_work_on() use
    __queue_delayed_work() for queueing, it seems that the better approach
    is to move the 0 delay special handling to the function instead of
    duplicating it in mod_delayed_work_on().
    
    Fix the problem by moving 0 delay special case handling from
    queue_delayed_work_on() to __queue_delayed_work().  This replaces
    Joonsoo's patch.
    
    [1] http://thread.gmane.org/gmane.linux.kernel/1379011/focus=1379012Signed-off-by: default avatarTejun Heo <tj@kernel.org>
    Reported-and-tested-by: default avatarAnders Kaseorg <andersk@MIT.EDU>
    Reported-and-tested-by: default avatarZlatko Calusic <zlatko.calusic@iskon.hr>
    LKML-Reference: <alpine.DEB.2.00.1211280953350.26602@dr-wily.mit.edu>
    LKML-Reference: <50A78AA9.5040904@iskon.hr>
    Cc: Joonsoo Kim <js1304@gmail.com>
    8852aac2
workqueue.c 106 KB