LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@tv-sign.ru>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Horms <horms@verge.net.au>, Daniel Drake <dsd@gentoo.org>,
	Ingo Molnar <mingo@elte.hu>,
	linux-kernel@vger.kernel.org, David Howells <dhowells@redhat.com>
Subject: Re: [PATCH 3/6] workqueue: make cancel_rearming_delayed_workqueue() work on idle dwork
Date: Thu, 8 Feb 2007 12:46:01 +0300	[thread overview]
Message-ID: <20070208094601.GA128@tv-sign.ru> (raw)
In-Reply-To: <20070208003927.d8f8cb78.akpm@linux-foundation.org>

On 02/08, Andrew Morton wrote:
>
> On Thu, 8 Feb 2007 11:35:39 +0300 Oleg Nesterov <oleg@tv-sign.ru> wrote:
> 
> > Andrew, do you think it is worth to tweak delayed works so it would be
> > possible to use flush_work(dwork->work) ?
> > 
> 
> I've completely lost track of what you've been doing in there (this is a
> problem) but sure, if the patch isn't too horrid it's always better to be
> robust in the core than to have to work around inadequacies in the callers.

It is not so obvious to me what should be done. Note that this problem is not
connected to recent changes, there were (I hope) completely transparent for
the delayed works.

The comment for cancel_delayed_work() work says

	Note that the work callback function may still be running on return from
	cancel_delayed_work(). Run flush_scheduled_work() or flush_work() to wait
	on it.

The same is true for cancel_rearming_delayed_work(), but not documented. Note
also that the comment above is wrong, we can't use flush_work(dwork->work), it
was never supposed to do because queue_delayed_work() use work->data "wrongly".

Now,

	- We can change cancel_rearming_delayed_work() so it does a final
	  flush_workqueue(). But this means that 2 flavors of cancel delayed
	  work will have a subtle difference.

		OR

	- Document the fact that cancel_rearming_delayed_work() doesn't
	  garantee that ->func() is not running upon return, fix affected
	  callers.

Finally, we can also tweak delaed_works so it will actually be possible
to use flush_work(dwork->work) after cancel_{,rearming_}delayed_work().
Seems to make sense, but needs (hopefully not too horrid) changes.

And other problems. Currently cancel_rearming_delayed_work(dwork) will hang
if dwork was never scheduled, or cancel_rearming_delayed_work() was already
called before. The first problem is solved by this patch, the second is still
here. The fix is simple _unless_ we are going to implement "flush_work() works
on dwork->work" above.

Oh, I can't make a decision, please tell me...

Oleg.


      reply	other threads:[~2007-02-08  9:46 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-06 23:30 Oleg Nesterov
2007-02-07 14:33 ` Daniel Drake
2007-02-07 15:16   ` Oleg Nesterov
2007-02-07 17:43     ` Oleg Nesterov
2007-02-08  2:20       ` Horms
2007-02-08  8:35         ` Oleg Nesterov
2007-02-08  8:39           ` Andrew Morton
2007-02-08  9:46             ` Oleg Nesterov [this message]

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=20070208094601.GA128@tv-sign.ru \
    --to=oleg@tv-sign.ru \
    --cc=akpm@linux-foundation.org \
    --cc=dhowells@redhat.com \
    --cc=dsd@gentoo.org \
    --cc=horms@verge.net.au \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --subject='Re: [PATCH 3/6] workqueue: make cancel_rearming_delayed_workqueue() work on idle dwork' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).