[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ada7hwqto0z.fsf@cisco.com>
Date: Wed, 26 Aug 2009 11:42:36 -0700
From: Roland Dreier <rdreier@...co.com>
To: Oleg Nesterov <oleg@...hat.com>
Cc: linux-kernel@...r.kernel.org,
Dmitry Torokhov <dmitry.torokhov@...il.com>
Subject: Re: Is adding requeue_delayed_work() a good idea
> OK, in this case I think we have a simple solution,
>
> // like cancel_delayed_work, but uses del_timer().
> // this means, if it returns 0 the timer function may be
> // running and the queueing is in progress. The caller
> // can't rely on flush_workqueue/etc
> static inline int __cancel_delayed_work(struct delayed_work *work)
> {
> int ret;
>
> ret = del_timer(&work->timer);
> if (ret)
> work_clear_pending(&work->work);
> return ret;
> }
>
> Now, you can do
>
> spin_lock_irqsave(&mydata_lock);
> new_timeout = add_item_to_timeout_list();
>
> __cancel_delayed_work(&process_timeout_list_work);
> queue_delayed_work(wq, &process_timeout_list_work, new_timeout);
>
> spin_unlock_irqsave(&mydata_lock);
>
> If queue_delayed_work() fails, this means that WORK_STRUCT_PENDING is set,
> dwork->work is already queued or the queueing is in progress. In both
> cases it will run "soon" as if we just called queue_work(&dwork->work).
This looks like it would work well. If we can get this into 2.6.32 then
I will drop my patch and switch to this approach instead.
> But this assumes nobody else does queue_delayed_work(dwork, HUGE_DELAY) in
> parallel, otherwise we can lose the race and another caller can setup
> HUGE_DELAY timeout.
In my case this is fine -- all uses of queue_delayed_work() are
synchronized with the same lock. So any place that tries to shorten the
timeout will succeed.
> In particular, if process_timeout_list_work->func() itself uses
> queue_delay_work() to re-arm itself we can race. Bu t I think it is always
> possible to do something to synchronize with work->func, for example
> work->func() can re-arm itself _before_ it scans timeout_list (under the
> same lock). This way, if re-queue code above fails because work->func()
> wins, work->func() must see the new additions to timeout_list.
In my case, work function does do queue_delayed_work(), but with the
same lock as everyone else held. So there should be no race.
Thanks,
Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists