All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rusty Russell <rusty@rustcorp.com.au>
To: Andi Kleen <andi@firstfloor.org>
Cc: Max Krasnyanskiy <maxk@qualcomm.com>,
	Tejun Heo <htejun@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Kathy Staples <kathy.staples@au1.ibm.com>
Subject: Re: Module loading/unloading and "The Stop Machine"
Date: Tue, 4 Mar 2008 12:21:05 +1100	[thread overview]
Message-ID: <200803041221.05434.rusty@rustcorp.com.au> (raw)
In-Reply-To: <p73prup8awh.fsf@bingen.suse.de>

On Friday 22 February 2008 22:53:50 Andi Kleen wrote:
> Max Krasnyanskiy <maxk@qualcomm.com> writes:
> > static struct module *load_module(void __user *umod,
> >                                  unsigned long len,
> >                                  const char __user *uargs)
> > {
> >      ...
> >
> >      /* Now sew it into the lists so we can get lockdep and oops
> >         * info during argument parsing.  Noone should access us, since
> >         * strong_try_module_get() will fail. */
> >        stop_machine_run(__link_module, mod, NR_CPUS);
> >      ...
> > }
>
> Wow you found some really bad code. I bet it wouldn't be that
> difficult to fix the code to allow oops safe list insertion
> without using the big stop machine overkill hammer.

Yes it's overkill and it's becoming more noticable with larger machines.  
(Also, stop_machine is heavier than it should be, Kathy is working on fixing 
that though).

Most obvious is to add __ "lockless, but I'm in trouble anyway" accessors, but 
last I looked they'd have to be plumbed through a fair bit of code.

>From a quick reading of the code, a lockless add is possible, but it's fragile 
if one of the readers does something more complex in future.

Thanks,
Rusty.


  parent reply	other threads:[~2008-03-04  1:21 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-08  2:38 Module loading/unloading and "The Stop Machine" Max Krasnyansky
2008-02-08 23:11 ` Max Krasnyanskiy
2008-02-14  4:02 ` Tejun Heo
2008-02-22  1:24   ` Max Krasnyanskiy
2008-02-22  1:38     ` Tejun Heo
2008-02-22  1:47       ` Max Krasnyanskiy
2008-02-22  1:59         ` Tejun Heo
2008-02-22  2:16           ` Max Krasnyanskiy
2008-02-22 11:53         ` Andi Kleen
2008-02-22 22:41           ` Max Krasnyanskiy
2008-03-04  1:21           ` Rusty Russell [this message]
2008-02-14  5:02 ` Jike Song
2008-02-14  5:38   ` Tejun Heo
2008-03-04  3:51   ` 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=200803041221.05434.rusty@rustcorp.com.au \
    --to=rusty@rustcorp.com.au \
    --cc=akpm@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=htejun@gmail.com \
    --cc=kathy.staples@au1.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maxk@qualcomm.com \
    --cc=torvalds@linux-foundation.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.