Re: Is adding requeue_delayed_work() a good idea

From: Oleg Nesterov
Date: Mon Aug 24 2009 - 14:04:55 EST


On 08/21, Roland Dreier wrote:
>
> > The main question is: what should requeue_delayed_work(dwork) do when
> > dwork->timer is not pending but dwork->work is queued or running?
> > Should it cancel dwork->work is this case?
>
> In my particular case it doesn't really matter. In the queued case it
> could leave it to run whenever it gets to the head of the workqueue. In
> the already running case then I think the timer should be reset. The
> main point is that if I do requeue_delayed_work() I want to make sure
> the work runs all the way through from the beginning at some point in
> the future. The pattern I have in mind is something like:
>
> spin_lock_irqsave(&mydata_lock);
> new_timeout = add_item_to_timeout_list();
> requeue_delayed_work(wq, &process_timeout_list_work, new_timeout);
> spin_unlock_irqsave(&mydata_lock);
>
> so if the process_timeout_list_work runs early or twice it doesn't
> matter; I just want to make sure that the work runs from the beginning
> and sees the new item I added to the list at some point after the
> requeue.

Hmm. But, asuming that process_timeout_list_work->func() takes mydata_lock
too, you can just use queue_delayed_work() ?

process_timeout_list_work can't miss new items, queue_delayed_work()
can only fail if dwork is pending and its ->func has not started yet.

No?

> Perhaps the semantics are sufficiently fuzzy and not general enough, so
> that the best answer is my special-case open coded change for my
> specific case. I don't know whether other places would even want a
> requeue_delayed_work() ... I simply raise this point because when I find
> myself reimplementing the structure of work_struct + timer because
> delayed_work API is lacking, then it seems prudent to consider extending
> delayed_work API instead.

Yes, I completely agree with you.

I think we need requeue_xxx helper(s), but the exact semantics is not
clear to me.

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/