[PATCH v2 0/2] Replaced printk() with pr_err() and pr_cont() in lustre

[PATCH v2 0/2] Replaced printk() with pr_err() and pr_cont() in lustre

[Tina Johnson]

The following two patches replaces the printk() with pr_err() and pr_cont()
to remove the checkpatch warning:

Prefer [subsystem eg: netdev]_err([subsystem]dev)

Tina Johnson (2):
  staging: lustre: lustre: libcfs: Replaced printk() with pr_err()
  staging: lustre: lustre: libcfs: Replaced printk() with pr_err() and
    pr_cont()

 drivers/staging/lustre/lustre/libcfs/module.c    |    4 ++--
 drivers/staging/lustre/lustre/libcfs/tracefile.c |   19 +++++++++----------
 2 files changed, 11 insertions(+), 12 deletions(-)

-- 
1.7.10.4

[PATCH v2 1/2] staging: lustre: lustre: libcfs: Replaced printk() with pr_err()

[Tina Johnson]

The following checkpatch warning was fixed:

Prefer [subsystem eg: netdev]_err([subsystem]dev

with the help of Coccinelle. The following semantic patch was used:

@a@
expression e;
@@

printk(e,...);

@script:python b@
e << a.e;
y;
@@

import re
match = re.match('KERN_ERR ', e);
if (match == None):
    cocci.include_match(False)

else:
    m = re.sub('KERN_ERR ', '', e)
    coccinelle.y = m;

@c@
expression a.e;
identifier b.y;
@@

- printk(e,
+ pr_err(y,
  ...);

Signed-off-by: Tina Johnson <tinajohnson.1234@gmail.com>
---
 drivers/staging/lustre/lustre/libcfs/module.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lustre/libcfs/module.c b/drivers/staging/lustre/lustre/libcfs/module.c
index e35f598..97597e0 100644
--- a/drivers/staging/lustre/lustre/libcfs/module.c
+++ b/drivers/staging/lustre/lustre/libcfs/module.c
@@ -347,7 +347,7 @@ static int init_libcfs_module(void)
 
 	rc = libcfs_debug_init(5 * 1024 * 1024);
 	if (rc < 0) {
-		pr_err("LustreError: libcfs_debug_init: %d\n", rc);
+		pr_err( "LustreError: libcfs_debug_init: %d\n", rc);
 		return rc;
 	}
 
@@ -433,7 +433,7 @@ static void exit_libcfs_module(void)
 
 	rc = libcfs_debug_cleanup();
 	if (rc)
-		pr_err("LustreError: libcfs_debug_cleanup: %d\n", rc);
+		pr_err( "LustreError: libcfs_debug_cleanup: %d\n", rc);
 
 	libcfs_arch_cleanup();
 }
-- 
1.7.10.4

Re: [Outreachy kernel] [PATCH v2 1/2] staging: lustre: lustre: libcfs: Replaced printk() with pr_err()

[Tina Johnson]

Sorry, my bad. Re-sending the patch right away.

Re: [Outreachy kernel] [PATCH v2 1/2] staging: lustre: lustre: libcfs: Replaced printk() with pr_err()

[Greg KH]

On Mon, Mar 02, 2015 at 10:15:46PM +0530, Tina Johnson wrote:
> The following checkpatch warning was fixed:
> 
> Prefer [subsystem eg: netdev]_err([subsystem]dev
> 
> with the help of Coccinelle. The following semantic patch was used:
> 
> @a@
> expression e;
> @@
> 
> printk(e,...);
> 
> @script:python b@
> e << a.e;
> y;
> @@
> 
> import re
> match = re.match('KERN_ERR ', e);
> if (match == None):
>     cocci.include_match(False)
> 
> else:
>     m = re.sub('KERN_ERR ', '', e)
>     coccinelle.y = m;
> 
> @c@
> expression a.e;
> identifier b.y;
> @@
> 
> - printk(e,
> + pr_err(y,
>   ...);
> 
> Signed-off-by: Tina Johnson <tinajohnson.1234@gmail.com>
> ---
>  drivers/staging/lustre/lustre/libcfs/module.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/libcfs/module.c b/drivers/staging/lustre/lustre/libcfs/module.c
> index e35f598..97597e0 100644
> --- a/drivers/staging/lustre/lustre/libcfs/module.c
> +++ b/drivers/staging/lustre/lustre/libcfs/module.c
> @@ -347,7 +347,7 @@ static int init_libcfs_module(void)
>  
>  	rc = libcfs_debug_init(5 * 1024 * 1024);
>  	if (rc < 0) {
> -		pr_err("LustreError: libcfs_debug_init: %d\n", rc);
> +		pr_err( "LustreError: libcfs_debug_init: %d\n", rc);

Your fix just removed a space, it didn't have anything to do with
changing the function call being made.  So the above changelog doesn't
make much sense :(

thanks,

greg k-h

[PATCH v2 2/2] staging: lustre: lustre: libcfs: Replaced printk() with pr_err() and pr_cont()

[Tina Johnson]

The following checkpatch warning was fixed:

Prefer [subsystem eg: netdev]_err([subsystem]dev

with the help of Coccinelle. pr_cont() was used to replace those printk
statements which followed a printk that did not end with a '\n'. The following
semantic patch was used to replace printk() with pr_err():

@a@
expression e;
@@

printk(e,...);

@script:python b@
e << a.e;
y;
@@

import re
match = re.match('KERN_ERR ', e);
if (match == None):
    cocci.include_match(False)

else:
    m = re.sub('KERN_ERR ', '', e)
    coccinelle.y = m;

@c@
expression a.e;
identifier b.y;
@@

- printk(e,
+ pr_err(y,
  ...);

Signed-off-by: Tina Johnson <tinajohnson.1234@gmail.com>
---
Changes in v2:
  - Replaced those printk statements which followed a printk
    that did not end in '\n' with pr_cont() instead of
    pr_err().

 drivers/staging/lustre/lustre/libcfs/tracefile.c |   26 ++++++++++------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/lustre/lustre/libcfs/tracefile.c b/drivers/staging/lustre/lustre/libcfs/tracefile.c
index eb65b50..f8d8a2f 100644
--- a/drivers/staging/lustre/lustre/libcfs/tracefile.c
+++ b/drivers/staging/lustre/lustre/libcfs/tracefile.c
@@ -224,8 +224,7 @@ static struct cfs_trace_page *cfs_trace_get_tage(struct cfs_trace_cpu_data *tcd,
 	 */
 
 	if (len > PAGE_CACHE_SIZE) {
-		printk(KERN_ERR
-		       "cowardly refusing to write %lu bytes in a page\n", len);
+		pr_err("cowardly refusing to write %lu bytes in a page\n", len);
 		return NULL;
 	}
 
@@ -688,8 +687,8 @@ int cfs_tracefile_dump_all_pages(char *filename)
 	if (IS_ERR(filp)) {
 		rc = PTR_ERR(filp);
 		filp = NULL;
-		printk(KERN_ERR "LustreError: can't open %s for dump: rc %d\n",
-		      filename, rc);
+		pr_err("LustreError: can't open %s for dump: rc %d\n",
+			filename, rc);
 		goto out;
 	}
 
@@ -726,7 +725,7 @@ int cfs_tracefile_dump_all_pages(char *filename)
 	MMSPACE_CLOSE;
 	rc = vfs_fsync(filp, 1);
 	if (rc)
-		printk(KERN_ERR "sync returns %d\n", rc);
+		pr_err("sync returns %d\n", rc);
 close:
 	filp_close(filp, NULL);
 out:
@@ -1048,22 +1047,21 @@ static int tracefiled(void *arg)
 			int i;
 
 			printk(KERN_ALERT "Lustre: trace pages aren't empty\n");
-			printk(KERN_ERR "total cpus(%d): ",
-			       num_possible_cpus());
+			pr_err("total cpus(%d): ",
+				num_possible_cpus());
 			for (i = 0; i < num_possible_cpus(); i++)
 				if (cpu_online(i))
-					printk(KERN_ERR "%d(on) ", i);
+					pr_cont("%d(on) ", i);
 				else
-					printk(KERN_ERR "%d(off) ", i);
-			printk(KERN_ERR "\n");
+					pr_cont("%d(off) ", i);
+			pr_cont("\n");
 
 			i = 0;
 			list_for_each_entry_safe(tage, tmp, &pc.pc_pages,
 						     linkage)
-				printk(KERN_ERR "page %d belongs to cpu %d\n",
-				       ++i, tage->cpu);
-			printk(KERN_ERR "There are %d pages unwritten\n",
-			       i);
+				pr_err("page %d belongs to cpu %d\n",
+					++i, tage->cpu);
+			pr_err("There are %d pages unwritten\n", i);
 		}
 		__LASSERT(list_empty(&pc.pc_pages));
 end_loop:
-- 
1.7.10.4

Re: [Outreachy kernel] [PATCH v2 2/2] staging: lustre: lustre: libcfs: Replaced printk() with pr_err() and pr_cont()

[Greg KH]

On Mon, Mar 02, 2015 at 10:15:47PM +0530, Tina Johnson wrote:
> The following checkpatch warning was fixed:
> 
> Prefer [subsystem eg: netdev]_err([subsystem]dev
> 
> with the help of Coccinelle. pr_cont() was used to replace those printk
> statements which followed a printk that did not end with a '\n'. The following
> semantic patch was used to replace printk() with pr_err():
> 
> @a@
> expression e;
> @@
> 
> printk(e,...);
> 
> @script:python b@
> e << a.e;
> y;
> @@
> 
> import re
> match = re.match('KERN_ERR ', e);
> if (match == None):
>     cocci.include_match(False)
> 
> else:
>     m = re.sub('KERN_ERR ', '', e)
>     coccinelle.y = m;
> 
> @c@
> expression a.e;
> identifier b.y;
> @@
> 
> - printk(e,
> + pr_err(y,
>   ...);
> 
> Signed-off-by: Tina Johnson <tinajohnson.1234@gmail.com>
> ---
> Changes in v2:
>   - Replaced those printk statements which followed a printk
>     that did not end in '\n' with pr_cont() instead of
>     pr_err().
> 
>  drivers/staging/lustre/lustre/libcfs/tracefile.c |   26 ++++++++++------------
>  1 file changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/libcfs/tracefile.c b/drivers/staging/lustre/lustre/libcfs/tracefile.c
> index eb65b50..f8d8a2f 100644
> --- a/drivers/staging/lustre/lustre/libcfs/tracefile.c
> +++ b/drivers/staging/lustre/lustre/libcfs/tracefile.c
> @@ -224,8 +224,7 @@ static struct cfs_trace_page *cfs_trace_get_tage(struct cfs_trace_cpu_data *tcd,
>  	 */
>  
>  	if (len > PAGE_CACHE_SIZE) {
> -		printk(KERN_ERR
> -		       "cowardly refusing to write %lu bytes in a page\n", len);
> +		pr_err("cowardly refusing to write %lu bytes in a page\n", len);
>  		return NULL;
>  	}
>  
> @@ -688,8 +687,8 @@ int cfs_tracefile_dump_all_pages(char *filename)
>  	if (IS_ERR(filp)) {
>  		rc = PTR_ERR(filp);
>  		filp = NULL;
> -		printk(KERN_ERR "LustreError: can't open %s for dump: rc %d\n",
> -		      filename, rc);
> +		pr_err("LustreError: can't open %s for dump: rc %d\n",
> +			filename, rc);
>  		goto out;
>  	}
>  
> @@ -726,7 +725,7 @@ int cfs_tracefile_dump_all_pages(char *filename)
>  	MMSPACE_CLOSE;
>  	rc = vfs_fsync(filp, 1);
>  	if (rc)
> -		printk(KERN_ERR "sync returns %d\n", rc);
> +		pr_err("sync returns %d\n", rc);
>  close:
>  	filp_close(filp, NULL);
>  out:
> @@ -1048,22 +1047,21 @@ static int tracefiled(void *arg)
>  			int i;
>  
>  			printk(KERN_ALERT "Lustre: trace pages aren't empty\n");
> -			printk(KERN_ERR "total cpus(%d): ",
> -			       num_possible_cpus());
> +			pr_err("total cpus(%d): ",
> +				num_possible_cpus());
>  			for (i = 0; i < num_possible_cpus(); i++)
>  				if (cpu_online(i))
> -					printk(KERN_ERR "%d(on) ", i);
> +					pr_cont("%d(on) ", i);
>  				else
> -					printk(KERN_ERR "%d(off) ", i);
> -			printk(KERN_ERR "\n");
> +					pr_cont("%d(off) ", i);
> +			pr_cont("\n");
>  
>  			i = 0;
>  			list_for_each_entry_safe(tage, tmp, &pc.pc_pages,
>  						     linkage)
> -				printk(KERN_ERR "page %d belongs to cpu %d\n",
> -				       ++i, tage->cpu);
> -			printk(KERN_ERR "There are %d pages unwritten\n",
> -			       i);
> +				pr_err("page %d belongs to cpu %d\n",
> +					++i, tage->cpu);
> +			pr_err("There are %d pages unwritten\n", i);
>  		}
>  		__LASSERT(list_empty(&pc.pc_pages));
>  end_loop:

Are you sure you don't have access to a struct device somewhere in these
functions to use dev_err() instead?

thanks,

greg k-h

Re: [Outreachy kernel] [PATCH v2 2/2] staging: lustre: lustre: libcfs: Replaced printk() with pr_err() and pr_cont()

[Tina Johnson]

On Mon, Mar 2, 2015 at 10:31 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> Are you sure you don't have access to a struct device somewhere in these
> functions to use dev_err() instead?


Since lustre is a file system, I think pr_err() should be fine. I had
asked around and most of them said pr_err() will be quite fine.

-- 
Regards,
Tina Johnson,
FOSS@Amrita
A patch a day keep the bugs away!