mm/zsmalloc: avoid unregister a NOT-registered zsmalloc zpool driver
Subsystems (2)
PATCH 1/3 mm/zsmalloc: avoid unregister a NOT-registered zsmalloc zpool driver
Now zsmalloc can be registered as a zpool driver into zpool when CONFIG_ZPOOL is enabled. During the init of zsmalloc, when error happens, we need to do cleanup. But in current code, it will unregister a not yet registered zsmalloc zpool driver(*zs_zpool_driver*).
This patch puts the cleanup in zs_init() instead of calling zs_exit() where it will unregister a not-registered zpool driver.
--- mm/zsmalloc.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
mm/zsmalloc.c +8 -4
@@ -907,10 +907,8 @@ static int zs_init(void)__register_cpu_notifier (&zs_cpu_nb);for_each_online_cpu (cpu) { ret =zs_cpu_notifier (NULL ,CPU_UP_PREPARE , (void *)(long )cpu); -if (notifier_to_errno (ret)) { -cpu_notifier_register_done (); +if (notifier_to_errno (ret)) goto fail; - } }cpu_notifier_register_done ();
@@ -920,8 +918,14 @@ static int zs_init(void)#endif return 0 ; + fail: -zs_exit (); +for_each_online_cpu (cpu) +zs_cpu_notifier (NULL ,CPU_UP_CANCELED , (void *)(long )cpu); +__unregister_cpu_notifier (&zs_cpu_nb); + +cpu_notifier_register_done (); +return notifier_to_errno (ret); }
Signature · 7 lines
1.7.9.5 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 2/3 mm/zsmalloc: add __init/__exit to zs_init/zs_exit
After patch [1], the zs_exit is only called in module exit. So add __init/__exit to zs_init/zs_exit.
[1] mm/zsmalloc: avoid unregister a NOT-registered zsmalloc zpool driver
--- mm/zsmalloc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
mm/zsmalloc.c +2 -2
@@ -881,7 +881,7 @@ static struct notifier_block zs_cpu_nb = { .notifier_call = zs_cpu_notifier }; -static void zs_exit (void ) +static void __exit zs_exit(void) {int cpu;
@@ -898,7 +898,7 @@ static void zs_exit(void)cpu_notifier_register_done (); } -static int zs_init (void ) +static int __init zs_init (void ) {int cpu, ret;
Signature · 7 lines
1.7.9.5 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 2/3] mm/zsmalloc: add __init/__exit to zs_init/zs_exit
Quoted 6 lines · Mahendran Ganesh
After patch [1], the
zs_exitis only called in module exit. So add __init/__exit to zs_init/zs_exit.[1] mm/zsmalloc: avoid unregister a NOT-registered zsmalloc zpool driver
Signed-off-by Mahendran Ganesh <[email protected]>
makes sense.
-ss
Quoted 29 lines
--- mm/zsmalloc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)mm/zsmalloc.c +2
-2@@ -881,7 +881,7 @@ static struct notifier_block zs_cpu_nb = { .notifier_call = zs_cpu_notifier }; -static void zs_exit (void ) +static void __exit zs_exit(void) {int cpu;@@ -898,7 +898,7 @@ static void zs_exit(void)cpu_notifier_register_done (); } -static int zs_init (void ) +static int __init zs_init (void ) {int cpu, ret;Signature · 1 line
1.7.9.5
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 3/3 mm/zsmalloc: adjust zs_init/zs_exit location
In reply to Mahendran GaneshIn previous code design, the zs_exit() will be called by zs_init(). So function zs_exit() is located before zs_init(). And after patch [1], the zs_exit() will not be called by zs_init().
So we can move the zs_exit() after zs_init() and put these two module init/exit functions to the end of the file which is the common style.
[1] mm/zsmalloc: avoid unregister a NOT-registered zsmalloc zpool driver
--- mm/zsmalloc.c | 96 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 48 insertions(+), 48 deletions(-)
mm/zsmalloc.c +48 -48
@@ -881,54 +881,6 @@ static struct notifier_block zs_cpu_nb = { .notifier_call = zs_cpu_notifier }; -static void __exit zs_exit(void) -{ -int cpu; - -#ifdef CONFIG_ZPOOL -zpool_unregister_driver (&zs_zpool_driver); -#endif - -cpu_notifier_register_begin (); - -for_each_online_cpu (cpu) -zs_cpu_notifier (NULL ,CPU_DEAD , (void *)(long )cpu); -__unregister_cpu_notifier (&zs_cpu_nb); - -cpu_notifier_register_done (); -} - -static int __init zs_init (void ) -{ -int cpu, ret; - -cpu_notifier_register_begin (); - -__register_cpu_notifier (&zs_cpu_nb); -for_each_online_cpu (cpu) { - ret =zs_cpu_notifier (NULL ,CPU_UP_PREPARE , (void *)(long )cpu); -if (notifier_to_errno (ret)) - goto fail; - } - -cpu_notifier_register_done (); - -#ifdef CONFIG_ZPOOL -zpool_register_driver (&zs_zpool_driver); -#endif - -return 0 ; - -fail: -for_each_online_cpu (cpu) -zs_cpu_notifier (NULL ,CPU_UP_CANCELED , (void *)(long )cpu); -__unregister_cpu_notifier (&zs_cpu_nb); - -cpu_notifier_register_done (); - -return notifier_to_errno (ret); -} -/** * zs_create_pool - Creates an allocation pool to work from. * @flags: allocation flags used to allocate pool metadata
@@ -1187,6 +1139,54 @@ unsigned long zs_get_total_pages(struct zs_pool *pool) }EXPORT_SYMBOL_GPL (zs_get_total_pages); +static int __init zs_init (void ) +{ +int cpu, ret; + +cpu_notifier_register_begin (); + +__register_cpu_notifier (&zs_cpu_nb); +for_each_online_cpu (cpu) { + ret =zs_cpu_notifier (NULL ,CPU_UP_PREPARE , (void *)(long )cpu); +if (notifier_to_errno (ret)) + goto fail; + } + +cpu_notifier_register_done (); + +#ifdef CONFIG_ZPOOL +zpool_register_driver (&zs_zpool_driver); +#endif + +return 0 ; + +fail: +for_each_online_cpu (cpu) +zs_cpu_notifier (NULL ,CPU_UP_CANCELED , (void *)(long )cpu); +__unregister_cpu_notifier (&zs_cpu_nb); + +cpu_notifier_register_done (); + +return notifier_to_errno (ret); +} + +static void __exit zs_exit(void) +{ +int cpu; + +#ifdef CONFIG_ZPOOL +zpool_unregister_driver (&zs_zpool_driver); +#endif + +cpu_notifier_register_begin (); + +for_each_online_cpu (cpu) +zs_cpu_notifier (NULL ,CPU_DEAD , (void *)(long )cpu); +__unregister_cpu_notifier (&zs_cpu_nb); + +cpu_notifier_register_done (); +} +module_init (zs_init);module_exit (zs_exit);
Signature · 7 lines
1.7.9.5 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 33 lines · Mahendran Ganesh
Now zsmalloc can be registered as a zpool driver into zpool when
CONFIG_ZPOOLis enabled. During the init of zsmalloc, when error happens, we need to do cleanup. But in current code, it will unregister a not yet registered zsmalloc zpooldriver(*zs_zpool_driver*).This patch puts the cleanup in
zs_init()instead of callingzs_exit()where it will unregister a not-registered zpool driver.Signed-off-by Mahendran Ganesh <[email protected]>--- mm/zsmalloc.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)mm/zsmalloc.c +1
-3@@ -907,10 +907,8 @@ static int zs_init(void)__register_cpu_notifier (&zs_cpu_nb);for_each_online_cpu (cpu) { ret =zs_cpu_notifier (NULL ,CPU_UP_PREPARE , (void *)(long )cpu); -if (notifier_to_errno (ret)) { -cpu_notifier_register_done (); +if (notifier_to_errno (ret)) goto fail; - } }cpu_notifier_register_done ();@@ -920,8 +918,14 @@ static int zs_init(void)#endif return 0 ;
mm/zsmalloc.c static int zs_init(void) · line 929
Quoted 10 lines
mm/zsmalloc.c +7
-1+ fail: -zs_exit (); +for_each_online_cpu (cpu) +zs_cpu_notifier (NULL ,CPU_UP_CANCELED , (void *)(long )cpu); +__unregister_cpu_notifier (&zs_cpu_nb); + +cpu_notifier_register_done (); +return notifier_to_errno (ret);
so we duplicate same code, and there is a bit confusing part
now: zs_cpu_notifier(CPU_UP_CANCELED) and zs_cpu_notifier(CPU_DEAD) calls.
how about something like this?
Factor out zsmalloc cpu notifier unregistration code and call it from both zs_exit() and zs_init() error path.
---
mm/zsmalloc.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
mm/zsmalloc.c +11 -9
@@ -881,14 +881,10 @@ static struct notifier_block zs_cpu_nb = { .notifier_call = zs_cpu_notifier }; -static void zs_exit (void ) +static inline void zs_unregister_cpu_notifier () {int cpu; -#ifdef CONFIG_ZPOOL -zpool_unregister_driver (&zs_zpool_driver); -#endif -cpu_notifier_register_begin ();for_each_online_cpu (cpu)
@@ -898,6 +894,14 @@ static void zs_exit(void)cpu_notifier_register_done (); } +static void zs_exit (void ) +{ +#ifdef CONFIG_ZPOOL +zpool_unregister_driver (&zs_zpool_driver); +#endif +zs_unregister_cpu_notifier (); +} +static int zs_init (void ) {int cpu, ret;
@@ -907,10 +911,8 @@ static int zs_init(void)__register_cpu_notifier (&zs_cpu_nb);for_each_online_cpu (cpu) { ret =zs_cpu_notifier (NULL ,CPU_UP_PREPARE , (void *)(long )cpu); -if (notifier_to_errno (ret)) { -cpu_notifier_register_done (); +if (notifier_to_errno (ret)) goto fail; - } }cpu_notifier_register_done ();
@@ -921,7 +923,7 @@ static int zs_init(void)return 0 ; fail: -zs_exit (); +zs_unregister_cpu_notifier ();return notifier_to_errno (ret); }
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 49 lines · Sergey Senozhatsky
Quoted 33 lines
Now zsmalloc can be registered as a zpool driver into zpool when
CONFIG_ZPOOLis enabled. During the init of zsmalloc, when error happens, we need to do cleanup. But in current code, it will unregister a not yet registered zsmalloc zpooldriver(*zs_zpool_driver*).This patch puts the cleanup in
zs_init()instead of callingzs_exit()where it will unregister a not-registered zpool driver.Signed-off-by Mahendran Ganesh <[email protected]>--- mm/zsmalloc.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)mm/zsmalloc.c +1
-3@@ -907,10 +907,8 @@ static int zs_init(void)__register_cpu_notifier (&zs_cpu_nb);for_each_online_cpu (cpu) { ret =zs_cpu_notifier (NULL ,CPU_UP_PREPARE , (void *)(long )cpu); -if (notifier_to_errno (ret)) { -cpu_notifier_register_done (); +if (notifier_to_errno (ret)) goto fail; - } }cpu_notifier_register_done ();@@ -920,8 +918,14 @@ static int zs_init(void)#endif return 0 ;mm/zsmalloc.c static int zs_init(void) · line 929
Quoted 10 lines
mm/zsmalloc.c +7
-1+ fail: -zs_exit (); +for_each_online_cpu (cpu) +zs_cpu_notifier (NULL ,CPU_UP_CANCELED , (void *)(long )cpu); +__unregister_cpu_notifier (&zs_cpu_nb); + +cpu_notifier_register_done (); +return notifier_to_errno (ret);so we duplicate same code, and there is a bit confusing part
now:zs_cpu_notifier(CPU_UP_CANCELED)andzs_cpu_notifier(CPU_DEAD)calls.
Quoted 1 line
how about something like this?
arghhh... silly me. sorry! this wasn't even compile tested. it should be zs_unregister_cpu_notifier(void). here it is.
Factor out zsmalloc cpu notifier unregistration code and call it from both zs_exit() and zs_init() error path.
---
mm/zsmalloc.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
mm/zsmalloc.c +11 -9
@@ -881,14 +881,10 @@ static struct notifier_block zs_cpu_nb = { .notifier_call = zs_cpu_notifier }; -static void zs_exit (void ) +static void zs_unregister_cpu_notifier (void ) {int cpu; -#ifdef CONFIG_ZPOOL -zpool_unregister_driver (&zs_zpool_driver); -#endif -cpu_notifier_register_begin ();for_each_online_cpu (cpu)
@@ -898,6 +894,14 @@ static void zs_exit(void)cpu_notifier_register_done (); } +static void zs_exit (void ) +{ +#ifdef CONFIG_ZPOOL +zpool_unregister_driver (&zs_zpool_driver); +#endif +zs_unregister_cpu_notifier (); +} +static int zs_init (void ) {int cpu, ret;
@@ -907,10 +911,8 @@ static int zs_init(void)__register_cpu_notifier (&zs_cpu_nb);for_each_online_cpu (cpu) { ret =zs_cpu_notifier (NULL ,CPU_UP_PREPARE , (void *)(long )cpu); -if (notifier_to_errno (ret)) { -cpu_notifier_register_done (); +if (notifier_to_errno (ret)) goto fail; - } }cpu_notifier_register_done ();
@@ -921,7 +923,7 @@ static int zs_init(void)return 0 ; fail: -zs_exit (); +zs_unregister_cpu_notifier ();return notifier_to_errno (ret); }
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 2 lines · Sergey Senozhatsky
Factor out zsmalloc cpu notifier unregistration code and call it from both
zs_exit()andzs_init()error path.
I should had have a good sleep before posting this. shame on me!
v3
---
mm/zsmalloc.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
mm/zsmalloc.c +10 -6
@@ -881,14 +881,10 @@ static struct notifier_block zs_cpu_nb = { .notifier_call = zs_cpu_notifier }; -static void zs_exit (void ) +static void zs_unregister_cpu_notifier (void ) {int cpu; -#ifdef CONFIG_ZPOOL -zpool_unregister_driver (&zs_zpool_driver); -#endif -cpu_notifier_register_begin ();for_each_online_cpu (cpu)
@@ -898,6 +894,14 @@ static void zs_exit(void)cpu_notifier_register_done (); } +static void zs_exit (void ) +{ +#ifdef CONFIG_ZPOOL +zpool_unregister_driver (&zs_zpool_driver); +#endif +zs_unregister_cpu_notifier (); +} +static int zs_init (void ) {int cpu, ret;
@@ -921,7 +925,7 @@ static int zs_init(void)return 0 ; fail: -zs_exit (); +zs_unregister_cpu_notifier ();return notifier_to_errno (ret); }
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Minchan Kim
maintainerHello Sergey,
Quoted 10 lines · Sergey Senozhatsky
Quoted 2 lines · Sergey Senozhatsky
Factor out zsmalloc cpu notifier unregistration code and call it from both
zs_exit()andzs_init()error path.I should had have a good sleep before posting this. shame on me!
v3
Signed-ogg-by Sergey Senozhatsky <[email protected]>
Looks good to me. a nitpick.
Could you factor out cpu register part as well as cpu unregister, too? Then, please resend it with formal description and includes reported-by.
Thanks.
Quoted 56 lines
---
mm/zsmalloc.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)mm/zsmalloc.c +10
-7@@ -881,14 +881,10 @@ static struct notifier_block zs_cpu_nb = { .notifier_call = zs_cpu_notifier }; -static void zs_exit (void ) +static void zs_unregister_cpu_notifier (void ) {int cpu; -#ifdef CONFIG_ZPOOL -zpool_unregister_driver (&zs_zpool_driver); -#endif -cpu_notifier_register_begin ();for_each_online_cpu (cpu)@@ -898,6 +894,14 @@ static void zs_exit(void)cpu_notifier_register_done (); } +static void zs_exit (void ) +{ +#ifdef CONFIG_ZPOOL +zpool_unregister_driver (&zs_zpool_driver); +#endif +zs_unregister_cpu_notifier (); +} +static int zs_init (void ) {int cpu, ret;@@ -921,7 +925,7 @@ static int zs_init(void)return 0 ; fail: -zs_exit (); +zs_unregister_cpu_notifier ();return notifier_to_errno (ret); } --To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Signature · 8 lines
Kind regards, Minchan Kim -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Hello
2014-11-13 23:22 GMT+08:00 Sergey Senozhatsky <[email protected]>:
Quoted 33 lines · Mahendran Ganesh
Quoted 32 lines
Now zsmalloc can be registered as a zpool driver into zpool when
CONFIG_ZPOOLis enabled. During the init of zsmalloc, when error happens, we need to do cleanup. But in current code, it will unregister a not yet registered zsmalloc zpooldriver(*zs_zpool_driver*).This patch puts the cleanup in
zs_init()instead of callingzs_exit()where it will unregister a not-registered zpool driver.Signed-off-by Mahendran Ganesh <[email protected]>--- mm/zsmalloc.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)mm/zsmalloc.c +1
-3@@ -907,10 +907,8 @@ static int zs_init(void)__register_cpu_notifier (&zs_cpu_nb);for_each_online_cpu (cpu) { ret =zs_cpu_notifier (NULL ,CPU_UP_PREPARE , (void *)(long )cpu); -if (notifier_to_errno (ret)) { -cpu_notifier_register_done (); +if (notifier_to_errno (ret)) goto fail; - } }cpu_notifier_register_done ();@@ -920,8 +918,14 @@ static int zs_init(void)#endif
Quoted 15 lines
mm/zsmalloc.c static int zs_init(void) · line 929
Quoted 11 lines
mm/zsmalloc.c +7
-1return 0 ; + fail: -zs_exit (); +for_each_online_cpu (cpu) +zs_cpu_notifier (NULL ,CPU_UP_CANCELED , (void *)(long )cpu); +__unregister_cpu_notifier (&zs_cpu_nb); + +cpu_notifier_register_done (); +return notifier_to_errno (ret);so we duplicate same code, and there is a bit confusing part
now:zs_cpu_notifier(CPU_UP_CANCELED)andzs_cpu_notifier(CPU_DEAD)calls.
Thanks for your review.
CPU_UP_CANCELED and CPU_DEAD have different scenario. CPU_UP_CANCELED is better for fail to complete initialization. While CPU_DEAD is better for exit(such as removing module)
Thanks.
Quoted 61 lines
how about something like this?
Factor out zsmalloc cpu notifier unregistration code and call it from both
zs_exit()andzs_init()error path.Signed-off-by Sergey Senozhatsky <[email protected]>---
mm/zsmalloc.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)mm/zsmalloc.c +10
-8@@ -881,14 +881,10 @@ static struct notifier_block zs_cpu_nb = { .notifier_call = zs_cpu_notifier }; -static void zs_exit (void ) +static inline void zs_unregister_cpu_notifier () {int cpu; -#ifdef CONFIG_ZPOOL -zpool_unregister_driver (&zs_zpool_driver); -#endif -cpu_notifier_register_begin ();for_each_online_cpu (cpu)@@ -898,6 +894,14 @@ static void zs_exit(void)cpu_notifier_register_done (); } +static void zs_exit (void ) +{ +#ifdef CONFIG_ZPOOL +zpool_unregister_driver (&zs_zpool_driver); +#endif +zs_unregister_cpu_notifier (); +} +static int zs_init (void ) {int cpu, ret;@@ -907,10 +911,8 @@ static int zs_init(void)__register_cpu_notifier (&zs_cpu_nb);for_each_online_cpu (cpu) { ret =zs_cpu_notifier (NULL ,CPU_UP_PREPARE , (void *)(long )cpu); -if (notifier_to_errno (ret)) { -cpu_notifier_register_done (); +if (notifier_to_errno (ret)) goto fail; - } }cpu_notifier_register_done ();
mm/zsmalloc.c static int zs_init(void) · line 929
Quoted 8 lines
mm/zsmalloc.c +1
-1@@ -921,7 +923,7 @@ static int zs_init(void)return 0 ; fail: -zs_exit (); +zs_unregister_cpu_notifier ();return notifier_to_errno (ret); }
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>