[PATCH 0/3] zswap: Adjustments for three function implementations

[PATCH 0/3] zswap: Adjustments for three function implementations

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 21 May 2017 10:20:03 +0200

Three update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
  Delete an error message for a failed memory allocation in zswap_pool_create()
  Improve a size determination in zswap_frontswap_init()
  Delete an error message for a failed memory allocation in zswap_dstmem_prepare()

 mm/zswap.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

-- 
2.13.0

--
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 1/3] zswap: Delete an error message for a failed memory allocation in zswap_pool_create()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 20 May 2017 22:33:21 +0200

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 mm/zswap.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/mm/zswap.c b/mm/zswap.c
index eedc27894b10..18d8e87119a6 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -518,7 +518,5 @@ static struct zswap_pool *zswap_pool_create(char *type, char *compressor)
-	if (!pool) {
-		pr_err("pool alloc failed\n");
+	if (!pool)
 		return NULL;
-	}
 
 	/* unique name for each pool specifically required by zsmalloc */
 	snprintf(name, 38, "zswap%x", atomic_inc_return(&zswap_pools_count));
-- 
2.13.0

--
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] zswap: Delete an error message for a failed memory allocation in zswap_pool_create()

[Dan Streetman]

On Sun, May 21, 2017 at 4:25 AM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 20 May 2017 22:33:21 +0200
>
> Omit an extra message for a memory allocation failure in this function.
>
> This issue was detected by using the Coccinelle software.
>
> Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Acked-by: Dan Streetman <ddstreet@ieee.org>

> ---
>  mm/zswap.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index eedc27894b10..18d8e87119a6 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -518,7 +518,5 @@ static struct zswap_pool *zswap_pool_create(char *type, char *compressor)
> -       if (!pool) {
> -               pr_err("pool alloc failed\n");
> +       if (!pool)
>                 return NULL;
> -       }
>
>         /* unique name for each pool specifically required by zsmalloc */
>         snprintf(name, 38, "zswap%x", atomic_inc_return(&zswap_pools_count));
> --
> 2.13.0
>
> --
> 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>

--
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] zswap: Improve a size determination in zswap_frontswap_init()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 20 May 2017 22:44:03 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 mm/zswap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/zswap.c b/mm/zswap.c
index 18d8e87119a6..a6e67633be03 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -1156,5 +1156,5 @@ static void zswap_frontswap_init(unsigned type)
 {
 	struct zswap_tree *tree;
 
-	tree = kzalloc(sizeof(struct zswap_tree), GFP_KERNEL);
+	tree = kzalloc(sizeof(*tree), GFP_KERNEL);
 	if (!tree) {
-- 
2.13.0

--
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] zswap: Improve a size determination in zswap_frontswap_init()

[Dan Streetman]

On Sun, May 21, 2017 at 4:26 AM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 20 May 2017 22:44:03 +0200
>
> Replace the specification of a data structure by a pointer dereference
> as the parameter for the operator "sizeof" to make the corresponding size
> determination a bit safer according to the Linux coding style convention.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Acked-by: Dan Streetman <ddstreet@ieee.org>

> ---
>  mm/zswap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 18d8e87119a6..a6e67633be03 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -1156,5 +1156,5 @@ static void zswap_frontswap_init(unsigned type)
>  {
>         struct zswap_tree *tree;
>
> -       tree = kzalloc(sizeof(struct zswap_tree), GFP_KERNEL);
> +       tree = kzalloc(sizeof(*tree), GFP_KERNEL);
>         if (!tree) {
> --
> 2.13.0
>

--
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] zswap: Delete an error message for a failed memory allocation in zswap_dstmem_prepare()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 21 May 2017 09:29:25 +0200

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 mm/zswap.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/mm/zswap.c b/mm/zswap.c
index 3f0a9a1daef4..ed7312291df9 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -374,7 +374,6 @@ static int zswap_dstmem_prepare(unsigned int cpu)
-	if (!dst) {
-		pr_err("can't allocate compressor buffer\n");
+	if (!dst)
 		return -ENOMEM;
-	}
+
 	per_cpu(zswap_dstmem, cpu) = dst;
 	return 0;
 }
-- 
2.13.0

--
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 3/3] zswap: Delete an error message for a failed memory allocation in zswap_dstmem_prepare()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 497 bytes --]


