<feed xmlns='http://www.w3.org/2005/Atom'>
<title>linux.git/kernel/module.c, branch v2.6.35-rc2</title>
<subtitle>Linux kernel source tree</subtitle>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/'/>
<entry>
<title>module: fix bne2 "gave up waiting for init of module libcrc32c"</title>
<updated>2010-06-05T01:47:37+00:00</updated>
<author>
<name>Rusty Russell</name>
<email>rusty@rustcorp.com.au</email>
</author>
<published>2010-06-05T17:17:37+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=9bea7f23952d5948f8e5dfdff4de09bb9981fb5f'/>
<id>9bea7f23952d5948f8e5dfdff4de09bb9981fb5f</id>
<content type='text'>
Problem: it's hard to avoid an init routine stumbling over a
request_module these days.  And it's not clear it's always a bad idea:
for example, a module like kvm with dynamic dependencies on kvm-intel
or kvm-amd would be neater if it could simply request_module the right
one.

In this particular case, it's libcrc32c:

	libcrc32c_mod_init
	 crypto_alloc_shash
	  crypto_alloc_tfm
	   crypto_find_alg
	    crypto_alg_mod_lookup
	     crypto_larval_lookup
	      request_module

If another module is waiting inside resolve_symbol() for libcrc32c to
finish initializing (ie. bne2 depends on libcrc32c) then it does so
holding the module lock, and our request_module() can't make progress
until that is released.

Waiting inside resolve_symbol() without the lock isn't all that hard:
we just need to pass the -EBUSY up the call chain so we can sleep
where we don't hold the lock.  Error reporting is a bit trickier: we
need to copy the name of the unfinished module before releasing the
lock.

Other notes:
1) This also fixes a theoretical issue where a weak dependency would allow
   symbol version mismatches to be ignored.
2) We rename use_module to ref_module to make life easier for the only
   external user (the out-of-tree ksplice patches).

Signed-off-by: Rusty Russell &lt;rusty@rustcorp.com.au&gt;
Cc: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
Cc: Tim Abbot &lt;tabbott@ksplice.com&gt;
Tested-by: Brandon Philips &lt;bphilips@suse.de&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Problem: it's hard to avoid an init routine stumbling over a
request_module these days.  And it's not clear it's always a bad idea:
for example, a module like kvm with dynamic dependencies on kvm-intel
or kvm-amd would be neater if it could simply request_module the right
one.

In this particular case, it's libcrc32c:

	libcrc32c_mod_init
	 crypto_alloc_shash
	  crypto_alloc_tfm
	   crypto_find_alg
	    crypto_alg_mod_lookup
	     crypto_larval_lookup
	      request_module

If another module is waiting inside resolve_symbol() for libcrc32c to
finish initializing (ie. bne2 depends on libcrc32c) then it does so
holding the module lock, and our request_module() can't make progress
until that is released.

Waiting inside resolve_symbol() without the lock isn't all that hard:
we just need to pass the -EBUSY up the call chain so we can sleep
where we don't hold the lock.  Error reporting is a bit trickier: we
need to copy the name of the unfinished module before releasing the
lock.

Other notes:
1) This also fixes a theoretical issue where a weak dependency would allow
   symbol version mismatches to be ignored.
2) We rename use_module to ref_module to make life easier for the only
   external user (the out-of-tree ksplice patches).

Signed-off-by: Rusty Russell &lt;rusty@rustcorp.com.au&gt;
Cc: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
Cc: Tim Abbot &lt;tabbott@ksplice.com&gt;
Tested-by: Brandon Philips &lt;bphilips@suse.de&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>module: verify_export_symbols under the lock</title>
<updated>2010-06-05T01:47:37+00:00</updated>
<author>
<name>Rusty Russell</name>
<email>rusty@rustcorp.com.au</email>
</author>
<published>2010-06-05T17:17:37+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=be593f4ce4eb1bd40e38fdc403371f149f6f12eb'/>
<id>be593f4ce4eb1bd40e38fdc403371f149f6f12eb</id>
<content type='text'>
It disabled preempt so it was "safe", but nothing stops another module
slipping in before this module is added to the global list now we don't
hold the lock the whole time.

So we check this just after we check for duplicate modules, and just
before we put the module in the global list.

(find_symbol finds symbols in coming and going modules, too).

Signed-off-by: Rusty Russell &lt;rusty@rustcorp.com.au&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
It disabled preempt so it was "safe", but nothing stops another module
slipping in before this module is added to the global list now we don't
hold the lock the whole time.

So we check this just after we check for duplicate modules, and just
before we put the module in the global list.

