Created attachment 5870 [details] proposed patch already_loaded function should not return true if the state of the module in /proc/modules is not 'Live'. Reason being, Parallel module load request on SMP architecture might result in failure or unexpected behaviour via modprobe. Added the check of 'Live' state in already_loaded function to handle the same.
(In reply to comment #0) > already_loaded function should not return true if the state > of the module in /proc/modules is not 'Live'. Reason being, > Parallel module load request on SMP architecture might result in > failure or unexpected behaviour via modprobe. Added the check of > 'Live' state in already_loaded function to handle the same. The state can be Unloading, Loading, or Live. The code which checks already_loaded() is this one: if (!is_remove && already_loaded(name)) { dbg1_error_msg("nothing to do for '%s'", name); return; } I don't see why it would be better if we change "Loading" modules to be reported as !already_loaded(). If we do, then an attempt to load a module while it is already being loaded will be made (currently, we skip such concurrent loading). Is this better? Why?
(In reply to comment #1) > (In reply to comment #0) > > already_loaded function should not return true if the state > > of the module in /proc/modules is not 'Live'. Reason being, > > Parallel module load request on SMP architecture might result in > > failure or unexpected behaviour via modprobe. Added the check of > > 'Live' state in already_loaded function to handle the same. > > The state can be Unloading, Loading, or Live. > > The code which checks already_loaded() is this one: > > if (!is_remove && already_loaded(name)) { > dbg1_error_msg("nothing to do for '%s'", name); > return; > } > > I don't see why it would be better if we change "Loading" modules to be > reported as !already_loaded(). If we do, then an attempt to load a module while > it is already being loaded will be made (currently, we skip such concurrent > loading). Is this better? Why? usecase: two sd cards are being mounted in parallel at same time on dual core. example modules which are getting loaded is nls_cp437. While one module is being loaded , it makes state in /proc/modules as 'coming' and then starts doing its module init function ( in our case - registering nls). meanwhile on other core , if modprobe returns that is has already been loaded, then it will continue and search for the nls list which is not yet finished from first module init. This fails resulting in not mounting sd card. In general - with this patch - an attempt to load a module while it is already being loaded will be made as you mentioned. However since it is handled properly in linux kernel (module.c file ) it will not make much difference.
Thanks! Fixed in git.
(In reply to comment #3) > Thanks! > Fixed in git. Nice fix - But one review comment - ret | 1; condition is wrong. it always returns true and already_loaded as success. I wish my name was also there in <Signed-off> section !!
(In reply to comment #4) > (In reply to comment #3) > > Thanks! > > Fixed in git. > > Nice fix - > > But one review comment - > > ret | 1; condition is wrong. > it always returns true and already_loaded as success. > > > I wish my name was also there in <Signed-off> section !! Yet again disappointed that my name is not present in signed-off section even for the review comment fix - http://git.busybox.net/busybox/commit/?id=402afe1cc69ea505c9bf82ffe06e51ffecf694df Even the reviewer's name could have been included along the with the coders name.