linux-kernel - Re: Is adding requeue_delayed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 31 Aug 2009 17:44:31 -0700
From:	Dmitry Torokhov <dmitry.torokhov@...il.com>
To:	Roland Dreier <rdreier@...co.com>
Cc:	Oleg Nesterov <oleg@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: Is adding requeue_delayed_work() a good idea

On Wed, Aug 26, 2009 at 11:42:36AM -0700, Roland Dreier wrote:
> 
>  > 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.
> 

I can add the necessary locking in my case - I think it is actually needed
there even without this particular change.

-- 
Dmitry
--
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/