LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@tv-sign.ru>
To: Horms <horms@verge.net.au>
Cc: Daniel Drake <dsd@gentoo.org>,
	Andrew Morton <akpm@linux-foundation.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 11:35:39 +0300	[thread overview]
Message-ID: <20070208083539.GA85@tv-sign.ru> (raw)
In-Reply-To: <20070208022051.GC17585@verge.net.au>

On 02/08, Horms wrote:
>
> On Wed, Feb 07, 2007 at 08:43:55PM +0300, Oleg Nesterov wrote:
> > 
> > I think we have another problem with delayed_works.
> > 
> > cancel_rearming_delayed_workqueue() doesn't garantee that the ->func() is not
> > running upon return. I don't know if it is bug or not, the comment says nothing
> > about that.
> > 
> > However, we have the callers which seem to assume the opposite, example
> > 
> > 	net/ipv4/ipvs/ip_vs_core.c
> > 
> > 		module_exit
> > 		    ip_vs_cleanup
> > 		        ip_vs_control_cleanup
> > 		            cancel_rearming_delayed_work
> > 		// done
> > 
> > This is unsafe. The module may be unloaded and the memory may be freed
> > while defense_work_handler() is still running/preempted.
> > 
> > Unless I missed something, which side should be fixed?
> 
> Assuming the decision is to fix the ipvs side, is the fix
> just to remove the call to cancel_rearming_delayed_work() in
> ip_vs_control_cleanup() ?

I think ip_vs_control_cleanup() should also do flush_workqueue() after
cancel_rearming_delayed_work().

This is ugly, because we have flush_work() but can't use it on delayed
works. This is possible to change, but not so trivial.

Andrew, do you think it is worth to tweak delayed works so it would be
possible to use flush_work(dwork->work) ?

Oleg.


  reply	other threads:[~2007-02-08  8:35 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 [this message]
2007-02-08  8:39           ` Andrew Morton
2007-02-08  9:46             ` 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=20070208083539.GA85@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).