All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: "Frank Ch. Eigler" <fche@redhat.com>,
	linux-kernel@vger.kernel.org, akpm@linux-foundation.org
Subject: [PATCH] Module : call synchronize_sched() between module exit() and free.
Date: Tue, 29 Jul 2008 22:27:51 -0400	[thread overview]
Message-ID: <20080730022751.GA15866@Krystal> (raw)
In-Reply-To: <200807301140.59745.rusty@rustcorp.com.au>

> Hi Mathieu,
> 
>    Yes: stop_machine is merely used to atomically check the module refcount 
> for zero and set the state so it can't be incremented again (ie. 
> try_module_get will fail).
> 
>    So placing a tracepoint or marker in a module does not bump the module 
> refcount?  If that's true, then there needs to be some kind of 
> remove_markers_from_module() call after module->exit(), which should do the 
> synchronize_sched() or whatever, right?
> 
> Rusty.

Actually, it's not placing a marker/tracepoint in a module which causes
a problem, this is a simple function call after all, and correctly dealt
with by current module.c code.

The problem comes from a probe function (the callback) that would be
registered to be called from a marker and would sit in an unloadable 
kernel module. I would not want to tie the refcount of the probe modules
to the fact that they are connected to a marker because it would then
become impossible to unload them due to the fact that unregistration is
done in module exit().

This is one of the reasons why I disable preemption around the marker
site (the function call) : to make sure I can can unregister the
callback, wait for a quiescent state (with synchronize_sched()) and then
free the module memory.

This would give the following supplementary guarantee about module
teardown : every function called with preemption off and unregistered in
the module exit() would reach a quiescent state before the module is
freed. Given this does apply to rarely used code (module unload), I
think it might be ok to simply add a call to synchronize_sched() before
the module memory is freed. Not tying this to markers/tracepoints would
keep the behavior consistant across various build options, which is IMHO
a good thing.

I could also just document that a mandatory "synchronize_sched()" should
be called at the end of the probe module exit() function which makes
sure the probes has reached a quiescent state.

I don't want to add a synchronize_sched() into the marker/tracepoint
probe unregistration code because I want to keep batch probe
unregistration fast enough so it does no take ~5 seconds to unload ~100
probes. (may take longer on a loaded SMP system)

Mathieu

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

  reply	other threads:[~2008-07-30  2:28 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-17 15:57 [patch 0/4] Port KVM-trace to tracepoints Mathieu Desnoyers
2008-07-17 15:57 ` [patch 1/4] kvm move VMCS Encodings to system headers Mathieu Desnoyers
2008-07-17 15:57 ` [patch 2/4] kvm move VMCS read " Mathieu Desnoyers
2008-07-17 15:57 ` [patch 3/4] KVM move register read-write " Mathieu Desnoyers
2008-07-17 15:57 ` [patch 4/4] KVM-trace port to tracepoints Mathieu Desnoyers
2008-07-17 16:49   ` Jan Kiszka
2008-07-17 17:28     ` Mathieu Desnoyers
2008-07-22 16:04       ` Jan Kiszka
2008-07-22 18:46         ` Avi Kivity
2008-07-23  7:49           ` Peter Zijlstra
2008-07-23  8:08             ` Avi Kivity
2008-07-23  8:55               ` Peter Zijlstra
2008-07-23  9:32                 ` Avi Kivity
2008-07-23  9:53                   ` Peter Zijlstra
2008-07-23 13:15                     ` Mathieu Desnoyers
2008-07-23 10:03                 ` Christoph Hellwig
2008-07-23 10:08                   ` Avi Kivity
2008-07-23 10:13                     ` Christoph Hellwig
2008-07-23 13:20               ` Mathieu Desnoyers
2008-07-17 16:52   ` Anthony Liguori
2008-07-17 17:04     ` Mathieu Desnoyers
2008-07-22 18:42 ` [patch 0/4] Port KVM-trace " Avi Kivity
2008-07-22 19:16   ` Frank Ch. Eigler
2008-07-22 19:31     ` Avi Kivity
2008-07-22 19:54       ` Frank Ch. Eigler
2008-07-22 22:12       ` [patch 0/4] Port KVM-trace to tracepoints -> LTTng ? Mathieu Desnoyers
2008-07-27 10:11         ` Avi Kivity
2008-07-28  0:54           ` [RFC] LTTng merge plan Mathieu Desnoyers
2008-07-28  0:54             ` Mathieu Desnoyers
2008-07-29 16:18             ` Frank Ch. Eigler
2008-07-29 16:18               ` Frank Ch. Eigler
2008-07-29 17:01               ` Mathieu Desnoyers
2008-07-29 17:01                 ` Mathieu Desnoyers
     [not found]                 ` <20080729211543.GB17097@redhat.com>
2008-07-29 22:41                   ` module-placed markers/tracepoints Mathieu Desnoyers
2008-07-29 23:01                     ` Frank Ch. Eigler
2008-07-29 23:19                       ` Mathieu Desnoyers
2008-07-30  1:40                     ` Rusty Russell
2008-07-30  2:27                       ` Mathieu Desnoyers [this message]
2008-07-30  3:04                         ` [PATCH] Module : call synchronize_sched() between module exit() and free Rusty Russell
2008-07-30  4:05                           ` Mathieu Desnoyers
2008-07-30 11:40                         ` Frank Ch. Eigler
2008-07-30 14:09                           ` Mathieu Desnoyers
2008-07-31  0:54                             ` Rusty Russell

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=20080730022751.GA15866@Krystal \
    --to=mathieu.desnoyers@polymtl.ca \
    --cc=akpm@linux-foundation.org \
    --cc=fche@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rusty@rustcorp.com.au \
    /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.