(find_symbol finds symbols in coming and going modules, too).

Signed-off-by: Rusty Russell &lt;rusty@rustcorp.com.au&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>module: move find_module check to end</title>
<updated>2010-06-05T01:47:37+00:00</updated>
<author>
<name>Linus Torvalds</name>
<email>torvalds@linux-foundation.org</email>
</author>
<published>2010-06-05T17:17:36+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=3bafeb6247042dcbb72b0141ec7c7107de9f0b99'/>
<id>3bafeb6247042dcbb72b0141ec7c7107de9f0b99</id>
<content type='text'>
I think Rusty may have made the lock a bit _too_ finegrained there, and
didn't add it to some places that needed it. It looks, for example, like
PATCH 1/2 actually drops the lock in places where it's needed
("find_module()" is documented to need it, but now load_module() didn't
hold it at all when it did the find_module()).

Rather than adding a new "module_loading" list, I think we should be able
to just use the existing "modules" list, and just fix up the locking a
bit.

In fact, maybe we could just move the "look up existing module" a bit
later - optimistically assuming that the module doesn't exist, and then
just undoing the work if it turns out that we were wrong, just before
adding ourselves to the list.

Signed-off-by: Rusty Russell &lt;rusty@rustcorp.com.au&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
I think Rusty may have made the lock a bit _too_ finegrained there, and
didn't add it to some places that needed it. It looks, for example, like
PATCH 1/2 actually drops the lock in places where it's needed
("find_module()" is documented to need it, but now load_module() didn't
hold it at all when it did the find_module()).

Rather than adding a new "module_loading" list, I think we should be able
to just use the existing "modules" list, and just fix up the locking a
bit.

In fact, maybe we could just move the "look up existing module" a bit
later - optimistically assuming that the module doesn't exist, and then
just undoing the work if it turns out that we were wrong, just before
adding ourselves to the list.

Signed-off-by: Rusty Russell &lt;rusty@rustcorp.com.au&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>module: make locking more fine-grained.</title>
<updated>2010-06-05T01:47:36+00:00</updated>
<author>
<name>Rusty Russell</name>
<email>rusty@rustcorp.com.au</email>
</author>
<published>2010-06-05T17:17:36+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=75676500f8298f0ee89db12db97294883c4b768e'/>
<id>75676500f8298f0ee89db12db97294883c4b768e</id>
<content type='text'>
Kay Sievers &lt;kay.sievers@vrfy.org&gt; reports that we still have some
contention over module loading which is slowing boot.

Linus also disliked a previous "drop lock and regrab" patch to fix the
bne2 "gave up waiting for init of module libcrc32c" message.

This is more ambitious: we only grab the lock where we need it.

Signed-off-by: Rusty Russell &lt;rusty@rustcorp.com.au&gt;
Cc: Brandon Philips &lt;brandon@ifup.org&gt;
Cc: Kay Sievers &lt;kay.sievers@vrfy.org&gt;
Cc: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Kay Sievers &lt;kay.sievers@vrfy.org&gt; reports that we still have some
contention over module loading which is slowing boot.

Linus also disliked a previous "drop lock and regrab" patch to fix the
bne2 "gave up waiting for init of module libcrc32c" message.

This is more ambitious: we only grab the lock where we need it.

Signed-off-by: Rusty Russell &lt;rusty@rustcorp.com.au&gt;
Cc: Brandon Philips &lt;brandon@ifup.org&gt;
Cc: Kay Sievers &lt;kay.sievers@vrfy.org&gt;
Cc: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>module: Make module sysfs functions private.</title>
<updated>2010-06-05T01:47:36+00:00</updated>
<author>
<name>Rusty Russell</name>
<email>rusty@rustcorp.com.au</email>
</author>
<published>2010-06-05T17:17:36+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=6407ebb271fc34440b306f305e1efb7685eece26'/>
<id>6407ebb271fc34440b306f305e1efb7685eece26</id>
<content type='text'>
These were placed in the header in ef665c1a06 to get the various
SYSFS/MODULE config combintations to compile.

That may have been necessary then, but it's not now.  These functions
are all local to module.c.

Signed-off-by: Rusty Russell &lt;rusty@rustcorp.com.au&gt;
Cc: Randy Dunlap &lt;randy.dunlap@oracle.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
These were placed in the header in ef665c1a06 to get the various
SYSFS/MODULE config combintations to compile.

That may have been necessary then, but it's not now.  These functions
are all local to module.c.

