| Submitter | Theodore Ts'o |
|---|---|
| Date | March 19, 2010, 12:50 a.m. |
| Message ID | <1268959842-4697-3-git-send-email-tytso@mit.edu> |
| Download | mbox | patch |
| Permalink | /patch/48093/ |
| State | Rejected |
| Headers | show |
Comments
On 2010-03-18, at 18:50, Theodore Ts'o wrote: > @@ -363,12 +363,19 @@ void ext4_error_inode(const char *function, > struct inode *inode, > { > va_list args; > > + if (!ext4_test_inode_state(inode, EXT4_STATE_ERR_SQUELCHED) || > + !(EXT4_SB(inode->i_sb)->s_mount_flags & EXT4_MF_FS_SQUELCH)) { > + va_start(args, fmt); > + printk(KERN_CRIT "EXT4-fs error (device %s): %s: " > + "inode #%lu: (comm %s) ", > + inode->i_sb->s_id, function, inode->i_ino, > + current->comm); > + vprintk(fmt, args); > + printk("\n"); > + va_end(args); Shouldn't this really be using ext4_msg()? Cheers, Andreas -- Andreas Dilger Principal Engineer, Lustre Group Oracle Corporation Canada Inc. -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Mar 19, 2010 at 04:56:13PM -0600, Andreas Dilger wrote: > On 2010-03-18, at 18:50, Theodore Ts'o wrote: > >@@ -363,12 +363,19 @@ void ext4_error_inode(const char *function, > >struct inode *inode, > >{ > > va_list args; > > > >+ if (!ext4_test_inode_state(inode, EXT4_STATE_ERR_SQUELCHED) || > >+ !(EXT4_SB(inode->i_sb)->s_mount_flags & EXT4_MF_FS_SQUELCH)) { > >+ va_start(args, fmt); > >+ printk(KERN_CRIT "EXT4-fs error (device %s): %s: " > >+ "inode #%lu: (comm %s) ", > >+ inode->i_sb->s_id, function, inode->i_ino, > >+ current->comm); > >+ vprintk(fmt, args); > >+ printk("\n"); > >+ va_end(args); > > Shouldn't this really be using ext4_msg()? ext4_msg only prints messages of the form: EXT4-fs (<dev>): message ext4_error() and friends only prints messages of the form: EXT4-fs error (<dev>): error message And ext4_warning() and friends only prints messages of the form: EXT4-fs warning (<dev>): error message (at various different priority levels) So no, at the moment, when we are printing something that is intending to be found via syslog parsers as an ext4 file system error, we can't use ext4_msg(). Could we do some more factorization of our various ext4 printk functions? Probably. But the point of this patch wasn't to refactorize the ext4 printk functions.... - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Patch
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index bf938cf..58b4983 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -895,6 +895,7 @@ struct ext4_super_block { */ #define EXT4_MF_MNTDIR_SAMPLED 0x0001 #define EXT4_MF_FS_ABORTED 0x0002 /* Fatal error detected */ +#define EXT4_MF_FS_SQUELCH 0x0004 /* Squelch file system errors */ /* * fourth extended-fs super-block data in memory @@ -1062,6 +1063,7 @@ enum { EXT4_STATE_DA_ALLOC_CLOSE, /* Alloc DA blks on close */ EXT4_STATE_EXT_MIGRATE, /* Inode is migrating */ EXT4_STATE_DIO_UNWRITTEN, /* need convert on dio done*/ + EXT4_STATE_ERR_SQUELCHED, /* squeched error */ }; static inline int ext4_test_inode_state(struct inode *inode, int bit) diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 1d23b65..c28e8bc 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -363,12 +363,19 @@ void ext4_error_inode(const char *function, struct inode *inode, { va_list args; - va_start(args, fmt); - printk(KERN_CRIT "EXT4-fs error (device %s): %s: inode #%lu: (comm %s) ", - inode->i_sb->s_id, function, inode->i_ino, current->comm); - vprintk(fmt, args); - printk("\n"); - va_end(args); + if (!ext4_test_inode_state(inode, EXT4_STATE_ERR_SQUELCHED) || + !(EXT4_SB(inode->i_sb)->s_mount_flags & EXT4_MF_FS_SQUELCH)) { + va_start(args, fmt); + printk(KERN_CRIT "EXT4-fs error (device %s): %s: " + "inode #%lu: (comm %s) ", + inode->i_sb->s_id, function, inode->i_ino, + current->comm); + vprintk(fmt, args); + printk("\n"); + va_end(args); + if (EXT4_SB(inode->i_sb)->s_mount_flags & EXT4_MF_FS_SQUELCH) + ext4_set_inode_state(inode, EXT4_STATE_ERR_SQUELCHED); + } ext4_handle_error(inode->i_sb); } @@ -2354,6 +2361,7 @@ EXT4_RW_ATTR_SBI_UI(mb_order2_req, s_mb_order2_reqs); EXT4_RW_ATTR_SBI_UI(mb_stream_req, s_mb_stream_request); EXT4_RW_ATTR_SBI_UI(mb_group_prealloc, s_mb_group_prealloc); EXT4_RW_ATTR_SBI_UI(max_writeback_mb_bump, s_max_writeback_mb_bump); +EXT4_RW_ATTR_SBI_BOOL(squelch_errors, s_mount_flags, EXT4_MF_FS_SQUELCH); static struct attribute *ext4_attrs[] = { ATTR_LIST(delayed_allocation_blocks), @@ -2368,6 +2376,7 @@ static struct attribute *ext4_attrs[] = { ATTR_LIST(mb_stream_req), ATTR_LIST(mb_group_prealloc), ATTR_LIST(max_writeback_mb_bump), + ATTR_LIST(squelch_errors), NULL, };
Only print one error per inode; this is enough to know that something is wrong with an inode, without filling dmesg by spamming the system with messages over and over again. This is enabled via sysfs option, which is currently off by default. Some environments may want to turn this on by default. Eventually we may want to make this be something which is tunable by a superblock flag, perhaps. Addresses-Google-Bug: #2507977 Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> --- fs/ext4/ext4.h | 2 ++ fs/ext4/super.c | 21 +++++++++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-)