Re: [PATCH] erofs: Use common kernel logging style

2019-08-18 Thread Gao Xiang
Hi Joe,

On Sun, Aug 18, 2019 at 10:47:17PM -0700, Joe Perches wrote:
> On Mon, 2019-08-19 at 13:52 +0800, Gao Xiang wrote:
> > Hi Joe,
> 
> Hello.
> 
> > On Sun, Aug 18, 2019 at 10:28:41PM -0700, Joe Perches wrote:
> > > Rename errln, infoln, and debugln to the typical pr_ uses
> > > to the typical kernel styles of pr_
> > 
> > How about using erofs_err / ... to instead that?
> 
>   I've no opinion.
> It seems most fs/*/* filesystems actually do use pr_
> sed works well if you want that.

Sorry, I mainly refer to ext4, ext2, xfs and f2fs...
I didn't notice the other filesystems, you are right.

Okay, I have no opinion as well (maybe we could turn back later to
introduce sb parameter...)

> 
> >  - I can hardly see directly use pr_ for those filesystems in fs/...
> 
> just fyi:
> 
> There was this one existing pr_ use in erofs

That is just because the following piece of code was taken from fs/mpage.c,
I tend to replace it with iomap in the later version after tail-end packing 
inline is done.

Thanks,
Gao Xiang

> 
> drivers/staging/erofs/data.c:366:   pr_err("%s, 
> readahead error at page %lu of nid %llu\n",
> drivers/staging/erofs/data.c-367-  
> __func__, page->index,
> drivers/staging/erofs/data.c-368-  
> EROFS_V(mapping->host)->nid);
> 
> 
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] erofs: Use common kernel logging style

2019-08-18 Thread Joe Perches
On Mon, 2019-08-19 at 13:52 +0800, Gao Xiang wrote:
> Hi Joe,

Hello.

> On Sun, Aug 18, 2019 at 10:28:41PM -0700, Joe Perches wrote:
> > Rename errln, infoln, and debugln to the typical pr_ uses
> > to the typical kernel styles of pr_
> 
> How about using erofs_err / ... to instead that?

  I've no opinion.
It seems most fs/*/* filesystems actually do use pr_
sed works well if you want that.

>  - I can hardly see directly use pr_ for those filesystems in fs/...

just fyi:

There was this one existing pr_ use in erofs

drivers/staging/erofs/data.c:366:   pr_err("%s, 
readahead error at page %lu of nid %llu\n",
drivers/staging/erofs/data.c-367-  
__func__, page->index,
drivers/staging/erofs/data.c-368-  
EROFS_V(mapping->host)->nid);



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] erofs: Use common kernel logging style

2019-08-18 Thread Joe Perches
Rename errln, infoln, and debugln to the typical pr_ uses
to the typical kernel styles of pr_

Miscellanea:

o Add newline terminations to the uses
o Use "%s: ...", __func__ and not the atypical "%s, ...", __func__
o Trivial grammar changes in output logging
o Delete the now unused macros

Signed-off-by: Joe Perches 
---
 drivers/staging/erofs/data.c |  6 ++--
 drivers/staging/erofs/decompressor.c |  6 ++--
 drivers/staging/erofs/dir.c  |  8 +++---
 drivers/staging/erofs/inode.c| 16 +--
 drivers/staging/erofs/internal.h |  8 ++
 drivers/staging/erofs/namei.c|  4 +--
 drivers/staging/erofs/super.c| 54 +++-
 drivers/staging/erofs/xattr.c|  4 +--
 drivers/staging/erofs/zdata.c| 12 
 drivers/staging/erofs/zdata.h|  2 +-
 drivers/staging/erofs/zmap.c | 26 -
 11 files changed, 73 insertions(+), 73 deletions(-)