Signed-off-by: Rusty Russell &lt;rusty@rustcorp.com.au&gt;
Cc: Randy Dunlap &lt;randy.dunlap@oracle.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>module: move sysfs exposure to end of load_module</title>
<updated>2010-06-05T01:47:36+00:00</updated>
<author>
<name>Rusty Russell</name>
<email>rusty@rustcorp.com.au</email>
</author>
<published>2010-06-05T17:17:36+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=80a3d1bb410e000e176931a076cdf19a1e89a955'/>
<id>80a3d1bb410e000e176931a076cdf19a1e89a955</id>
<content type='text'>
This means a little extra work, but is more logical: we don't put
anything in sysfs until we're about to put the module into the
global list an parse its parameters.

This also gives us a logical place to put duplicate module detection
in the next patch.

Signed-off-by: Rusty Russell &lt;rusty@rustcorp.com.au&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This means a little extra work, but is more logical: we don't put
anything in sysfs until we're about to put the module into the
global list an parse its parameters.

This also gives us a logical place to put duplicate module detection
in the next patch.

Signed-off-by: Rusty Russell &lt;rusty@rustcorp.com.au&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>module: fix kdb's illicit use of struct module_use.</title>
<updated>2010-06-05T01:47:36+00:00</updated>
<author>
<name>Rusty Russell</name>
<email>rusty@rustcorp.com.au</email>
</author>
<published>2010-06-05T17:17:35+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=c8e21ced08b39ef8dfe7236fb2a923a95f645262'/>
<id>c8e21ced08b39ef8dfe7236fb2a923a95f645262</id>
<content type='text'>
Linus changed the structure, and luckily this didn't compile any more.

Reported-by: Stephen Rothwell &lt;sfr@canb.auug.org.au&gt;
Signed-off-by: Rusty Russell &lt;rusty@rustcorp.com.au&gt;
Cc: Jason Wessel &lt;jason.wessel@windriver.com&gt;
Cc: Martin Hicks &lt;mort@sgi.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Linus changed the structure, and luckily this didn't compile any more.

Reported-by: Stephen Rothwell &lt;sfr@canb.auug.org.au&gt;
Signed-off-by: Rusty Russell &lt;rusty@rustcorp.com.au&gt;
Cc: Jason Wessel &lt;jason.wessel@windriver.com&gt;
Cc: Martin Hicks &lt;mort@sgi.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>module: Make the 'usage' lists be two-way</title>
<updated>2010-06-05T01:47:35+00:00</updated>
<author>
<name>Linus Torvalds</name>
<email>torvalds@linux-foundation.org</email>
</author>
<published>2010-05-31T19:19:37+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=2c02dfe7fe3fba97a5665d329d039d2415ea5607'/>
<id>2c02dfe7fe3fba97a5665d329d039d2415ea5607</id>
<content type='text'>
When adding a module that depends on another one, we used to create a
one-way list of "modules_which_use_me", so that module unloading could
see who needs a module.

It's actually quite simple to make that list go both ways: so that we
not only can see "who uses me", but also see a list of modules that are
"used by me".

In fact, we always wanted that list in "module_unload_free()": when we
unload a module, we want to also release all the other modules that are
used by that module.  But because we didn't have that list, we used to
first iterate over all modules, and then iterate over each "used by me"
list of that module.

By making the list two-way, we simplify module_unload_free(), and it
allows for some trivial fixes later too.

Signed-off-by: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
Signed-off-by: Rusty Russell &lt;rusty@rustcorp.com.au&gt; (cleaned &amp; rebased)
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When adding a module that depends on another one, we used to create a
one-way list of "modules_which_use_me", so that module unloading could
see who needs a module.

It's actually quite simple to make that list go both ways: so that we
not only can see "who uses me", but also see a list of modules that are
"used by me".

In fact, we always wanted that list in "module_unload_free()": when we
unload a module, we want to also release all the other modules that are
used by that module.  But because we didn't have that list, we used to
first iterate over all modules, and then iterate over each "used by me"
list of that module.

By making the list two-way, we simplify module_unload_free(), and it
allows for some trivial fixes later too.

