Messages in this thread | | | From | Pravin Shedge <> | Date | Mon, 1 Jan 2018 22:40:36 +0530 | Subject | Re: [PATCH 2/2] lib: Remove module auto-unloading which encourages inconsistent behaviour |
| |
On Fri, Dec 29, 2017 at 10:17 PM, Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Sat, Dec 23, 2017 at 5:28 PM, Pravin Shedge > <pravin.shedge4linux@gmail.com> wrote: >> The module auto-unload might seem like a nice optimization, but >> it encourages inconsistent behaviour. And behaviour that is >> different from all other normal modules. >> >> rbtree_test.c and percpu_test.c returns -EAGAIN from module_init() >> on successful completion. Normal module return 0 on success but >> returning -EAGAIN perform auto module unloading which seem like >> a nice optimization but it brings inconsistent behaviour as well. >> >> I face the similar problem in my previous patch, "Paul Gortmaker" >> gives nice review comment on module auto-unloading brings >> inconsistent behaviour. >> >> Imagine something as simple as this: >> >> for i in $LIST ; do >> modprobe $i >> lsmod | grep -q $i >> if [ $? != 0 ]; then echo Module $i is not present! ; fi >> done >> >> OK, not ideal code, ignoring the modprobe return -- but what it reports >> is true -- >> rbtree_test.c and percpu_test.c test module (if it passed) will NOT be >> present. >> >> All other modules from linux/lib/* follow the same semantics that >> module_init() returns 0 on successful completion & let external entity >> like rmmod do the module unloading task. >> > > Suggested-by: Andrew ... ? Hi Andy,
Please check the reference patch "[PATCH] lib: add module unload support to sort tests" this patch was added for lib/test_sort.c file. test_sort.c missing unloading feature so I added auto-unload feature for the same. But after reviewers comment I am agree on the part that module auto-unload feature will break the consistency from users point of view.
The reviewers for this patch are Andrew, Paul Gortmaker.
Thanks & Regards, PraviN
> >> Signed-off-by: Pravin Shedge <pravin.shedge4linux@gmail.com> >> --- >> lib/percpu_test.c | 3 ++- >> lib/rbtree_test.c | 2 +- >> 2 files changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/lib/percpu_test.c b/lib/percpu_test.c >> index 0b5d14d..57a637b 100644 >> --- a/lib/percpu_test.c >> +++ b/lib/percpu_test.c >> @@ -123,7 +123,8 @@ static int __init percpu_test_init(void) >> preempt_enable(); >> >> pr_info("percpu test done\n"); >> - return -EAGAIN; /* Fail will directly unload the module */ >> + >> + return 0; >> } >> >> static void __exit percpu_test_exit(void) >> diff --git a/lib/rbtree_test.c b/lib/rbtree_test.c >> index 7d36c1e..5c52112 100644 >> --- a/lib/rbtree_test.c >> +++ b/lib/rbtree_test.c >> @@ -394,7 +394,7 @@ static int __init rbtree_test_init(void) >> >> kfree(nodes); >> >> - return -EAGAIN; /* Fail will directly unload the module */ >> + return 0; >> } >> >> static void __exit rbtree_test_exit(void) >> -- >> 2.7.4 >> > > > > -- > With Best Regards, > Andy Shevchenko
| |