[PATCH 1/3] mm/zsmalloc: avoid unregister a NOT-registered zsmalloc zpool driver

[PATCH 1/3] mm/zsmalloc: avoid unregister a NOT-registered zsmalloc zpool driver

[Mahendran Ganesh]

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.

Signed-off-by: Mahendran Ganesh <opensource.ganesh@gmail.com>
---
 mm/zsmalloc.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 839a48c..3d2bb36 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -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);
 }
 
-- 
1.7.9.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/3] mm/zsmalloc: add __init/__exit to zs_init/zs_exit

[Mahendran Ganesh]

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

Signed-off-by: Mahendran Ganesh <opensource.ganesh@gmail.com>
---
 mm/zsmalloc.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 3d2bb36..92af030 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -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;
 
-- 
1.7.9.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 3/3] mm/zsmalloc: adjust zs_init/zs_exit location

[Mahendran Ganesh]

In 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

Signed-off-by: Mahendran Ganesh <opensource.ganesh@gmail.com>
---
 mm/zsmalloc.c |   96 ++++++++++++++++++++++++++++-----------------------------
 1 file changed, 48 insertions(+), 48 deletions(-)

diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 92af030..4fcb7c9 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -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);
 
-- 
1.7.9.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3] mm/zsmalloc: avoid unregister a NOT-registered zsmalloc zpool driver

[Sergey Senozhatsky]

On (11/13/14 21:37), Mahendran Ganesh wrote:
> 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.
> 
> Signed-off-by: Mahendran Ganesh <opensource.ganesh@gmail.com>
> ---
>  mm/zsmalloc.c |   12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index 839a48c..3d2bb36 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -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);

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.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

---

 mm/zsmalloc.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index b3b57ef..c4d2d60 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -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 majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3] mm/zsmalloc: avoid unregister a NOT-registered zsmalloc zpool driver

[Sergey Senozhatsky]

On (11/14/14 00:22), Sergey Senozhatsky wrote:
> > 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.
> > 
> > Signed-off-by: Mahendran Ganesh <opensource.ganesh@gmail.com>
> > ---
> >  mm/zsmalloc.c |   12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> > index 839a48c..3d2bb36 100644
> > --- a/mm/zsmalloc.c
> > +++ b/mm/zsmalloc.c
> > @@ -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);
> 
> 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?

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.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

---

 mm/zsmalloc.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index b3b57ef..8dda7c7 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -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 majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3] mm/zsmalloc: avoid unregister a NOT-registered zsmalloc zpool driver

[Sergey Senozhatsky]

On (11/14/14 00:30), Sergey Senozhatsky wrote:
> Factor out zsmalloc cpu notifier unregistration code and call
> it from both zs_exit() and zs_init() error path.

I should had have a good sleep before posting this.
shame on me!

v3

Signed-ogg-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

---

 mm/zsmalloc.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index b3b57ef..cd4efa1 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -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 majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3] mm/zsmalloc: avoid unregister a NOT-registered zsmalloc zpool driver

[Minchan Kim]

Hello Sergey,

On Fri, Nov 14, 2014 at 07:31:27AM +0900, Sergey Senozhatsky wrote:
> On (11/14/14 00:30), Sergey Senozhatsky wrote:
> > Factor out zsmalloc cpu notifier unregistration code and call
> > it from both zs_exit() and zs_init() error path.
> 
> I should had have a good sleep before posting this.
> shame on me!
> 
> v3
> 
> Signed-ogg-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

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.

> 
> ---
> 
>  mm/zsmalloc.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index b3b57ef..cd4efa1 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -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 majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
Kind regards,
Minchan Kim

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3] mm/zsmalloc: avoid unregister a NOT-registered zsmalloc zpool driver

[Ganesh Mahendran]

Hello

2014-11-13 23:22 GMT+08:00 Sergey Senozhatsky <sergey.senozhatsky@gmail.com>:
> On (11/13/14 21:37), Mahendran Ganesh wrote:
>> 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.
>>
>> Signed-off-by: Mahendran Ganesh <opensource.ganesh@gmail.com>
>> ---
>>  mm/zsmalloc.c |   12 ++++++++----
>>  1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
>> index 839a48c..3d2bb36 100644
>> --- a/mm/zsmalloc.c
>> +++ b/mm/zsmalloc.c
>> @@ -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);
>
> 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.

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.

>
> how about something like this?
>
> Factor out zsmalloc cpu notifier unregistration code and call
> it from both zs_exit() and zs_init() error path.
>
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
>
> ---
>
>  mm/zsmalloc.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index b3b57ef..c4d2d60 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -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 majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/3] mm/zsmalloc: add __init/__exit to zs_init/zs_exit

[Sergey Senozhatsky]

On (11/13/14 21:37), Mahendran Ganesh wrote:
> 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
> 
> Signed-off-by: Mahendran Ganesh <opensource.ganesh@gmail.com>

makes sense.

	-ss

> ---
>  mm/zsmalloc.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index 3d2bb36..92af030 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -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;
>  
> -- 
> 1.7.9.5
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>