try_then_request_module
Quoted 5 lines
ChangeSet 2003/05/17 12:39:18-07:00, torvalds @ home.transmeta.com [diffview]
Make
request_module()take a printf-like vararg argument instead of a string.This is what a lot of the callers really wanted.
Excellent! I'll close my older version of the same thing.
If someone is feeling eager, many callers could change to try_then_request_module(), eg:
drivers/net/ppp_generic.c +2 -7
@@ -1959,13 +1959,8 @@ ppp_set_compress(struct ppp *ppp, unsign || ccp_option[1] < 2 || ccp_option[1] > data.length) goto out; - cp =find_compressor (ccp_option[0 ]); -#ifdef CONFIG_KMOD -if (cp ==0 ) { -request_module ("ppp-compress-%d" , ccp_option[0 ]); - cp =find_compressor (ccp_option[0 ]); - } -#endif /* CONFIG_KMOD */ + cp =try_then_request_module (find_compressor (ccp_option[0 ]), +"ppp-compress-%d" , ccp_option[0 ]);if (cp ==0 ) goto out;
Thanks!
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
Hi Rusty,
hi LKML,
Quoted 2 lines · Rusty Russell
If someone is feeling eager, many callers could change to
try_then_request_module(), eg:
[search || request_module]
Many implementation do this with a search and retry the search (if clever with a goto and a flag variable to save kernel size) after module loading.
All that implemented in the search routine, which you have to supply anyway.
So try_then_request_module() will consolidate the the branch or in the worst case just duplicating that code everywhere (depends on wether you implement it as a non-inline function or define).
Usally this is all as simple as:
int module_loaded_flag=0;retry_with_module_loaded:
/* search code */
if (!module_loaded_flag && !found) {
module_loaded_flag=1;
if (!request_module(bla))
goto retry_with_module_loaded;
}
return found;which is very space effecient and also still readable.
Regards
Ingo Oeser
Hi Ingo.
Quoted 15 lines
Usually this is all as simple as:
int module_loaded_flag=0;
retry_with_module_loaded:/* search code */ if (!module_loaded_flag && !found) { module_loaded_flag=1; if (!request_module(bla)) goto retry_with_module_loaded; } return found;
Quoted 1 line
which is very space efficient and also still readable.
Out of curiosity, what exactly is the purpose of the goto in the above code? Since we set module_loaded_flag just prior to it, the first if statement must fail after the goto, so we just fall down to where we would have been without the goto.
It all looks a tad pointless to me...
Best wishes from Riley.
---
* Nothing as pretty as a smile, nothing as ugly as a frown.
---
Outgoing mail is certified Virus Free.
Checked by AVG anti-virus system (http://www.grisoft.com).
Version: 6.0.481 / Virus Database: 277 - Release Date: 13-May-2003
Hi Riley,
Quoted 1 line · Riley Williams
Quoted 1 line
int module_loaded_flag=0;
Tell that we don't know, whether the module is loaded
Quoted 3 lines
Quoted 3 lines
retry_with_module_loaded:/* search code */
Do the search.
Quoted 1 line
Quoted 1 line
if (!module_loaded_flag && !found) {
Test, whether we did not yet explicitly load the module and not found the entry either.
Quoted 1 line
Quoted 1 line
module_loaded_flag=1;
Tell that we loaded it (if we cannot load it, then we fall through).
Quoted 2 lines
Quoted 2 lines
if (!request_module(bla)) goto retry_with_module_loaded;
Restart search after successful module load.
Quoted 7 lines
Quoted 2 lines
} return found;Out of curiosity, what exactly is the purpose of the goto in the above code? Since we set
module_loaded_flagjust prior to it, the first if statement must fail after the goto, so we just fall down to where we would have been without the goto.
That is intended. I just reuse the search code here instead of duplicating it.
Since I load the module to broaden my search range, I can also try to load the module there. Without module support this goto will never execute and most of that code there compiled away.
That's why I consider try_then_request_module() not needed. But people seem to have big problems with using gotos and still reading the code (although it's quite common in the kernel), so try_then_request_module() might solve this *social* problem ;-)
Regards
Ingo Oeser
Hi Ingo.
Quoted 6 lines
Quoted 1 line
Quoted 1 line
int module_loaded_flag=0;Tell that we don't know, whether the module is loaded
Quoted 2 lines
Quoted 2 lines
retry_with_module_loaded:
Quoted 8 lines
Quoted 1 line
Quoted 1 line
/* search code */Do the search.
Quoted 1 line
Quoted 1 line
if (!module_loaded_flag && !found) {Test, whether we did not yet explicitly load the module and not found the entry either.
So if either we explicitly loaded the problem or we found it, the test fails and we skip the entire body of that if statement.
Remember, if module_loaded_flag is non-zero then !module_loaded_flag fails. Since you're using && and the link, !found isn't even tested in that case.
Quoted 9 lines
Quoted 1 line
Quoted 1 line
module_loaded_flag=1;Tell that we loaded it (if we cannot load it, then we fall through).
Quoted 2 lines
Quoted 2 lines
if (!request_module(bla)) goto retry_with_module_loaded;Restart search after successful module load.
Doesn't happen - the logic on the if test at the top will always fail and we'll just fall straight back down to just below this line.
Quoted 10 lines
Quoted 7 lines
Quoted 2 lines
} return found;Out of curiosity, what exactly is the purpose of the goto in the above code? Since we set
module_loaded_flagjust prior to it, the first if statement must fail after the goto, so we just fall down to where we would have been without the goto.That is intended. I just reuse the search code here instead of duplicating it.
It doesn't get reused though...
Quoted 3 lines
Since I load the module to broaden my search range, I can also try to load the module there. Without module support this goto will never execute and most of that code there compiled away.
True.
Quoted 4 lines
That's why I consider
try_then_request_module()not needed. But people seem to have big problems with using gotos and still reading the code (although it's quite common in the kernel), sotry_then_request_module()might solve this *social* problem ;-)
Personally, I need a good reason for using goto's, more so than too many of the kernel developers, and your code is a perfect example of the reason why - the goto accomplishes absolutely nothing because of the faulty logic in the if statement - but the faulty logic is actually hidden by the goto in this case.
Best wishes from Riley.
---
* Nothing as pretty as a smile, nothing as ugly as a frown.
---
Outgoing mail is certified Virus Free.
Checked by AVG anti-virus system (http://www.grisoft.com).
Version: 6.0.483 / Virus Database: 279 - Release Date: 19-May-2003
In message <[email protected]> you write:
Quoted 4 lines
So
try_then_request_module()will consolidate the the branch or in the worst case just duplicating that code everywhere (depends on wether you implement it as a non-inline function or define).
I'm not speculating: here it is from kmod.h:
#define try_then_request_module(x, mod...) ((x) ?: request_module(mod), (x))
It really should be:
#ifdef CONFIG_KMOD
#define try_then_request_module(x, mod...) ((x) ?: request_module(mod), (x)) #else
#define try_then_request_module(x, mod...) (x)
#endif /* CONFIG_KMOD */
Patches welcome.
Getting rid of the CONFIG_KMOD's in general code without leaving unused code around is the aim here.
Hope that clarifies!
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.