Signed-off-by: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
Signed-off-by: Rusty Russell &lt;rusty@rustcorp.com.au&gt; (cleaned &amp; rebased)
</pre>
</div>
</content>
</entry>
<entry>
<title>Merge branch 'for-35' of git://repo.or.cz/linux-kbuild</title>
<updated>2010-06-01T15:55:52+00:00</updated>
<author>
<name>Linus Torvalds</name>
<email>torvalds@linux-foundation.org</email>
</author>
<published>2010-06-01T15:55:52+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=1f73897861b8ef0be64ff4b801f8d6f830f683b5'/>
<id>1f73897861b8ef0be64ff4b801f8d6f830f683b5</id>
<content type='text'>
* 'for-35' of git://repo.or.cz/linux-kbuild: (81 commits)
  kbuild: Revert part of e8d400a to resolve a conflict
  kbuild: Fix checking of scm-identifier variable
  gconfig: add support to show hidden options that have prompts
  menuconfig: add support to show hidden options which have prompts
  gconfig: remove show_debug option
  gconfig: remove dbg_print_ptype() and dbg_print_stype()
  kconfig: fix zconfdump()
  kconfig: some small fixes
  add random binaries to .gitignore
  kbuild: Include gen_initramfs_list.sh and the file list in the .d file
  kconfig: recalc symbol value before showing search results
  .gitignore: ignore *.lzo files
  headerdep: perlcritic warning
  scripts/Makefile.lib: Align the output of LZO
  kbuild: Generate modules.builtin in make modules_install
  Revert "kbuild: specify absolute paths for cscope"
  kbuild: Do not unnecessarily regenerate modules.builtin
  headers_install: use local file handles
  headers_check: fix perl warnings
  export_report: fix perl warnings
  ...
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
* 'for-35' of git://repo.or.cz/linux-kbuild: (81 commits)
  kbuild: Revert part of e8d400a to resolve a conflict
  kbuild: Fix checking of scm-identifier variable
  gconfig: add support to show hidden options that have prompts
  menuconfig: add support to show hidden options which have prompts
  gconfig: remove show_debug option
  gconfig: remove dbg_print_ptype() and dbg_print_stype()
  kconfig: fix zconfdump()
  kconfig: some small fixes
  add random binaries to .gitignore
  kbuild: Include gen_initramfs_list.sh and the file list in the .d file
  kconfig: recalc symbol value before showing search results
  .gitignore: ignore *.lzo files
  headerdep: perlcritic warning
  scripts/Makefile.lib: Align the output of LZO
  kbuild: Generate modules.builtin in make modules_install
  Revert "kbuild: specify absolute paths for cscope"
  kbuild: Do not unnecessarily regenerate modules.builtin
  headers_install: use local file handles
  headers_check: fix perl warnings
  export_report: fix perl warnings
  ...
</pre>
</div>
</content>
</entry>
<entry>
<title>module: fix reference to mod-&gt;percpu after freeing module.</title>
<updated>2010-05-31T18:21:32+00:00</updated>
<author>
<name>Rusty Russell</name>
<email>rusty@rustcorp.com.au</email>
</author>
<published>2010-05-31T10:23:50+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=293a7cfeedc2b2380a7c7274902323c3cf5f7575'/>
<id>293a7cfeedc2b2380a7c7274902323c3cf5f7575</id>
<content type='text'>
Rafael sees a sometimes crash at precpu_modfree from kernel/module.c; it
only occurred with another (since-reverted) patch, but that patch simply
changed timing to uncover this bug, it was otherwise unrelated.

The comment about the mod being freed is self-explanatory, but neither
Tejun nor I read it.  This bug was introduced in 259354deaa, after it
had previously been fixed in 6e2b75740b.  How embarrassing.

Reported-by: "Rafael J. Wysocki" &lt;rjw@sisk.pl&gt;
Signed-off-by: Rusty Russell &lt;rusty@rustcorp.com.au&gt;
Embarrassingly-Acked-by: Tejun Heo &lt;tj@kernel.org&gt;
Cc: Masami Hiramatsu &lt;mhiramat@redhat.com&gt;
Tested-by: "Rafael J. Wysocki" &lt;rjw@sisk.pl&gt;
Signed-off-by: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Rafael sees a sometimes crash at precpu_modfree from kernel/module.c; it
only occurred with another (since-reverted) patch, but that patch simply
changed timing to uncover this bug, it was otherwise unrelated.

The comment about the mod being freed is self-explanatory, but neither
Tejun nor I read it.  This bug was introduced in 259354deaa, after it
had previously been fixed in 6e2b75740b.  How embarrassing.

Reported-by: "Rafael J. Wysocki" &lt;rjw@sisk.pl&gt;
Signed-off-by: Rusty Russell &lt;rusty@rustcorp.com.au&gt;
Embarrassingly-Acked-by: Tejun Heo &lt;tj@kernel.org&gt;
Cc: Masami Hiramatsu &lt;mhiramat@redhat.com&gt;
Tested-by: "Rafael J. Wysocki" &lt;rjw@sisk.pl&gt;
Signed-off-by: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
</pre>
</div>
</content>
</entry>
</feed>
