[PATCH, RFC 1/3] ext4: Convert calls of ext4_error() to EXT4_ERROR_INODE()

[PATCH, RFC 1/3] ext4: Convert calls of ext4_error() to EXT4_ERROR_INODE()

[Theodore Ts'o]

EXT4_ERROR_INODE() tends to provide better error information and in a
more consistent format.

Addresses-Google-Bug: #2507977

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---

Yes, there are more calls to ext4_error that we probably want to convert
to use ext4_error_inode; this is just for illustrative purposes, and
because it's the one which is most obnoxious if there is a bad entry in
a particular inode which is referenced over and over again (see patch 3
in this series).

 fs/ext4/namei.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 0c070fa..0c0cba4 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1072,9 +1072,9 @@ static struct dentry *ext4_lookup(struct inode *dir, struct dentry *dentry, stru
 		inode = ext4_iget(dir->i_sb, ino);
 		if (unlikely(IS_ERR(inode))) {
 			if (PTR_ERR(inode) == -ESTALE) {
-				ext4_error(dir->i_sb,
-						"deleted inode referenced: %u",
-						ino);
+				EXT4_ERROR_INODE(dir,
+						 "deleted inode referenced: %u",
+						 ino);
 				return ERR_PTR(-EIO);
 			} else {
 				return ERR_CAST(inode);
-- 
1.6.6.1.1.g974db.dirty

[PATCH, RFC 2/3] ext4: Add support for adding boolean toggls to ext4's sysfs directory

[Theodore Ts'o]

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/super.c |   37 ++++++++++++++++++++++++++++++++++---
 1 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 3d1a056..1d23b65 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2213,6 +2213,7 @@ struct ext4_attr {
 	ssize_t (*store)(struct ext4_attr *, struct ext4_sb_info *, 
 			 const char *, size_t);
 	int offset;
+	unsigned int mask;
 };
 
 static int parse_strtoul(const char *buf,
@@ -2294,12 +2295,40 @@ static ssize_t sbi_ui_store(struct ext4_attr *a,
 	return count;
 }
 
-#define EXT4_ATTR_OFFSET(_name,_mode,_show,_store,_elname) \
+static ssize_t sbi_bool_show(struct ext4_attr *a,
+			     struct ext4_sb_info *sbi, char *buf)
+{
+	unsigned int *ui = (unsigned int *) (((char *) sbi) + a->offset);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n",
+			((*ui & a->mask) == 0) ? 0 : 1);
+}
+
+static ssize_t sbi_bool_store(struct ext4_attr *a,
+			      struct ext4_sb_info *sbi,
+			      const char *buf, size_t count)
+{
+	unsigned int *ui = (unsigned int *) (((char *) sbi) + a->offset);
+	unsigned long t;
+
+	if (parse_strtoul(buf, 0xffffffff, &t))
+		return -EINVAL;
+	if (t)
+		*ui |= a->mask;
+	else
+		*ui &= ~a->mask;
+
+	return count;
+}
+
+
+#define EXT4_ATTR_OFFSET(_name,_mode,_show,_store,_elname,_mask)\
 static struct ext4_attr ext4_attr_##_name = {			\
 	.attr = {.name = __stringify(_name), .mode = _mode },	\
 	.show	= _show,					\
 	.store	= _store,					\
 	.offset = offsetof(struct ext4_sb_info, _elname),	\
+	.mask   = (_mask),                                      	\
 }
 #define EXT4_ATTR(name, mode, show, store) \
 static struct ext4_attr ext4_attr_##name = __ATTR(name, mode, show, store)
@@ -2307,14 +2336,16 @@ static struct ext4_attr ext4_attr_##name = __ATTR(name, mode, show, store)
 #define EXT4_RO_ATTR(name) EXT4_ATTR(name, 0444, name##_show, NULL)
 #define EXT4_RW_ATTR(name) EXT4_ATTR(name, 0644, name##_show, name##_store)
 #define EXT4_RW_ATTR_SBI_UI(name, elname)	\
-	EXT4_ATTR_OFFSET(name, 0644, sbi_ui_show, sbi_ui_store, elname)
+	EXT4_ATTR_OFFSET(name, 0644, sbi_ui_show, sbi_ui_store, elname, 0)
+#define EXT4_RW_ATTR_SBI_BOOL(name, elname, mask)			\
+EXT4_ATTR_OFFSET(name, 0644, sbi_bool_show, sbi_bool_store, elname, mask)
 #define ATTR_LIST(name) &ext4_attr_##name.attr
 
 EXT4_RO_ATTR(delayed_allocation_blocks);
 EXT4_RO_ATTR(session_write_kbytes);
 EXT4_RO_ATTR(lifetime_write_kbytes);
 EXT4_ATTR_OFFSET(inode_readahead_blks, 0644, sbi_ui_show,
-		 inode_readahead_blks_store, s_inode_readahead_blks);
+		 inode_readahead_blks_store, s_inode_readahead_blks, 0);
 EXT4_RW_ATTR_SBI_UI(inode_goal, s_inode_goal);
 EXT4_RW_ATTR_SBI_UI(mb_stats, s_mb_stats);
 EXT4_RW_ATTR_SBI_UI(mb_max_to_scan, s_mb_max_to_scan);
-- 
1.6.6.1.1.g974db.dirty

[PATCH, RFC 3/3] ext4: Add option for squelching ext4 errors to prevent dmesg from filling up

[Theodore Ts'o]

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(-)

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,
 };
 
-- 
1.6.6.1.1.g974db.dirty

Re: [PATCH, RFC 3/3] ext4: Add option for squelching ext4 errors to prevent dmesg from filling up

[Andreas Dilger]

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.

Re: [PATCH, RFC 3/3] ext4: Add option for squelching ext4 errors to prevent dmesg from filling up

[tytso]

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