Markus, can you please stop CCing me on every of those patches?

> Omit an extra message for a memory allocation failure in this function.
> 
> This issue was detected by using the Coccinelle software.
> 
> Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

And why do you create a patch for every occasion in the same file? Do
you want to increase your patch count?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: zswap: Delete an error message for a failed memory allocation in zswap_dstmem_prepare()

[SF Markus Elfring]

> Markus, can you please stop CCing me on every of those patches?

Yes, of course.


>> Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf

Did I interpret any information from your presentation slides in an
inappropriate way?


> And why do you create a patch for every occasion in the same file?

This can occasionally happen when I am more unsure about the change acceptance
for a specific place.


> Do you want to increase your patch count?

This can also happen as a side effect if such a source code search pattern
will point hundreds of places out for further software development considerations.
How would you prefer to clarify the remaining update candidates there?

Regards,
Markus

--
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: zswap: Delete an error message for a failed memory allocation in zswap_dstmem_prepare()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 983 bytes --]

Markus,

> > Markus, can you please stop CCing me on every of those patches?
> 
> Yes, of course.

Thanks!

> >> Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
> 
> Did I interpret any information from your presentation slides in an
> inappropriate way?

Have you read my LWN article "Best practices for a big patch series"?

https://lwn.net/Articles/585782/

> > And why do you create a patch for every occasion in the same file?
> 
> This can occasionally happen when I am more unsure about the change acceptance
> for a specific place.

Why were you unsure here?

> This can also happen as a side effect if such a source code search pattern
> will point hundreds of places out for further software development considerations.
> How would you prefer to clarify the remaining update candidates there?

Maybe the article mentioned can provice further guidance?

Have a nice sunday,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Using best practices for big software change possibilities

[SF Markus Elfring]

> Have you read my LWN article "Best practices for a big patch series"?
> 
> https://lwn.net/Articles/585782/

Yes.


>> This can also happen as a side effect if such a source code search pattern
>> will point hundreds of places out for further software development considerations.
>> How would you prefer to clarify the remaining update candidates there?
> 
> Maybe the article mentioned can provice further guidance?

Partly, yes.

I am trying to achieve some software improvements also for special change patterns.
This approach can trigger corresponding communication difficulties.
How do you think about to resolve them by additional means besides mail exchange?

Regards,
Markus

--
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: Using best practices for big software change possibilities

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 300 bytes --]


> How do you think about to resolve them by additional means besides mail exchange?

That can work. E.g. meeting at conferences often solved mail
communication problems.

For now, I still wonder why you were unsure about grouping the changes
into one patch? Maybe there is something to be learned?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Using best practices for big software change possibilities

[SF Markus Elfring]

>> How do you think about to resolve them by additional means besides mail exchange?
> 
> That can work.

I am curious to find out which other communication means could really help here.


> E.g. meeting at conferences often solved mail communication problems.

I find my resources too limited at the moment to attend conferences on site.

How are the chances for further clarification by ordinary telephone calls?


> For now, I still wonder why you were unsure about grouping the changes
> into one patch?

I am varying the patch granularity for affected software areas to some degree.
But I came also places along where I got an impression for higher uncertainty.


> Maybe there is something to be learned?

This is also generally possible.

Would you like to extend the scope for the change pattern around questionable
error messages from a single source file to whole subsystem trees in Linux?

Regards,
Markus

--
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 3/3] zswap: Delete an error message for a failed memory allocation in zswap_dstmem_prepare()

[Dan Streetman]

On Sun, May 21, 2017 at 4:27 AM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 21 May 2017 09:29:25 +0200
>
> Omit an extra message for a memory allocation failure in this function.
>
> This issue was detected by using the Coccinelle software.
>
> Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Acked-by: Dan Streetman <ddstreet@ieee.org>

> ---
>  mm/zswap.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 3f0a9a1daef4..ed7312291df9 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -374,7 +374,6 @@ static int zswap_dstmem_prepare(unsigned int cpu)
> -       if (!dst) {
> -               pr_err("can't allocate compressor buffer\n");
> +       if (!dst)
>                 return -ENOMEM;
> -       }
> +
>         per_cpu(zswap_dstmem, cpu) = dst;
>         return 0;
>  }
> --
> 2.13.0
>

--
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>