jfs: logging neatening
cover jfs: logging neatening
This patchset fixes the uses of jfs_info, jfs_warn and jfs_err that have terminating newlines and a couple other trivialities to make the logging a bit more consistent.
There is a difference in use between jfs_error and the other jfs_info, jfs_warn, and jfs_err logging macros. jfs_error is more like the rest of the kernel and requires a newline as the last character of the format.
The jfs_info, jfs_warn, and jfs_err macros add the terminating newline to the format so the uses do not require them.
It is a habituated style for many people to add the terminating newline for many people and this difference in use between the various macro styles causes trivial defects in logging output.
It might be better if the jfs_info, jfs_warn, and jfs_err macros were changed to require a newline termination and the uses changed to include the newline, but that's a larger change.
Joe Perches (3):
jfs: Remove terminating newlines from jfs_info, jfs_warn, jfs_err uses
jfs: Remove unnecessary line continuations and terminating newlines
jfs: Coalesce some formats
fs/jfs/inode.c | 4 ++-- fs/jfs/jfs_discard.c | 6 ++---- fs/jfs/jfs_dtree.c | 10 ++++------ fs/jfs/jfs_imap.c | 3 +-- fs/jfs/jfs_inode.c | 2 +- fs/jfs/jfs_logmgr.c | 14 ++++++-------- fs/jfs/jfs_txnmgr.c | 21 +++++++++------------ fs/jfs/namei.c | 4 ++-- fs/jfs/super.c | 4 ++-- 9 files changed, 29 insertions(+), 39 deletions(-)
Signature · 1 line
2.8.0.rc4.16.g56331f8
PATCH 1/3 jfs: Remove terminating newlines from jfs_info, jfs_warn, jfs_err uses
These macros add the newline so these cause extra blank lines in logging output.
--- fs/jfs/jfs_inode.c | 2 +- fs/jfs/jfs_logmgr.c | 4 ++-- fs/jfs/jfs_txnmgr.c | 4 ++-- fs/jfs/super.c | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-)
fs/jfs/jfs_inode.c +1 -1
@@ -151,7 +151,7 @@ struct inode *ialloc(struct inode *parent, umode_t mode) jfs_inode->xtlid =0 ;jfs_set_inode_flags (inode); -jfs_info ("ialloc returns inode = 0x%p\n" , inode); +jfs_info ("ialloc returns inode = 0x%p" , inode);return inode;
fs/jfs/jfs_logmgr.c +2 -2
@@ -1094,7 +1094,7 @@ int lmLogOpen(struct super_block *sb)if (log->bdev->bd_dev == sbi->logdev) {if (memcmp (log->uuid, sbi->loguuid,sizeof (log->uuid))) { -jfs_warn ("wrong uuid on JFS journal\n" ); +jfs_warn ("wrong uuid on JFS journal" );mutex_unlock (&jfs_log_mutex);return -EINVAL ; }
@@ -2136,7 +2136,7 @@ static void lbmStartIO(struct lbuf * bp)struct bio *bio;struct jfs_log *log = bp->l_log; -jfs_info ("lbmStartIO\n" ); +jfs_info ("lbmStartIO" ); bio =bio_alloc (GFP_NOFS ,1 ); bio->bi_iter.bi_sector = bp->l_blkno << (log->l2bsize -9 );
fs/jfs/jfs_txnmgr.c +2 -2
@@ -1764,7 +1764,7 @@ static void xtLog(struct jfs_log * log, struct tblock * tblk, struct lrd * lrd,if (lwm == next) goto out;if (lwm > next) { -jfs_err ("xtLog: lwm > next\n" ); +jfs_err ("xtLog: lwm > next" ); goto out; } tlck->flag |= tlckUPDATEMAP;
@@ -2814,7 +2814,7 @@ int jfs_lazycommit(void *arg)if (!list_empty (&TxAnchor.unlock_queue))jfs_err ("jfs_lazycommit being killed w/pending transactions!" );else -jfs_info ("jfs_lazycommit being killed\n" ); +jfs_info ("jfs_lazycommit being killed" );return 0 ; }
fs/jfs/super.c +2 -2
@@ -84,7 +84,7 @@ static void jfs_handle_error(struct super_block *sb)panic ("JFS (device %s): panic forced after error\n" , sb->s_id); else if (sbi->flag & JFS_ERR_REMOUNT_RO) { -jfs_err ("ERROR: (device %s): remounting filesystem as read-only\n" , +jfs_err ("ERROR: (device %s): remounting filesystem as read-only" , sb->s_id); sb->s_flags |=MS_RDONLY ; }
@@ -641,7 +641,7 @@ static int jfs_freeze(struct super_block *sb) } rc =updateSuper (sb,FM_CLEAN );if (rc) { -jfs_err ("jfs_freeze: updateSuper failed\n" ); +jfs_err ("jfs_freeze: updateSuper failed" );/* * Don't fail here. Everything succeeded except * marking the superblock clean, so there's really
Signature · 1 line
2.8.0.rc4.16.g56331f8
PATCH 2/3 jfs: Remove unnecessary line continuations and terminating newlines
In reply to Joe PerchesThese jfs_<level> uses need neither a line continuation to assemble the format strings nor newline terminations in the formats.
--- fs/jfs/jfs_discard.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
fs/jfs/jfs_discard.c +2 -4
@@ -49,14 +49,12 @@ void jfs_issue_discard(struct inode *ip, u64 blkno, u64 nblocks) r =sb_issue_discard (sb, blkno, nblocks,GFP_NOFS ,0 );if (unlikely (r !=0 )) { -jfs_err ("JFS: sb_issue_discard" \ -"(%p, %llu, %llu, GFP_NOFS, 0) = %d => failed!\n" , +jfs_err ("JFS: sb_issue_discard(%p, %llu, %llu, GFP_NOFS, 0) = %d => failed!" , sb, (unsigned long long )blkno, (unsigned long long )nblocks, r); } -jfs_info ("JFS: sb_issue_discard" \ -"(%p, %llu, %llu, GFP_NOFS, 0) = %d\n" , +jfs_info ("JFS: sb_issue_discard(%p, %llu, %llu, GFP_NOFS, 0) = %d" , sb, (unsigned long long )blkno, (unsigned long long )nblocks, r);
Signature · 1 line
2.8.0.rc4.16.g56331f8
PATCH 3/3 jfs: Coalesce some formats
In reply to Joe PerchesFormats are better kept as a single line for easier grep.
--- fs/jfs/inode.c | 4 ++-- fs/jfs/jfs_dtree.c | 10 ++++------ fs/jfs/jfs_imap.c | 3 +-- fs/jfs/jfs_logmgr.c | 10 ++++------ fs/jfs/jfs_txnmgr.c | 17 +++++++---------- fs/jfs/namei.c | 4 ++-- 6 files changed, 20 insertions(+), 28 deletions(-)
fs/jfs/inode.c +2 -2
@@ -102,8 +102,8 @@ int jfs_commit_inode(struct inode *inode, int wait)* partitions and may think inode is dirty */ if (!special_file (inode->i_mode) && noisy) { -jfs_err ("jfs_commit_inode(0x%p) called on " -"read-only volume" , inode); +jfs_err ("jfs_commit_inode(0x%p) called on read-only volume" , + inode);jfs_err ("Is remount racy?" ); noisy--; }
fs/jfs/jfs_dtree.c +4 -6
@@ -3072,8 +3072,7 @@ int jfs_readdir(struct file *file, struct dir_context *ctx) }if (dirtab_slot.flag ==DIR_INDEX_FREE ) {if (loop_count++ >JFS_IP (ip)->next_index) { -jfs_err ("jfs_readdir detected " -"infinite loop!" ); +jfs_err ("jfs_readdir detected infinite loop!" ); ctx->pos =DIREND ;return 0 ; }
@@ -3151,8 +3150,7 @@ int jfs_readdir(struct file *file, struct dir_context *ctx)if (!dir_emit (ctx,".." ,2 ,PARENT (ip),DT_DIR ))return 0 ; }else { -jfs_err ("jfs_readdir called with " -"invalid offset!" ); +jfs_err ("jfs_readdir called with invalid offset!" ); } dtoffset->pn =1 ; dtoffset->index =0 ;
@@ -3165,8 +3163,8 @@ int jfs_readdir(struct file *file, struct dir_context *ctx) }if ((rc =dtReadNext (ip, &ctx->pos, &btstack))) { -jfs_err ("jfs_readdir: unexpected rc = %d " -"from dtReadNext" , rc); +jfs_err ("jfs_readdir: unexpected rc = %d from dtReadNext" , + rc); ctx->pos =DIREND ;return 0 ; }
fs/jfs/jfs_imap.c +1 -2
@@ -534,8 +534,7 @@ void diWriteSpecial(struct inode *ip, int secondary)/* read the page of fixed disk inode (AIT) in raw mode */ mp =read_metapage (ip, address << sbi->l2nbperpage,PSIZE ,1 );if (mp ==NULL ) { -jfs_err ("diWriteSpecial: failed to read aggregate inode " -"extent!" ); +jfs_err ("diWriteSpecial: failed to read aggregate inode extent!" );return ; }
fs/jfs/jfs_logmgr.c +4 -6
@@ -1333,9 +1333,8 @@ int lmLogInit(struct jfs_log * log) rc = -EINVAL ; goto errout20; } -jfs_info ("lmLogInit: inline log:0x%p base:0x%Lx " -"size:0x%x" , log, - (unsigned long long ) log->base, log->size); +jfs_info ("lmLogInit: inline log:0x%p base:0x%Lx size:0x%x" , + log, (unsigned long long )log->base, log->size); }else {if (memcmp (logsuper->uuid, log->uuid,16 )) {jfs_warn ("wrong uuid on JFS log device" );
@@ -1343,9 +1342,8 @@ int lmLogInit(struct jfs_log * log) } log->size =le32_to_cpu (logsuper->size); log->l2bsize =le32_to_cpu (logsuper->l2bsize); -jfs_info ("lmLogInit: external log:0x%p base:0x%Lx " -"size:0x%x" , log, - (unsigned long long ) log->base, log->size); +jfs_info ("lmLogInit: external log:0x%p base:0x%Lx size:0x%x" , + log, (unsigned long long )log->base, log->size); } log->page =le32_to_cpu (logsuper->end) /LOGPSIZE ;
fs/jfs/jfs_txnmgr.c +7 -10
@@ -1798,8 +1798,8 @@ static void xtLog(struct jfs_log * log, struct tblock * tblk, struct lrd * lrd, xadlock->xdlist = &p->xad[lwm]; tblk->xflag &= ~COMMIT_LAZY ; } -jfs_info ("xtLog: alloc ip:0x%p mp:0x%p tlck:0x%p lwm:%d " -"count:%d" , tlck->ip, mp, tlck, lwm, xadlock->count); +jfs_info ("xtLog: alloc ip:0x%p mp:0x%p tlck:0x%p lwm:%d count:%d" , + tlck->ip, mp, tlck, lwm, xadlock->count); maplock->index =1 ;
@@ -2025,8 +2025,7 @@ static void xtLog(struct jfs_log * log, struct tblock * tblk, struct lrd * lrd, xadlock->count = next - lwm; xadlock->xdlist = &p->xad[lwm]; -jfs_info ("xtLog: alloc ip:0x%p mp:0x%p count:%d " -"lwm:%d next:%d" , +jfs_info ("xtLog: alloc ip:0x%p mp:0x%p count:%d lwm:%d next:%d" , tlck->ip, mp, xadlock->count, lwm, next); maplock->index++; xadlock++;
@@ -2047,8 +2046,8 @@ static void xtLog(struct jfs_log * log, struct tblock * tblk, struct lrd * lrd, pxdlock->count =1 ; pxdlock->pxd = pxd; -jfs_info ("xtLog: truncate ip:0x%p mp:0x%p count:%d " -"hwm:%d" , ip, mp, pxdlock->count, hwm); +jfs_info ("xtLog: truncate ip:0x%p mp:0x%p count:%d hwm:%d" , + ip, mp, pxdlock->count, hwm); maplock->index++; xadlock++; }
@@ -2066,8 +2065,7 @@ static void xtLog(struct jfs_log * log, struct tblock * tblk, struct lrd * lrd, xadlock->count = hwm - next +1 ; xadlock->xdlist = &p->xad[next]; -jfs_info ("xtLog: free ip:0x%p mp:0x%p count:%d " -"next:%d hwm:%d" , +jfs_info ("xtLog: free ip:0x%p mp:0x%p count:%d next:%d hwm:%d" , tlck->ip, mp, xadlock->count, next, hwm); maplock->index++; }
@@ -2523,8 +2521,7 @@ void txFreeMap(struct inode *ip, xlen =lengthXAD (xad);dbUpdatePMap (ipbmap, true, xaddr, (s64 ) xlen, tblk); -jfs_info ("freePMap: xaddr:0x%lx " -"xlen:%d" , +jfs_info ("freePMap: xaddr:0x%lx xlen:%d" , (ulong ) xaddr, xlen); } }
fs/jfs/namei.c +2 -2
@@ -1225,8 +1225,8 @@ static int jfs_rename(struct inode *old_dir, struct dentry *old_dentry, rc =dtSearch (new_dir, &new_dname, &ino, &btstack,JFS_CREATE );if (rc) { -jfs_err ("jfs_rename didn't expect dtSearch to fail " -"w/rc = %d" , rc); +jfs_err ("jfs_rename didn't expect dtSearch to fail w/rc = %d" , + rc); goto out_tx; }
Signature · 1 line
2.8.0.rc4.16.g56331f8
Quoted 3 lines · Joe Perches
This patchset fixes the uses of
jfs_info,jfs_warnandjfs_errthat have terminating newlines and a couple other trivialities to make the logging a bit more consistent.
These patches look good. I'm pushing them out to the -next build.
Quoted 7 lines
There is a difference in use between
jfs_errorand the otherjfs_info,jfs_warn, andjfs_errlogging macros.jfs_erroris more like the rest of the kernel and requires a newline as the last character of the format.The
jfs_info,jfs_warn, andjfs_errmacros add the terminating newline to the format so the uses do not require them.
I think there's an argument for both ways of doing it. I'm sure I had my reasons for automatically adding the newline back when I implemented those macros. (They probably should be inline functions, but that's another issue.)
Quoted 7 lines
It is a habituated style for many people to add the terminating newline for many people and this difference in use between the various macro styles causes trivial defects in logging output.
It might be better if the
jfs_info,jfs_warn, andjfs_errmacros were changed to require a newline termination and the uses changed to include the newline, but that's a larger change.
Yeah, these patches are the obvious improvement, without changing anything from a design standpoint.
Quoted 17 lines
Joe Perches (3):
jfs: Remove terminating newlines fromjfs_info,jfs_warn,jfs_erruses
jfs: Remove unnecessary line continuations and terminating newlines
jfs: Coalesce some formatsfs/jfs/inode.c | 4 ++-- fs/jfs/jfs_discard.c | 6 ++---- fs/jfs/jfs_dtree.c | 10 ++++------ fs/jfs/jfs_imap.c | 3 +-- fs/jfs/jfs_inode.c | 2 +- fs/jfs/jfs_logmgr.c | 14 ++++++-------- fs/jfs/jfs_txnmgr.c | 21 +++++++++------------ fs/jfs/namei.c | 4 ++-- fs/jfs/super.c | 4 ++-- 9 files changed, 29 insertions(+), 39 deletions(-)
Quoted 13 lines · Dave Kleikamp
Quoted 4 lines · Joe Perches
This patchset fixes the uses of
jfs_info,jfs_warnandjfs_errthat have terminating newlines and a couple other trivialities to make the logging a bit more consistent.These patches look good. I'm pushing them out to the -next build.
Quoted 6 lines
There is a difference in use between
jfs_errorand the otherjfs_info,jfs_warn, andjfs_errlogging macros. jfs_error is more like the rest of the kernel and requires a newline as the last character of the format.
Quoted 6 lines
Quoted 2 lines
The
jfs_info,jfs_warn, andjfs_errmacros add the terminating newline to the format so the uses do not require them.I think there's an argument for both ways of doing it. I'm sure I had my reasons for automatically adding the newline back when I implemented those macros. (They probably should be inline functions, but that's another issue.)
Nah. It was me. I changed jfs_error awhile back to move the newline to the uses.
commit eb8630d7d2fd13589e6a7a3ae2fe1f75f867fbed
Author: Joe Perches <[email protected]>
Date: Tue Jun 4 16:39:15 2013 -0700
jfs: Update jfs_error
Use a more current logging style.
Add __printf format and argument verification.
Remove embedded function names from formats. Add %pf, __builtin_return_address(0) to jfs_error. Add newlines to formats for kernel style consistency. (One format already had an erroneous newline) Coalesce formats and align arguments.
Object size reduced ~1KiB.
$ size fs/jfs/built-in.o* text data bss dec hex filename 201891 35488 63936 301315 49903 fs/jfs/built-in.o.new 202821 35488 64192 302501 49da5 fs/jfs/built-in.o.old
Using inline functions would actually be more code as you'd have to handle the log level and newline via a vprintk of some type. At least the test could be consolidated into the inline though.
Many of the jfs_info calls appear to be function tracing and perhaps could be eliminated altogether.
Quoted 5 lines
It might be better if the
jfs_info,jfs_warn, andjfs_errmacrosQuoted 2 lines
were changed to require a newline termination and the uses changed to include the newline, but that's a larger change.
Yeah, these patches are the obvious improvement, without changing anything from a design standpoint.
Quoted 41 lines · Joe Perches
Quoted 7 lines · Dave Kleikamp
Quoted 6 lines · Joe Perches
There is a difference in use between
jfs_errorand the otherjfs_info,jfs_warn, andjfs_errlogging macros.jfs_erroris more like the rest of the kernel and requires a newline as the last character of the format.Quoted 6 lines
Quoted 2 lines
The
jfs_info,jfs_warn, andjfs_errmacros add the terminating newline to the format so the uses do not require them.I think there's an argument for both ways of doing it. I'm sure I had my reasons for automatically adding the newline back when I implemented those macros. (They probably should be inline functions, but that's another issue.)
Nah. It was me. I changed
jfs_errorawhile back to move the newline to the uses.commit
eb8630d7d2fd13589e6a7a3ae2fe1f75f867fbedAuthor: Joe Perches <[email protected]> Date: Tue Jun 4 16:39:15 2013 -0700jfs: Update
jfs_errorUse a more current logging style.
Add
__printfformat and argument verification.Remove embedded function names from formats. Add %pf,
__builtin_return_address(0)tojfs_error. Add newlines to formats for kernel style consistency. (One format already had an erroneous newline) Coalesce formats and align arguments.Object size reduced ~1KiB.
$ size
fs/jfs/built-in.o*
text data bss dec hex filename
201891 35488 63936 301315 49903fs/jfs/built-in.o.new
202821 35488 64192 302501 49da5fs/jfs/built-in.o.old
Quoted 4 lines
Using inline functions would actually be more code as you'd have to handle the log level and newline via a vprintk of some type. At least the test could be consolidated into the inline though.
Okay.
Quoted 2 lines
Many of the
jfs_infocalls appear to be function tracing and perhaps could be eliminated altogether.
Yeah. They've been in there forever. Should probably have been stripped out before the code was initially merged.