diff --git a/drivers/staging/erofs/data.c b/drivers/staging/erofs/data.c
index 4cdb743c8b8d..677d95e8fdeb 100644
--- a/drivers/staging/erofs/data.c
+++ b/drivers/staging/erofs/data.c
@@ -152,8 +152,8 @@ static int erofs_map_blocks_flatmode(struct inode *inode,
 
map->m_flags |= EROFS_MAP_META;
} else {
-   errln("internal error @ nid: %llu (size %llu), m_la 0x%llx",
- vi->nid, inode->i_size, map->m_la);
+   pr_err("internal error @ nid: %llu (size %llu), m_la 0x%llx\n",
+  vi->nid, inode->i_size, map->m_la);
DBG_BUGON(1);
err = -EIO;
goto err_out;
@@ -363,7 +363,7 @@ static int erofs_raw_access_readpages(struct file *filp,
 
/* all the page errors are ignored when readahead */
if (IS_ERR(bio)) {
-   pr_err("%s, readahead error at page %lu of nid 
%llu\n",
+   pr_err("%s: readahead error at page %lu of nid 
%llu\n",
   __func__, page->index,
   EROFS_V(mapping->host)->nid);
 
diff --git a/drivers/staging/erofs/decompressor.c 
b/drivers/staging/erofs/decompressor.c
index 5361a2bbedb6..24d450ce66c1 100644
--- a/drivers/staging/erofs/decompressor.c
+++ b/drivers/staging/erofs/decompressor.c
@@ -166,9 +166,9 @@ static int lz4_decompress(struct z_erofs_decompress_req 
*rq, u8 *out)
  inlen, rq->outputsize,
  rq->outputsize);
if (ret < 0) {
-   errln("%s, failed to decompress, in[%p, %u, %u] out[%p, %u]",
- __func__, src + inputmargin, inlen, inputmargin,
- out, rq->outputsize);
+   pr_err("%s: failed to decompress, in[%p, %u, %u] out[%p, %u]\n",
+  __func__, src + inputmargin, inlen, inputmargin,
+  out, rq->outputsize);
WARN_ON(1);
print_hex_dump(KERN_DEBUG, "[ in]: ", DUMP_PREFIX_OFFSET,
   16, 1, src + inputmargin, inlen, true);
diff --git a/drivers/staging/erofs/dir.c b/drivers/staging/erofs/dir.c
index 2fbfc4935077..526c7b5dd4db 100644
--- a/drivers/staging/erofs/dir.c
+++ b/drivers/staging/erofs/dir.c
@@ -29,8 +29,8 @@ static void debug_one_dentry(unsigned char d_type, const char 
*de_name,
memcpy(dbg_namebuf, de_name, de_namelen);
dbg_namebuf[de_namelen] = '\0';
 
-   debugln("found dirent %s de_len %u d_type %d", dbg_namebuf,
-   de_namelen, d_type);
+   pr_debug("found dirent %s de_len %u d_type %d\n",
+dbg_namebuf, de_namelen, d_type);
 #endif
 }
 
@@ -104,8 +104,8 @@ static int erofs_readdir(struct file *f, struct dir_context 
*ctx)
 
if (unlikely(nameoff < sizeof(struct erofs_dirent) ||
 nameoff >= PAGE_SIZE)) {
-   errln("%s, invalid de[0].nameoff %u",
- __func__, nameoff);
+   pr_err("%s: invalid de[0].nameoff %u\n",
+  __func__, nameoff);
 
err = -EIO;
goto skip_this;
diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
index 286729143365..7b91f3baf8d4 100644
--- a/drivers/staging/erofs/inode.c
+++ b/drivers/staging/erofs/inode.c
@@ -21,8 +21,8 @@ static int read_inode(struct inode *inode, void *data)
vi->datamode = __inode_data_mapping(advise);
 
if (unlikely(vi->datamode >= EROFS_INODE_LAYOUT_MAX)) {
-   errln("unsupported data mapping %u of nid %llu",
- vi->datamode, vi->nid);
+   pr_err("unsupported data mapping %u of nid %llu\n",
+  vi->datamode, vi->nid);
DBG_BUGON(1);
return -EIO;
}
@@ -92,8 +92,8 @@ static int 

Re: [PATCH] erofs: Use common kernel logging style

2019-08-18 Thread Gao Xiang
Hi Joe,

On Sun, Aug 18, 2019 at 10:28:41PM -0700, Joe Perches wrote:
> Rename errln, infoln, and debugln to the typical pr_ uses
> to the typical kernel styles of pr_

How about using erofs_err / ... to instead that?
 - I can hardly see directly use pr_ for those filesystems in fs/...

 - maybe we will introduce super_block to print more information
   about the specific filesystem...

> 
> Miscellanea:
> 
> o Add newline terminations to the uses
> o Use "%s: ...", __func__ and not the atypical "%s, ...", __func__

Agreed.

Thanks,
Gao Xiang

> o Trivial grammar changes in output logging
> o Delete the now unused macros
> 
> Signed-off-by: Joe Perches 
> ---
>  drivers/staging/erofs/data.c |  6 ++--
>  drivers/staging/erofs/decompressor.c |  6 ++--
>  drivers/staging/erofs/dir.c  |  8 +++---
>  drivers/staging/erofs/inode.c| 16 +--
>  drivers/staging/erofs/internal.h |  8 ++
>  drivers/staging/erofs/namei.c|  4 +--
>  drivers/staging/erofs/super.c| 54 
> +++-
>  drivers/staging/erofs/xattr.c|  4 +--
>  drivers/staging/erofs/zdata.c| 12 
>  drivers/staging/erofs/zdata.h|  2 +-
>  drivers/staging/erofs/zmap.c | 26 -
>  11 files changed, 73 insertions(+), 73 deletions(-)
> 
> diff --git a/drivers/staging/erofs/data.c b/drivers/staging/erofs/data.c
> index 4cdb743c8b8d..677d95e8fdeb 100644
> --- a/drivers/staging/erofs/data.c
> +++ b/drivers/staging/erofs/data.c
> @@ -152,8 +152,8 @@ static int erofs_map_blocks_flatmode(struct inode *inode,
>  
>   map->m_flags |= EROFS_MAP_META;
>   } else {
> - errln("internal error @ nid: %llu (size %llu), m_la 0x%llx",
> -   vi->nid, inode->i_size, map->m_la);
> + pr_err("internal error @ nid: %llu (size %llu), m_la 0x%llx\n",
> +vi->nid, inode->i_size, map->m_la);
>   DBG_BUGON(1);
>   err = -EIO;
>   goto err_out;
> @@ -363,7 +363,7 @@ static int erofs_raw_access_readpages(struct file *filp,
>  
>   /* all the page errors are ignored when readahead */
>   if (IS_ERR(bio)) {
> - pr_err("%s, readahead error at page %lu of nid 
> %llu\n",
> + pr_err("%s: readahead error at page %lu of nid 
> %llu\n",
>  __func__, page->index,
>  EROFS_V(mapping->host)->nid);
>  
> diff --git a/drivers/staging/erofs/decompressor.c 
> b/drivers/staging/erofs/decompressor.c
> index 5361a2bbedb6..24d450ce66c1 100644
> --- a/drivers/staging/erofs/decompressor.c
> +++ b/drivers/staging/erofs/decompressor.c
> @@ -166,9 +166,9 @@ static int lz4_decompress(struct z_erofs_decompress_req 
> *rq, u8 *out)
> inlen, rq->outputsize,
> rq->outputsize);
>   if (ret < 0) {
> - errln("%s, failed to decompress, in[%p, %u, %u] out[%p, %u]",
> -   __func__, src + inputmargin, inlen, inputmargin,
> -   out, rq->outputsize);
> + pr_err("%s: failed to decompress, in[%p, %u, %u] out[%p, %u]\n",
> +__func__, src + inputmargin, inlen, inputmargin,
> +out, rq->outputsize);
>   WARN_ON(1);
>   print_hex_dump(KERN_DEBUG, "[ in]: ", DUMP_PREFIX_OFFSET,
>  16, 1, src + inputmargin, inlen, true);
> diff --git a/drivers/staging/erofs/dir.c b/drivers/staging/erofs/dir.c
> index 2fbfc4935077..526c7b5dd4db 100644
> --- a/drivers/staging/erofs/dir.c
> +++ b/drivers/staging/erofs/dir.c
> @@ -29,8 +29,8 @@ static void debug_one_dentry(unsigned char d_type, const 
> char *de_name,
>   memcpy(dbg_namebuf, de_name, de_namelen);
>   dbg_namebuf[de_namelen] = '\0';
>  
> - debugln("found dirent %s de_len %u d_type %d", dbg_namebuf,
> - de_namelen, d_type);
> + pr_debug("found dirent %s de_len %u d_type %d\n",
> +  dbg_namebuf, de_namelen, d_type);
>  #endif
>  }
>  
> @@ -104,8 +104,8 @@ static int erofs_readdir(struct file *f, struct 
> dir_context *ctx)
>  
>   if (unlikely(nameoff < sizeof(struct erofs_dirent) ||
>nameoff >= PAGE_SIZE)) {
> - errln("%s, invalid de[0].nameoff %u",
> -   __func__, nameoff);
> + pr_err("%s: invalid de[0].nameoff %u\n",
> +__func__, nameoff);
>  
>   err = -EIO;
>   goto skip_this;
> diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
> index 286729143365..7b91f3baf8d4 100644
> --- a/drivers/staging/erofs/inode.c
> +++ b/drivers/staging/erofs/inode.c
> @@ -21,8 +21,8 @@ static int read_inode(struct inode *inode, void *data)
>