All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Andi Kleen <andi@firstfloor.org>
Cc: rusty@rustcorp.com.au, linux-kernel@vger.kernel.org, akpm@osdl.org
Subject: Re: [PATCH] Remove stop_machine during module load
Date: Fri, 29 Aug 2008 15:07:37 -0700	[thread overview]
Message-ID: <20080829220737.GG6725@linux.vnet.ibm.com> (raw)
In-Reply-To: <20080829212330.GC26610@one.firstfloor.org>

On Fri, Aug 29, 2008 at 11:23:30PM +0200, Andi Kleen wrote:
> Hi Paul,
> 
> Thanks for the excellent review.
> 
> On Fri, Aug 29, 2008 at 01:44:57PM -0700, Paul E. McKenney wrote:
> > 
> > OK, what about the read side?  Not so good for __unlink_module() to yank
> 
> That's independent from my patch isn't it? I don't think I'm changing
> anything here. All of the issues you're pointing out are already
> in the code (except for the missing read_barrier_depends() perhaps)
> 
> I think the lockless users like oops or sysrq-t typically have preemption
> disabled, so they should be ok regarding that. 

Ah -- perhaps I was confusing preventing CPU hotplug with preventing
stop_machine().  So disabling preemption holds off stop_machine()?
Yep, looks that way.

> > the module out from under a reader.  Therefore, all readers must either
> > disable interrupts to block stop_machine() or must hold some sort of
> > mutex that prevents modules from being unloaded.
> > 
> > First, where the heck -is- the read side...
> > 
> > o	each_symbol() needs its list_for_each_entry() to become
> > 	list_for_each_entry_rcu() and needs local_irq_disable()
> 
> Ah that's needed for the Alpha barrier depends semantics,
> right? 

Yep!  And to prevent compiler optimizations that could have the
same effect.

> > 	Yet another approach would be to use call_rcu() to defer the
> > 	various kfree() &c calls later in free_module.
> 
> I think that would be a the better approach.

Or maybe just disable preemption around the remaining readers, preventing
any stop_machine()-based deletions from being carried out during the
searches.

(And here I call myself a fan of real-time response!!!  But I suppose
that stop_machine() is going to be pretty hard on realtime response in
any case, so just don't mess with modules while your real-time
application is running...)

							Thanx, Paul

  reply	other threads:[~2008-08-29 22:07 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-29 19:17 [PATCH] Remove stop_machine during module load Andi Kleen
2008-08-29 20:44 ` Paul E. McKenney
2008-08-29 21:23   ` Andi Kleen
2008-08-29 22:07     ` Paul E. McKenney [this message]
2008-09-01  7:02 ` Rusty Russell
2008-09-01  8:52   ` Andi Kleen

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=20080829220737.GG6725@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=akpm@osdl.org \
    --cc=andi@firstfloor.org \
    --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.