All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rusty Russell <rusty@rustcorp.com.au>
To: Nick Piggin <npiggin@gmail.com>
Cc: Nick Piggin <npiggin@suse.de>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-kernel@vger.kernel.org,
	Jon Masters <jonathan@jonmasters.org>
Subject: Re: Is module refcounting racy?
Date: Wed, 31 Mar 2010 14:14:49 +1030	[thread overview]
Message-ID: <201003311414.49364.rusty@rustcorp.com.au> (raw)
In-Reply-To: <e619185d1003290958w23037176uc869c7dbc0a48bcc@mail.gmail.com>

On Tue, 30 Mar 2010 03:28:49 am Nick Piggin wrote:
> On Mon, Mar 29, 2010 at 8:12 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> > On Thu, 18 Mar 2010 09:25:34 pm Nick Piggin wrote:
> >> Hey,
> >>
> >> I've been looking at weird and wonderful ways to do scalable refcounting,
> >> for the vfs...
> >>
> >> Sadly, module refcounting doesn't fit my bill. But as far as I could see,
> >> it is racy.
> >
> > Other than for advisory purposes, the refcount is only checked against zero
> > under stop_machine.  For exactly this reason.
> 
> There definitely looks to me like there is code that checks the refcount
> *without* stop_machine. module_refcount is an exported function, and you
> expect drivers to get this right (scsi_device_put for a trivial example)

No, but there's a lot of history of crap drivers which wanted to poke at it.
And it's cute for debugging.

The scsi code is simply wrong.  But noone cares, since module removal is
so rare.

> , but
> it even looks like it is used in a racy way in kernel/module.c code.

Yep, though I don't know if anyone uses waiting module removal AFAICT
though; there's not even a modprobe option for it.

> Either we need to take my patch, or audit t, and put a WARN_ON
> if it is called while not under stop_machine.

So can you send me a proper annotated signed-off patch to queue?

Note that years ago it was decided that module reference counting would be
best effort, rather than perfect.  I disagreed, but we've lived with it
surprisingly well.

I wonder if by caring even *less*, we can lose a lot of complexity without
noticeably increasing the bug count.  Make modules run their own reference
counts and just sleep for a while to see if the reference count changes.
If not, assume it's good to be removed.  If reference count still hasn't
moved after another minute or so, actually free the memory.

Thanks,
Rusty.

  reply	other threads:[~2010-03-31  3:44 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-18 10:55 Is module refcounting racy? Nick Piggin
2010-03-29  9:12 ` Rusty Russell
2010-03-29 16:58   ` Nick Piggin
2010-03-31  3:44     ` Rusty Russell [this message]
2010-04-01  8:09       ` Nick Piggin
2010-04-01 15:55         ` Linus Torvalds
2010-04-06  2:39           ` Rusty Russell
2010-04-06  5:05           ` Nick Piggin
2010-04-06  6:19             ` Eric Dumazet
2010-04-06  7:38               ` Nick Piggin

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=201003311414.49364.rusty@rustcorp.com.au \
    --to=rusty@rustcorp.com.au \
    --cc=jonathan@jonmasters.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=npiggin@gmail.com \
    --cc=npiggin@suse.de \
    --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.