From: Rusty Russell <rusty@rustcorp.com.au>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Nick Piggin <npiggin@suse.de>, Nick Piggin <npiggin@gmail.com>,
linux-kernel@vger.kernel.org,
Jon Masters <jonathan@jonmasters.org>
Subject: Re: Is module refcounting racy?
Date: Tue, 6 Apr 2010 12:09:10 +0930 [thread overview]
Message-ID: <201004061209.10647.rusty@rustcorp.com.au> (raw)
In-Reply-To: <alpine.LFD.2.00.1004010852170.3707@i5.linux-foundation.org>
On Fri, 2 Apr 2010 02:25:59 am Linus Torvalds wrote:
>
> On Thu, 1 Apr 2010, Nick Piggin wrote:
> >
> > I think it can be done racelessly with my patch, which is not really too
> > much overhead. I think if this is considered too much, then we should
> > either fix code and preferably de-export and remove module_refcount from
> > drivers, or remove module removal completely.
>
> I doubt your patch matters too much, but I like it conceptually and it
> seems to be a nice basis for perhaps doing something clever in the long
> run.
>
> [ ie avoiding the stop_machine and instead perhaps doing some optimistic
> thing like "see if we seem to be unused right now, then unregister us,
> and see - after unregistering - that the usage counts haven't increased,
> and re-register if they have. ]
I dislike that we can see spurious failure for some random try_module_get
caller.
But perhaps that's inherent in module removal: someone can miss out, and if
you care, don't try to remove modules.
And grepping for try_module_get() reveals a suspicious (growing) number of
try_module_get(THIS_MODULE) which is almost always wrong. If we're not
perfect, maybe we should aim for simple?
> So I'd like to apply it as a "good improvement, even if module unloading
> which is the only thing that _should_ care deeply should already be under
> stop-machine".
>
> But I'd like an ack or two first.
Yep.
Acked-by: Rusty Russell <rusty@rustcorp.com.au>
Cheers,
Rusty.
next prev parent reply other threads:[~2010-04-06 2:39 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
2010-04-01 8:09 ` Nick Piggin
2010-04-01 15:55 ` Linus Torvalds
2010-04-06 2:39 ` Rusty Russell [this message]
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=201004061209.10647.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.