All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: LKML <linux-kernel@vger.kernel.org>
Subject: Re: cancel_delayed_work and its use of del_timer_sync
Date: Mon, 24 Aug 2009 19:49:10 +0200	[thread overview]
Message-ID: <20090824174910.GA16202@redhat.com> (raw)
In-Reply-To: <20090824065111.A51D8526EC9@mailhub.coreip.homeip.net>

Hi Dmitry,

(add Roland and Stefan)

On 08/23, Dmitry Torokhov wrote:
>
> I noticed that you went back and forth on using del_timer() vs.
> del_timer_sync() in cancel_delayed_work(), finally settling on using
> del_timer_sync().

Yes. del_timer() can't work when the user does something like

	cancel_delayed_work(dw);
	flush_workqueue();

	kfree(dw);

Perhaps we can add __cancel_delayed_work() which uses del_timer(), most
callers don't really need del_timer_sync. I dunno.

> I must say that the fact that cancel_delayed_work()
> might sleep

Just in case, it can spin, but not sleep

> (given that there exists cancel_delayed_work_sync) catches a
> few drivers writers by surprise. Moreover it makes delayed works
> unsuitable in cases when we do want to cancel (so we can reschedule)
> work in an interrupt context.

Yep. it is not useable from irq. But only because nobody removed
"#ifdef CONFIG_SMP" from set_running_timer(). I guess I should send a
patch finally.

> it is possible to have a reschedule_delayed_work() taht would do a
> "soft" cancel and [re]submit the work. In the use cases I am concerned
> about we don't really care if work is not reliably cancelled, we just
> need to be able to schedule it earlier if it has already been scheduled
> for execution in some point in the future.

Well. this depends on how "soft" should be that cancel.

Consider the auto-rearming delayed work, its work->func() calls
queue_delayed_work(self, BIG_DELAY). The caller of requeue_work(SMALL_DELAY)
preempts cwq->thread right after it sets _PENDING.

Now, what should requeue_work() do ? Even if the requeue_work() and
work->func() run on different CPUs, in this case requeue_ must spin.

So. It is easy to create requeue_work() which never sleeps/spins, but
it can return the error in case it hits the queueing in progress.

Is it OK?

And another question, should it cancel (without sleep/spin) this dwork
if the timer has expired, the work is pending, but its ->func() has not
started yet?

Oleg.


  reply	other threads:[~2009-08-24 17:52 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-24  6:19 cancel_delayed_work and its use of del_timer_sync Dmitry Torokhov
2009-08-24 17:49 ` Oleg Nesterov [this message]
2009-08-24 18:07   ` Dmitry Torokhov
2009-08-24 17:51 ` Oleg Nesterov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090824174910.GA16202@redhat.com \
    --to=oleg@redhat.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.