Re: [PATCH 2/2] module: fix bne2"gave up waiting for init of module libcrc32c"

[Andrew Morton]

On Mon, 31 May 2010 21:32:27 +0930 Rusty Russell <rusty@xxxxxxxxxxxxxxx> wrote:

> 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 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 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.

Who's returning -EBUSY?  request_module()?  If so, are you requiring
that all code which might call request_module() be correctly
propagating error codes back?  Please spell this all out?

Because I keep on coming across code which does

	if (foo() < 0)
		return -EWHATEVER;

or
		return -1;

I try to stamp it out, but they have me outnumbered.

I think transgressions are sufficiently rare that the patch will be OK.
Plus we needed to fix transgressors anyway.

After your changes, what would be the observable effects if this code
encountered a return-value-corruptor?


Also, I bet there are drivers which return -EBUSY from their
module_init() functions if the hardware's in an unexpected state.  What
happens?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/