Re: [PATCH] erofs: Use common kernel logging style
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
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
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 read_inod
Re: [PATCH] erofs: Use common kernel logging style
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) >