Skip to contentSkip to conversation

ext4: Message logging corrections and neatening

PATCH Applied Joe Perches 12 participants 69 messages Mar 16, 2012 – Mar 22, 2012 ext4 file system

lore
Share & tools
lore
#0 lore raw

cover ext4: Message logging corrections and neatening

Joe Perches (9):
ext4: Add -DDEBUG to Makefile
ext4: Use pr_fmt and pr_<level>
ext4: Fix indentation
ext4: Add no_printk argument validation, fix fallout
ext4: Avoid output message interleaving in ext4_error_<foo>
ext4: Remove redundant "EXT4-fs: " from uses of ext4_msg
ext4: Format neatening for easier grep
ext4: Neaten ext4_error uses
ext4: Rename ext4_warning to ext4_warn and ext4_error to ext4_err

 fs/ext4/Makefile         |    2 +
 fs/ext4/balloc.c         |   38 ++--
 fs/ext4/block_validity.c |   11 +-
 fs/ext4/dir.c            |    5 +-
 fs/ext4/ext4.h           |   31 ++--
 fs/ext4/ext4_extents.h   |    4 +-
 fs/ext4/ext4_jbd2.c      |    3 +-
 fs/ext4/extents.c        |  140 +++++++--------
 fs/ext4/file.c           |    3 +-
 fs/ext4/fsync.c          |    2 +-
 fs/ext4/ialloc.c         |   75 ++++----
 fs/ext4/indirect.c       |   12 +-
 fs/ext4/inode.c          |   95 +++++-----
 fs/ext4/mballoc.c        |  163 ++++++++---------
 fs/ext4/mballoc.h        |   18 +-
 fs/ext4/mmp.c            |   39 ++---
 fs/ext4/move_extent.c    |   94 ++++------
 fs/ext4/namei.c          |  140 +++++++-------
 fs/ext4/page-io.c        |   29 ++--
 fs/ext4/resize.c         |  202 ++++++++++-----------
 fs/ext4/super.c          |  465 ++++++++++++++++++++++------------------------
 fs/ext4/xattr.c          |   37 ++--
 22 files changed, 766 insertions(+), 842 deletions(-)
Signature · 1 line
1.7.8.111.gad25c.dirty
#1 lore raw

PATCH 1/9 ext4: Add -DDEBUG to Makefile

Add -DDEBUG to enable future use of pr_debug. No changes to objects as no DEBUG uses currently exist.

---
 fs/ext4/Makefile |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
fs/ext4/Makefile +2 -0
index 56fd8f86..617a5d8 100644
@@ -2,6 +2,8 @@
 # Makefile for the linux ext4-filesystem routines.
 #
 
+ccflags-$(CONFIG_EXT4_FS) := -DDEBUG
+
 obj-$(CONFIG_EXT4_FS) += ext4.o
 
 ext4-y	:= balloc.o bitmap.o dir.o file.o fsync.o ialloc.o inode.o page-io.o \
Signature · 1 line
1.7.8.111.gad25c.dirty
#2 lore raw

Re: [PATCH 1/9] ext4: Add -DDEBUG to Makefile

Quoted 13 lines · Joe Perches

Add -DDEBUG to enable future use of pr_debug. No changes to objects as no DEBUG uses currently exist.

---
  fs/ext4/Makefile |    2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)
fs/ext4/Makefile +0 -0
index 56fd8f86..617a5d8 100644

fs/ext4/Makefile line 5

Quoted 4 lines
fs/ext4/Makefile +1 -0
@@ -2,6 +2,8 @@
  # Makefile for the linux ext4-filesystem routines.
  #
 
+ccflags-$(CONFIG_EXT4_FS) := -DDEBUG

In many other susbsystems/drivers, the definition of DEBUG is gated by a separate Kconfig symbol used to select debugging just for that susbsystem/driver (see CONFIG_MMC_DEBUG for example).

Why aren't you doing the same here?

David Daney

#3 lore raw

Re: [PATCH 1/9] ext4: Add -DDEBUG to Makefile

Quoted 3 lines · David Daney
Quoted 2 lines · Joe Perches

Add -DDEBUG to enable future use of pr_debug. No changes to objects as no DEBUG uses currently exist.

[]

Quoted 1 line

fs/ext4/Makefile

Quoted 1 line
fs/ext4/Makefile +0 -0

[]

Quoted 7 lines
Quoted 1 line

+ccflags-$(CONFIG_EXT4_FS) := -DDEBUG

In many other susbsystems/drivers, the definition of DEBUG is gated by a separate Kconfig symbol used to select debugging just for that susbsystem/driver (see CONFIG_MMC_DEBUG for example).

Why aren't you doing the same here?

It can be done later with an additional symbol if
desired.

For now the current code uses printk(KERN_DEBUG and a straight conversion to pr_debug would eliminate the KERN_DEBUG the output.

pr_debug without #define DEBUG or dynamic_debug is compiled away to nothing.

maintainer
#4 lore raw

Re: [PATCH 1/9] ext4: Add -DDEBUG to Makefile

In reply to David Daney
Quoted 4 lines · David Daney

In many other susbsystems/drivers, the definition of DEBUG is gated by a separate Kconfig symbol used to select debugging just for that susbsystem/driver (see CONFIG_MMC_DEBUG for example).

In ext4 we have many different debugging patches, and in general you'd never want to enable them all at once (you'd get way too much noise). The debugging statements are there when a developer is debugging very specific section of code (i.e., the directory index, or the extent tree, or the block allocator, etc.). It's really only ext4 developers who need to use those debugging statements, and even for them it's quite rare.

So I've never considered it worthwhile to enable them via a CONFIG_* menu item; developers who are debugging a specific problem will generally just drop in the specific #define on an ad hoc basis, and that works fine.

- Ted

#5 lore raw

Re: [PATCH 1/9] ext4: Add -DDEBUG to Makefile

Quoted 4 lines · Ted Ts'o

So I've never considered it worthwhile to enable them via a CONFIG_* menu item; developers who are debugging a specific problem will generally just drop in the specific #define on an ad hoc basis, and that works fine.

CONFIG_EXT4_DEBUG_MASK=value and/or a module_param(debug) is possible to add but as Ted said, it's not really too valuable.

dynamic_debugging the always compiled-in current KERN_DEBUG or eliminating them altogether might have some value though.

#6 lore raw

Re: [PATCH 1/9] ext4: Add -DDEBUG to Makefile

In reply to Ted Ts'o
Quoted 14 lines · Ted Ts'o
Quoted 4 lines · David Daney

In many other susbsystems/drivers, the definition of DEBUG is gated by a separate Kconfig symbol used to select debugging just for that susbsystem/driver (see CONFIG_MMC_DEBUG for example).

In ext4 we have many different debugging patches, and in general you'd never want to enable them all at once (you'd get way too much noise). The debugging statements are there when a developer is debugging very specific section of code (i.e., the directory index, or the extent tree, or the block allocator, etc.). It's really only ext4 developers who need to use those debugging statements, and even for them it's quite rare.

Quoted 5 lines

So I've never considered it worthwhile to enable them via a CONFIG_* menu item; developers who are debugging a specific problem will generally just drop in the specific #define on an ad hoc basis, and that works fine.

If you want to unconditionally define DEBUG for all of ext4, that is fine with me.

I was just noting that it seemed a little odd to me, and different from what is done elsewhere.

Thanks,
David Daney

maintainer
#7 lore raw

Re: [PATCH 1/9] ext4: Add -DDEBUG to Makefile

Quoted 3 lines · David Daney

If you want to unconditionally define DEBUG for all of ext4, that is fine with me.

Since I don't see the value of pr_debug, and don't plan to use it, I don't plan to unconditionally define DEBUG. :-)

- Ted

#8 lore raw

PATCH 2/9 ext4: Use pr_fmt and pr_<level>

In reply to Joe Perches

Use a more current logging style.

Add pr_fmt to consistently prefix with "EXT4-fs: " Convert printks with KERN_<LEVEL> to pr_<level>. Convert bare printks to pr_info and pr_cont where appropriate. Remove embedded function names from formats, use "%s: ", __func__. Coalesce formats.
Neaten macros that contain printks/pr_<level>.
Use ##__VA_ARGS__ in those macros.

---
 fs/ext4/balloc.c         |   13 +++--
 fs/ext4/block_validity.c |   11 +++--
 fs/ext4/dir.c            |    5 +-
 fs/ext4/ext4.h           |   21 ++++----
 fs/ext4/extents.c        |   50 +++++++++-----------
 fs/ext4/ialloc.c         |   40 +++++++--------
 fs/ext4/indirect.c       |    6 ++-
 fs/ext4/inode.c          |   34 +++++++------
 fs/ext4/mballoc.c        |   19 +++----
 fs/ext4/mballoc.h        |   18 ++++----
 fs/ext4/namei.c          |   74 +++++++++++++++--------------
 fs/ext4/page-io.c        |    7 ++-
 fs/ext4/resize.c         |   63 +++++++++++-------------
 fs/ext4/super.c          |  118 +++++++++++++++++++++------------------------
 fs/ext4/xattr.c          |   24 ++++------
 15 files changed, 246 insertions(+), 257 deletions(-)
fs/ext4/balloc.c +7 -6
index 4bbd07a..a5cd777 100644
@@ -11,6 +11,8 @@
  *        David S. Miller ([email protected]), 1995
  */
 
+#define pr_fmt(fmt) "EXT4-fs: " fmt
+
 #include <linux/time.h>
 #include <linux/capability.h>
 #include <linux/fs.h>
@@ -585,15 +587,14 @@ ext4_fsblk_t ext4_count_free_clusters(struct super_block *sb)
 			continue;
 
 		x = ext4_count_free(bitmap_bh, sb->s_blocksize);
-		printk(KERN_DEBUG "group %u: stored = %d, counted = %u\n",
-			i, ext4_free_group_clusters(sb, gdp), x);
+		pr_debug("group %u: stored = %d, counted = %u\n",
+			 i, ext4_free_group_clusters(sb, gdp), x);
 		bitmap_count += x;
 	}
 	brelse(bitmap_bh);
-	printk(KERN_DEBUG "ext4_count_free_clusters: stored = %llu"
-	       ", computed = %llu, %llu\n",
-	       EXT4_B2C(EXT4_SB(sb), ext4_free_blocks_count(es)),
-	       desc_count, bitmap_count);
+	pr_debug("%s: stored = %llu, computed = %llu, %llu\n",
+		 __func__, EXT4_B2C(EXT4_SB(sb), ext4_free_blocks_count(es)),
+		 desc_count, bitmap_count);
 	return bitmap_count;
 #else
 	desc_count = 0;
fs/ext4/block_validity.c +7 -4
index 3f11656..52a13cc 100644
@@ -8,6 +8,8 @@
  * should never be used as data blocks by files or directories.
  */
 
+#define pr_fmt(fmt) "EXT4-fs: " fmt
+
 #include <linux/time.h>
 #include <linux/fs.h>
 #include <linux/namei.h>
@@ -125,16 +127,17 @@ static void debug_print_tree(struct ext4_sb_info *sbi)
 	struct ext4_system_zone *entry;
 	int first = 1;
 
-	printk(KERN_INFO "System zones: ");
+	pr_info("System zones: ");
 	node = rb_first(&sbi->system_blks);
 	while (node) {
 		entry = rb_entry(node, struct ext4_system_zone, node);
-		printk("%s%llu-%llu", first ? "" : ", ",
-		       entry->start_blk, entry->start_blk + entry->count - 1);
+		pr_cont("%s%llu-%llu",
+			first ? "" : ", ",
+			entry->start_blk, entry->start_blk + entry->count - 1);
 		first = 0;
 		node = rb_next(node);
 	}
-	printk("\n");
+	pr_cont("\n");
 }
 
 int ext4_setup_system_zone(struct super_block *sb)
fs/ext4/dir.c +3 -2
index 14aa27c..3107786 100644
@@ -21,6 +21,8 @@
  *
  */
 
+#define pr_fmt(fmt) "EXT4-fs: " fmt
+
 #include <linux/fs.h>
 #include <linux/jbd2.h>
 #include <linux/buffer_head.h>
@@ -522,8 +524,7 @@ static int call_filldir(struct file *filp, void *dirent,
 	sb = inode->i_sb;
 
 	if (!fname) {
-		printk(KERN_ERR "EXT4-fs: call_filldir: called with "
-		       "null fname?!?\n");
+		pr_err("%s: called with null fname?!?\n", __func__);
 		return 0;
 	}
 	curr_pos = hash2pos(filp, fname->hash, fname->minor_hash);
fs/ext4/ext4.h +11 -10
index d0e6632..30d32e2 100644
@@ -46,12 +46,9 @@
  * Debug code
  */
 #ifdef EXT4FS_DEBUG
-#define ext4_debug(f, a...)						\
-	do {								\
-		printk(KERN_DEBUG "EXT4-fs DEBUG (%s, %d): %s:",	\
-			__FILE__, __LINE__, __func__);			\
-		printk(KERN_DEBUG f, ## a);				\
-	} while (0)
+#define ext4_debug(fmt, ...)						\
+	pr_debug("DEBUG (%s, %d): %s:" fmt,				\
+		 __FILE__, __LINE__, __func__, ##__VA_ARGS__)
 #else
 #define ext4_debug(f, a...)	do {} while (0)
 #endif
@@ -440,10 +437,14 @@ enum {
 };
 
 #define TEST_FLAG_VALUE(FLAG) (EXT4_##FLAG##_FL == (1 << EXT4_INODE_##FLAG))
-#define CHECK_FLAG_VALUE(FLAG) if (!TEST_FLAG_VALUE(FLAG)) { \
-	printk(KERN_EMERG "EXT4 flag fail: " #FLAG ": %d %d\n", \
-		EXT4_##FLAG##_FL, EXT4_INODE_##FLAG); BUG_ON(1); }
-
+#define CHECK_FLAG_VALUE(FLAG)						\
+do {									\
+	if (!TEST_FLAG_VALUE(FLAG)) {					\
+		pr_emerg("flag fail: " #FLAG ": %d %d\n",		\
+			 EXT4_##FLAG##_FL, EXT4_INODE_##FLAG);		\
+		BUG_ON(1);						\
+	}								\
+} while (0)
 /*
  * Since it's pretty easy to mix up bit numbers and hex values, and we
  * can't do a compile-time test for ENUM values, we use a run-time
fs/ext4/extents.c +23 -27

98 lines not on this page · raw message

fs/ext4/ialloc.c +19 -21
index b268599..024466a 100644
@@ -12,6 +12,8 @@
  *        David S. Miller ([email protected]), 1995
  */
 
+#define pr_fmt(fmt) "EXT4-fs: " fmt
+
 #include <linux/time.h>
 #include <linux/fs.h>
 #include <linux/jbd2.h>
@@ -205,18 +207,16 @@ void ext4_free_inode(handle_t *handle, struct inode *inode)
 	int fatal = 0, err, count, cleared;
 
 	if (atomic_read(&inode->i_count) > 1) {
-		printk(KERN_ERR "ext4_free_inode: inode has count=%d\n",
-		       atomic_read(&inode->i_count));
+		pr_err("%s: inode has count=%d\n",
+		       __func__, atomic_read(&inode->i_count));
 		return;
 	}
 	if (inode->i_nlink) {
-		printk(KERN_ERR "ext4_free_inode: inode has nlink=%d\n",
-		       inode->i_nlink);
+		pr_err("%s: inode has nlink=%d\n", __func__, inode->i_nlink);
 		return;
 	}
 	if (!sb) {
-		printk(KERN_ERR "ext4_free_inode: inode on "
-		       "nonexistent device\n");
+		pr_err("%s: inode on nonexistent device\n", __func__);
 		return;
 	}
 	sbi = EXT4_SB(sb);
@@ -965,17 +965,15 @@ iget_failed:
 	inode = NULL;
 bad_orphan:
 	ext4_warning(sb, "bad orphan inode %lu!  e2fsck was run?", ino);
-	printk(KERN_NOTICE "ext4_test_bit(bit=%d, block=%llu) = %d\n",
-	       bit, (unsigned long long)bitmap_bh->b_blocknr,
-	       ext4_test_bit(bit, bitmap_bh->b_data));
-	printk(KERN_NOTICE "inode=%p\n", inode);
+	pr_notice("ext4_test_bit(bit=%d, block=%llu) = %d\n",
+		  bit, (unsigned long long)bitmap_bh->b_blocknr,
+		  ext4_test_bit(bit, bitmap_bh->b_data));
+	pr_notice("inode=%p\n", inode);
 	if (inode) {
-		printk(KERN_NOTICE "is_bad_inode(inode)=%d\n",
-		       is_bad_inode(inode));
-		printk(KERN_NOTICE "NEXT_ORPHAN(inode)=%u\n",
-		       NEXT_ORPHAN(inode));
-		printk(KERN_NOTICE "max_ino=%lu\n", max_ino);
-		printk(KERN_NOTICE "i_nlink=%u\n", inode->i_nlink);
+		pr_notice("is_bad_inode(inode)=%d\n", is_bad_inode(inode));
+		pr_notice("NEXT_ORPHAN(inode)=%u\n", NEXT_ORPHAN(inode));
+		pr_notice("max_ino=%lu\n", max_ino);
+		pr_notice("i_nlink=%u\n", inode->i_nlink);
 		/* Avoid freeing blocks if we got a bad deleted inode */
 		if (inode->i_nlink == 0)
 			inode->i_blocks = 0;
@@ -1011,14 +1009,14 @@ unsigned long ext4_count_free_inodes(struct super_block *sb)
 			continue;
 
 		x = ext4_count_free(bitmap_bh, EXT4_INODES_PER_GROUP(sb) / 8);
-		printk(KERN_DEBUG "group %lu: stored = %d, counted = %lu\n",
-			(unsigned long) i, ext4_free_inodes_count(sb, gdp), x);
+		pr_debug("group %lu: stored = %d, counted = %lu\n",
+			 (unsigned long)i, ext4_free_inodes_count(sb, gdp), x);
 		bitmap_count += x;
 	}
 	brelse(bitmap_bh);
-	printk(KERN_DEBUG "ext4_count_free_inodes: "
-	       "stored = %u, computed = %lu, %lu\n",
-	       le32_to_cpu(es->s_free_inodes_count), desc_count, bitmap_count);
+	pr_debug("%s: stored = %u, computed = %lu, %lu\n",
+		 __func__, le32_to_cpu(es->s_free_inodes_count), desc_count,
+		 bitmap_count);
 	return desc_count;
 #else
 	desc_count = 0;
fs/ext4/indirect.c +4 -2
index 830e1b2..bd02261 100644
@@ -20,6 +20,8 @@
  *	([email protected]), 1993, 1998
  */
 
+#define pr_fmt(fmt) "EXT4-fs: " fmt
+
 #include "ext4_jbd2.h"
 #include "truncate.h"
 
@@ -354,8 +356,8 @@ static int ext4_alloc_blocks(handle_t *handle, struct inode *inode,
 			 * for the first direct block
 			 */
 			new_blocks[index] = current_block;
-			printk(KERN_INFO "%s returned more blocks than "
-						"requested\n", __func__);
+			pr_info("%s: returned more blocks than requested\n",
+				__func__);
 			WARN_ON(1);
 			break;
 		}
fs/ext4/inode.c +18 -16
index 5f8081c..c67be80 100644
@@ -18,6 +18,8 @@
  *  Assorted race fixes, rewrite of ext4_get_block() by Al Viro, 2000
  */
 
+#define pr_fmt(fmt) "EXT4-fs: " fmt
+
 #include <linux/fs.h>
 #include <linux/time.h>
 #include <linux/jbd2.h>
@@ -1428,21 +1430,21 @@ static void ext4_da_block_invalidatepages(struct mpage_da_data *mpd)
 static void ext4_print_free_blocks(struct inode *inode)
 {
 	struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
-	printk(KERN_CRIT "Total free blocks count %lld\n",
-	       EXT4_C2B(EXT4_SB(inode->i_sb),
-			ext4_count_free_clusters(inode->i_sb)));
-	printk(KERN_CRIT "Free/Dirty block details\n");
-	printk(KERN_CRIT "free_blocks=%lld\n",
-	       (long long) EXT4_C2B(EXT4_SB(inode->i_sb),
-		percpu_counter_sum(&sbi->s_freeclusters_counter)));
-	printk(KERN_CRIT "dirty_blocks=%lld\n",
-	       (long long) EXT4_C2B(EXT4_SB(inode->i_sb),
-		percpu_counter_sum(&sbi->s_dirtyclusters_counter)));
-	printk(KERN_CRIT "Block reservation details\n");
-	printk(KERN_CRIT "i_reserved_data_blocks=%u\n",
-	       EXT4_I(inode)->i_reserved_data_blocks);
-	printk(KERN_CRIT "i_reserved_meta_blocks=%u\n",
-	       EXT4_I(inode)->i_reserved_meta_blocks);
+	pr_crit("Total free blocks count %lld\n",
+		EXT4_C2B(EXT4_SB(inode->i_sb),
+			 ext4_count_free_clusters(inode->i_sb)));
+	pr_crit("Free/Dirty block details\n");
+	pr_crit("free_blocks=%lld\n",
+		(long long)EXT4_C2B(EXT4_SB(inode->i_sb),
+				    percpu_counter_sum(&sbi->s_freeclusters_counter)));
+	pr_crit("dirty_blocks=%lld\n",
+		(long long)EXT4_C2B(EXT4_SB(inode->i_sb),
+				    percpu_counter_sum(&sbi->s_dirtyclusters_counter)));
+	pr_crit("Block reservation details\n");
+	pr_crit("i_reserved_data_blocks=%u\n",
+		EXT4_I(inode)->i_reserved_data_blocks);
+	pr_crit("i_reserved_meta_blocks=%u\n",
+		EXT4_I(inode)->i_reserved_meta_blocks);
 	return;
 }
 
@@ -2809,7 +2811,7 @@ static void ext4_end_io_buffer_write(struct buffer_head *bh, int uptodate)
 		goto out;
 
 	if (!(io_end->inode->i_sb->s_flags & MS_ACTIVE)) {
-		printk("sb umounted, discard end_io request for inode %lu\n",
+		pr_info("sb umounted, discard end_io request for inode %lu\n",
 			io_end->inode->i_ino);
 		ext4_free_io_end(io_end);
 		goto out;
fs/ext4/mballoc.c +8 -11
index 7e6fab9..c26b600 100644
@@ -21,6 +21,8 @@
  * mballoc.c contains the multiblocks allocation routines
  */
 
+#define pr_fmt(fmt) "EXT4-fs: " fmt
+
 #include "ext4_jbd2.h"
 #include "mballoc.h"
 #include <linux/debugfs.h>
@@ -529,9 +531,8 @@ static inline void mb_cmp_bitmaps(struct ext4_buddy *e4b, void *bitmap)
 #define MB_CHECK_ASSERT(assert)						\
 do {									\
 	if (!(assert)) {						\
-		printk(KERN_EMERG					\
-			"Assertion failure in %s() at %s:%d: \"%s\"\n",	\
-			function, file, line, # assert);		\
+		pr_emerg("Assertion failure in %s() at %s:%d: \"%s\"\n", \
+			 function, file, line, #assert);		\
 		BUG();							\
 	}								\
 } while (0)
@@ -2033,7 +2034,7 @@ repeat:
 			 * Someone more lucky has already allocated it.
 			 * The only thing we can do is just take first
 			 * found block(s)
-			printk(KERN_DEBUG "EXT4-fs: someone won our chunk\n");
+			pr_debug("someone won our chunk\n");
 			 */
 			ac->ac_b_ex.fe_group = 0;
 			ac->ac_b_ex.fe_start = 0;
@@ -2367,8 +2368,7 @@ static int ext4_groupinfo_create_slab(size_t size)
 
 	mutex_unlock(&ext4_grpinfo_slab_create_mutex);
 	if (!cachep) {
-		printk(KERN_EMERG
-		       "EXT4-fs: no memory for groupinfo slab cache\n");
+		pr_emerg("no memory for groupinfo slab cache\n");
 		return -ENOMEM;
 	}
 
@@ -3901,17 +3901,14 @@ static void ext4_mb_show_ac(struct ext4_allocation_context *ac)
 			ext4_get_group_no_and_offset(sb, pa->pa_pstart,
 						     NULL, &start);
 			spin_unlock(&pa->pa_lock);
-			printk(KERN_ERR "PA:%u:%d:%u \n", i,
-			       start, pa->pa_len);
+			pr_err("PA:%u:%d:%u\n", i, start, pa->pa_len);
 		}
 		ext4_unlock_group(sb, i);
 
 		if (grp->bb_free == 0)
 			continue;
-		printk(KERN_ERR "%u: %d/%d \n",
-		       i, grp->bb_free, grp->bb_fragments);
+		pr_err("%u: %d/%d\n", i, grp->bb_free, grp->bb_fragments);
 	}
-	printk(KERN_ERR "\n");
 }
 #else
 static inline void ext4_mb_show_ac(struct ext4_allocation_context *ac)
fs/ext4/mballoc.h +9 -9
index c070618..62bdc95 100644
@@ -39,16 +39,16 @@
 #ifdef CONFIG_EXT4_DEBUG
 extern u8 mb_enable_debug;
 
-#define mb_debug(n, fmt, a...)	                                        \
-	do {								\
-		if ((n) <= mb_enable_debug) {		        	\
-			printk(KERN_DEBUG "(%s, %d): %s: ",		\
-			       __FILE__, __LINE__, __func__);		\
-			printk(fmt, ## a);				\
-		}							\
-	} while (0)
+#define mb_debug(n, fmt, ...)						\
+do {									\
+	if ((n) <= mb_enable_debug)					\
+		pr_debug("(%s, %d): %s: " fmt,				\
+			 __FILE__, __LINE__, __func__, ##__VA_ARGS__);	\
+} while (0)
 #else
-#define mb_debug(n, fmt, a...)
+#define mb_debug(n, fmt, ...)						\
+	no_printk(fmt, ##__VA_ARGS__)
+
 #endif
 
 #define EXT4_MB_HISTORY_ALLOC		1	/* allocation */
fs/ext4/namei.c +39 -35

210 lines not on this page · raw message

fs/ext4/page-io.c +4 -3
index 74cd1f7..09c9d4b 100644
@@ -6,6 +6,8 @@
  * Written by Theodore Ts'o, 2010.
  */
 
+#define pr_fmt(fmt) "EXT4-fs: " fmt
+
 #include <linux/fs.h>
 #include <linux/time.h>
 #include <linux/jbd2.h>
@@ -186,9 +188,8 @@ ext4_io_end_t *ext4_init_io_end(struct inode *inode, gfp_t flags)
 static void buffer_io_error(struct buffer_head *bh)
 {
 	char b[BDEVNAME_SIZE];
-	printk(KERN_ERR "Buffer I/O error on device %s, logical block %llu\n",
-			bdevname(bh->b_bdev, b),
-			(unsigned long long)bh->b_blocknr);
+	pr_err("Buffer I/O error on device %s, logical block %llu\n",
+	       bdevname(bh->b_bdev, b), (unsigned long long)bh->b_blocknr);
 }
 
 static void ext4_end_bio(struct bio *bio, int error)
fs/ext4/resize.c +29 -34

129 lines not on this page · raw message

fs/ext4/super.c +55 -63

248 lines not on this page · raw message

fs/ext4/xattr.c +10 -14
index 3369157..a8674d2 100644
@@ -67,20 +67,16 @@
 #define IS_LAST_ENTRY(entry) (*(__u32 *)(entry) == 0)
 
 #ifdef EXT4_XATTR_DEBUG
-# define ea_idebug(inode, f...) do { \
-		printk(KERN_DEBUG "inode %s:%lu: ", \
-			inode->i_sb->s_id, inode->i_ino); \
-		printk(f); \
-		printk("\n"); \
-	} while (0)
-# define ea_bdebug(bh, f...) do { \
-		char b[BDEVNAME_SIZE]; \
-		printk(KERN_DEBUG "block %s:%lu: ", \
-			bdevname(bh->b_bdev, b), \
-			(unsigned long) bh->b_blocknr); \
-		printk(f); \
-		printk("\n"); \
-	} while (0)
+# define ea_idebug(inode, fmt, ...)					\
+	pr_debug("inode %s:%lu: " fmt "\n",				\
+		 inode->i_sb->s_id, inode->i_ino, ##__VA_ARGS__)
+# define ea_bdebug(bh, fmt, ...)
+do {									\
+	char b[BDEVNAME_SIZE];						\
+	pr_debug("block %s:%lu: " fmt "\n",				\
+		 bdevname(bh->b_bdev, b),				\
+		 (unsigned long)bh->b_blocknr, ##__VA_ARGS__);		\
+} while (0)
 #else
 # define ea_idebug(f...)
 # define ea_bdebug(f...)
Signature · 1 line
1.7.8.111.gad25c.dirty
maintainer
#9 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 9 lines · Joe Perches

Use a more current logging style.

Add pr_fmt to consistently prefix with "EXT4-fs: " Convert printks with KERN_<LEVEL> to pr_<level>. Convert bare printks to pr_info and pr_cont where appropriate. Remove embedded function names from formats, use "%s: ", __func__. Coalesce formats.
Neaten macros that contain printks/pr_<level>.
Use ##__VA_ARGS__ in those macros.

Still too much in a single patch.

Changing printk's to pr_info and pr_cont is patch noise as far as I'm concerned. Adds no value, and just breaks other patches. Ditto using __func__ and and coalesing formats (much of which was broekn up back when checkpatch flamed developers about 80 column lines, and I patch noises from trivial patch monkeys offering to fix it, sigh).

And I really don't care about adding EXT4-fs to debugging messages, especially messages which can't be enabled except when developers have to explicitly add #define's...

One evidence that this patch is noise is that it doesn't apply cleanly just on top of my current patch set that I plan to send to Linus. And I consider it a waste of time to sort through and figure out what if any of this 1200 line patch is really worthwhile. (A quick scan didn't find anything, although if there are some places where I'm missing a pr_cont / KERN_CONT in a commonly used --- non debug --- codepath I'd be willing to fix it.)

- Ted

#10 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

From: "Ted Ts'o" <[email protected]>
Date: Mon, 19 Mar 2012 00:09:50 -0400

Quoted 2 lines

Changing printk's to pr_info and pr_cont is patch noise as far as I'm concerned.

Ted, get off your high horse.

I apply patches like Joe's all the time to the networking, consistency has high value whether you choose to believe it or not. And no we don't have huge problems with patch conflicts and stuff like that either, it simply isn't a real issue so don't use that as an excuse either.

maintainer
#11 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 7 lines · David Miller

I apply patches like Joe's all the time to the networking, consistency has high value whether you choose to believe it or not. And no we don't have huge problems with patch conflicts and stuff like that either, it simply isn't a real issue so don't use that as an excuse either.

It is for me.

Things like printk(KERN_INFO, ...) are quite well established, and changing really doesn't add any value. Sorry, I just don't buy the consistency argument. I'd much rather keep everything consistent within ext3 and ext4.

If you want to apply those sorts of patches, feel free. I choose not to, and I actively discourage kernel newbies from running checkpatch.pl --file and trying to fix random things. (A few years ago everyone agreed that that was a waste of time.)

- Ted

#12 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

From: "Ted Ts'o" <[email protected]>
Date: Mon, 19 Mar 2012 00:34:20 -0400

Quoted 2 lines

Things like printk(KERN_INFO, ...) are quite well established, and changing really doesn't add any value.

It also allows you to modify pr_fmt at the top of the file or elsewhere and have it influence the output, as just one example of how it's better.

As a maintainer you can only take one of two positions:

1) Accept all properly written patches that convert code over to use the new and recommended way of doing things, such as pr_info() et al.

2) Advocate on linux-kernel to remove these new interfaces or improve them if you don't find them suitable, and then once suitable go back to #1.

Just keeping things the way they are because you simply feel like it is really not acceptable Ted.

maintainer
#13 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 10 lines · David Miller

It also allows you to modify pr_fmt at the top of the file or elsewhere and have it influence the output, as just one example of how it's better.

As a maintainer you can only take one of two positions:

1) Accept all properly written patches that convert code over to use the new and recommended way of doing things, such as pr_info() et al.

Recommended by *who*?!? Who dictated that this is the best way to go?

And in the past we've always allowed for local variations of style; it's never been a strict style nazism. Linus has always given maintainers latitude and has frowned on patch churn for churn's sake.

For goodness sake we still have get_fs() and set_fs() in the kernel code even though we haven't used the actual fs segment register in over a decade.

And we're arguing over "there must be only one true logging interface?!?". This isn't Highlander.

- Ted

#14 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Ted Ts'o: this effort is like BKL remove, ok, not like that. In 5 years, to have an standard. Mmm, maybe, less.

Joe, prease remind to all one more time your goal, and the benefits.

#15 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 2 lines · Anca Emanuel

Ted Ts'o: this effort is like BKL remove, ok, not like that. In 5 years, to have an standard. Mmm, maybe, less.

Probably about right, though I hope less than 5.
This stuff is tedious to get accepted and there
seems to be a maximum possible conversion rate.

Quoted 1 line

Joe, prease remind to all one more time your goal, and the benefits.

o standardize logging with subsystem/module id
o standardize debugging style (mask/level)
o standardize logging style to reduce future defects
o reduce current logging defects
o reduce code size (singleton for pr_fmt, deduplicate formats, %pV)
o remove most all #define pr_fmt

maintainer
#16 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

In reply to Anca Emanuel
Quoted 4 lines · Anca Emanuel

Ted Ts'o: this effort is like BKL remove, ok, not like that. In 5 years, to have an standard. Mmm, maybe, less.

Joe, prease remind to all one more time your goal, and the benefits.

Getting rid of the BKL had real, user-visible benefits. Invoking a war on printk has no real-world benefits.

It's about the same utility as fixing "colour" to be "color" because people think standardized spelling is somehow critically important. Guess what? The world will keep spinning and users will not notice the difference if we use printk vs. pr_info

- Ted

#17 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

From: "Ted Ts'o" <[email protected]>
Date: Mon, 19 Mar 2012 12:14:25 -0400

Quoted 3 lines

It's about the same utility as fixing "colour" to be "color" because people think standardized spelling is somehow critically important. Guess what?

Not the same, since moving to pr_*() allows us to standardize on kernel log output formatting, amongst other things, so it has a real practical impact unlike the spelling fix.

Ted, just fold the tent and raise the white flag, you're wrong on this one.

maintainer
#18 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 4 lines · David Miller

Not the same, since moving to pr_*() allows us to standardize on kernel log output formatting, amongst other things, so it has a real practical impact unlike the spelling fix.

I've *already* gone far beyond the pr_fmt standardization, with the ext4_msg() and ext4_error() system --- not only does it standardize the prefix, it also standardizes information about the file system, block numbers, inode numbers, so that it's easier for dmesg scrapers to collate that information and use it for cluster-wide monitoring --- which is happening *today* using printk() on far more machines than probably anywhere else in the world.

One of the other reasons why I don't like the pr_* system because it doesn't go far enough for the pain that's involved with making such a change pervasively across the entire kernel. What I'd really like to see is a system that allows for semi-structured logging --- where structured data such as the block device involved, whether it's at a device driver driver, block layer, cfq or proportional I/O layer, or file system layer, was consistently named, and squirted out a high efficiency interface such as netlink or an ftrace ring buffer.

*That* would allow userspace to correlate errors from multiple kernel subsystems for a particular device, and use a much more efficient and less error-prone system than screen scraping dmesg output. But the proposed pr_* system is totally inadequate for this purpose, and if all you're going to be able to do is make it slightly easier standardize the prefix, it's just not interesting as far as I'm concerned.

- Ted

#19 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 3 lines · Ted Ts'o

On Mon, Mar 19, 2012 at 02:14:02PM -0400, David Miller wrote:
I've *already* gone far beyond the pr_fmt standardization, with the ext4_msg() and ext4_error() system

Please note the defects that were recently corrected there which occurred because of a lack of standardization both in prefix and termination style.

Quoted 3 lines

One of the other reasons why I don't like the pr_* system because it doesn't go far enough for the pain that's involved with making such a change pervasively across the entire kernel.

Any logging system, with or without an external notification mechanism, will be painful. pr_<foo> is at least a small start. I'd like to see a notification mechanism, perhaps ala netlink/ethtool to extend pr_<foo> or another call. A lot of these printk/pr_<level> uses really could generate notifications.

Quoted 6 lines

What I'd really like to
see is a system that allows for semi-structured logging --- where structured data such as the block device involved, whether it's at a device driver driver, block layer, cfq or proportional I/O layer, or file system layer, was consistently named, and squirted out a high efficiency interface such as netlink or an ftrace ring buffer.

Yay, we agree.

cheers, Joe

maintainer
#20 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 8 lines · Joe Perches
Quoted 3 lines · Ted Ts'o

On Mon, Mar 19, 2012 at 02:14:02PM -0400, David Miller wrote:
I've *already* gone far beyond the pr_fmt standardization, with the ext4_msg() and ext4_error() system

Please note the defects that were recently corrected there which occurred because of a lack of standardization both in prefix and termination style.

For debugging printk's that are #ifdef'ed for anyone other than ext4 developers and can't be enabled via CONFIG_*. Yawn.

Quoted 5 lines

Any logging system, with or without an external notification mechanism, will be painful. pr_<foo> is at least a small start. I'd like to see a notification mechanism, perhaps ala netlink/ethtool to extend pr_<foo> or another call. A lot of these printk/pr_<level> uses really could generate notifications.

Yes, but we can't do structured notifications with the current pr_<foo>. So why change literally tens of thousands of callsites when in order to really realize the full promise of structured notifications, we'll have to change them *again*?

- Ted

#21 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 6 lines · Ted Ts'o
Quoted 5 lines · Joe Perches
Quoted 3 lines · Ted Ts'o

On Mon, Mar 19, 2012 at 02:14:02PM -0400, David Miller wrote:
I've *already* gone far beyond the pr_fmt standardization, with the ext4_msg() and ext4_error() system

Quoted 6 lines
Quoted 3 lines

Please note the defects that were recently corrected there which occurred because of a lack of standardization both in prefix and termination style.

For debugging printk's that are #ifdef'ed for anyone other than ext4 developers and can't be enabled via CONFIG_*. Yawn.

Really? Huh. Are these messages debugging only?
Not really.

$ grep "ext4_msg.*EXT4-fs: " fs/ext4/*.c
fs/ext4/mballoc.c:			ext4_msg(sb, KERN_ERR, "EXT4-fs: can't allocate mem "
fs/ext4/mballoc.c:		ext4_msg(sb, KERN_ERR, "EXT4-fs: can't allocate buddy mem");
fs/ext4/mballoc.c:	ext4_msg(ac->ac_sb, KERN_ERR, "EXT4-fs: Can't allocate:"
fs/ext4/mballoc.c:	ext4_msg(ac->ac_sb, KERN_ERR, "EXT4-fs: status %d flags %d",
fs/ext4/mballoc.c:	ext4_msg(ac->ac_sb, KERN_ERR, "EXT4-fs: orig %lu/%lu/%lu@%lu, "
fs/ext4/mballoc.c:	ext4_msg(ac->ac_sb, KERN_ERR, "EXT4-fs: %lu scanned, %d found",
fs/ext4/mballoc.c:	ext4_msg(ac->ac_sb, KERN_ERR, "EXT4-fs: groups: ");

They _were_ doubly prefixed.

from ext4#dev

commit 2504a4a9c0c096e11bcc24691b85bf6d942df9fe

Author: Joe Perches <[email protected]>
Date:   Mon Mar 19 00:12:00 2012 -0400

ext4: remove redundant "EXT4-fs: " from uses of ext4_msg

ext4_msg adds "EXT4-fs: " to the messsage output. Remove the redundant bits from uses.

Quoted 10 lines
Quoted 5 lines

Any logging system, with or without an external notification mechanism, will be painful. pr_<foo> is at least a small start. I'd like to see a notification mechanism, perhaps ala netlink/ethtool to extend pr_<foo> or another call. A lot of these printk/pr_<level> uses really could generate notifications.

Yes, but we can't do structured notifications with the current pr_<foo>. So why change literally tens of thousands of callsites when in order to really realize the full promise of structured notifications, we'll have to change them *again*?

So that they are consistent and extensible and can use something like pr_<level>_notify, just like pr_<level>_once and pr_<level>_ratelimited, or some other similar form.

maintainer
#22 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 8 lines · Joe Perches

They _were_ doubly prefixed.

from ext4#dev

commit 2504a4a9c0c096e11bcc24691b85bf6d942df9fe

Author: Joe Perches <[email protected]>
Date:   Mon Mar 19 00:12:00 2012 -0400
Quoted 7 lines

ext4: remove redundant "EXT4-fs: " from uses of ext4_msg

ext4_msg adds "EXT4-fs: " to the messsage output. Remove the redundant bits from uses.

Yes, and I accepted that patch. I was referring to your complaints of printk's such as this:

#ifdef EXT4FS_DEBUG
			WARN_ON(ret <= 0);
			printk(KERN_ERR "%s: ext4_ext_map_blocks "
				    "returned error inode#%lu, block=%u, "
				    "max_blocks=%u", __func__,
				    inode->i_ino, map.m_lblk, max_blocks);

#endif

... and in that case, even if I was going to fix it, I wouldn't be fixing it via pr_err(). I'd be fixing using ext4_msg(). Why? Because then the message would include the block device and not just the EXT4-fs prefix.

Quoted 8 lines
Quoted 4 lines

Yes, but we can't do structured notifications with the current pr_<foo>. So why change literally tens of thousands of callsites when in order to really realize the full promise of structured notifications, we'll have to change them *again*?

So that they are consistent and extensible and can use something like pr_<level>_notify, just like pr_<level>_once and pr_<level>_ratelimited, or some other similar form.

Changing to pr_err() is pointless, because it doesn't do anything functional. You *have* to have an interface like ext4_msg(sb, ...) if you're going to send a semi-structured notification, or include relevant information about which ext4 file system was responsible for issuing the warning.

If you're going to change huge numbers of lines of code, you might as well do it in a way that significantly improves things. The change to pr_foo() is just syntactic sugar, and that's a whitespace-level change in my book. Adding a struct super * or or a struct block device *, which gets passed to the notification functions? That's ***far*** more interesting.

- Ted

#23 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 10 lines · Ted Ts'o
Quoted 7 lines · Joe Perches

They _were_ doubly prefixed.
from ext4#dev
commit 2504a4a9c0c096e11bcc24691b85bf6d942df9fe

Author: Joe Perches <[email protected]>
Date:   Mon Mar 19 00:12:00 2012 -0400

ext4: remove redundant "EXT4-fs: " from uses of ext4_msg ext4_msg adds "EXT4-fs: " to the messsage output.

Yes, and I accepted that patch. I was referring to your complaints of printk's such as this:

Quoted 7 lines
#ifdef EXT4FS_DEBUG
			WARN_ON(ret <= 0);
			printk(KERN_ERR "%s: ext4_ext_map_blocks "
				    "returned error inode#%lu, block=%u, "
				    "max_blocks=%u", __func__,
				    inode->i_ino, map.m_lblk, max_blocks);

#endif

and I was not. Doubly prefixed was as I said, doubly prefixed. ext4_msg wasn't a consistently used interface.

Quoted 5 lines

Changing to pr_err() is pointless, because it doesn't do anything functional. You *have* to have an interface like ext4_msg(sb, ...) if you're going to send a semi-structured notification, or include relevant information about which ext4 file system was responsible for issuing the warning.

Umm, ext4_msg does call printk.

Quoted 6 lines

If you're going to change huge numbers of lines of code, you might as well do it in a way that significantly improves things. The change to pr_foo() is just syntactic sugar, and that's a whitespace-level change in my book. Adding a struct super * or or a struct block device *, which gets passed to the notification functions? That's ***far*** more interesting.

It's hard to say that's true.
Look at the the trace_<foo> mechanisms.
Very useful stuff but once set, there's
been a strong desire to set the output
as an unchangeable ABI.

So I think defining the output correctly
_first_ is the most important element of
any notification mechanism. TLV use in
the output generally isn't human parsable
and there's value in that readability.

maintainer
#24 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 7 lines · Joe Perches
Quoted 5 lines

Changing to pr_err() is pointless, because it doesn't do anything functional. You *have* to have an interface like ext4_msg(sb, ...) if you're going to send a semi-structured notification, or include relevant information about which ext4 file system was responsible for issuing the warning.

Umm, ext4_msg does call printk.

Sure, but most of the code doesn't *use* printk. Instead they use ext4_msg(). That's the point. Changing printk() to ext4_msg() adds value. Changing printk() to pr_info() doesn't.

Quoted 14 lines
Quoted 7 lines

If you're going to change huge numbers of lines of code, you might as well do it in a way that significantly improves things. The change to pr_foo() is just syntactic sugar, and that's a whitespace-level change in my book. Adding a struct super * or or a struct block device *, which gets passed to the notification functions? That's ***far*** more interesting.

It's hard to say that's true.
Look at the the trace_<foo> mechanisms. Very useful stuff but once set, there's been a strong desire to set the output as an unchangeable ABI.

Quoted 3 lines

So I think defining the output correctly _first_ is the most important element of any notification mechanism.

But you can't set the output correctly if you don't pass the correct information. And you're right that there will be resistance in changing the output; which is why it's important that before a new file system like, say, btrfs goes into production, it has a way of providing a standardized printk output. Right now I'm very careful about changing certain ext4 error messages specifically because I *know* there are log scraping programs that are assume a certain format. So the sooner you standardize things, the better --- but in order to do that, you need to have the information so the output can include the critical information. What's important in other words is not the "EXT4-fs error" prefix; what's important is what follows it:

    EXT4-fs error (device %s): ...
                   ^^^^^^^^^

... because that's what the log scrapers are looking for. But you can't print that part of the message unless you pass in the struct super * parameter to ext4_error(). And this, by the way, is why we have ext4_msg() and ext4_error(). The log scrapers which look for "EXT4-fs error (device %s)" know that these messages are serious. But ext4_msg() log messages, which look like this: "EXT4-fs (%s): ..." are not things that log scrapers need to worry in terms of "the file system has been corrupted".

Quoted 3 lines

TLV use in
the output generally isn't human parsable and there's value in that readability.

Sure, that's why today we ext4_msg() and ext4_error() send human-readable messages to dmesg. I've never argued that we should replace that with something which is semi-structured; instead, we should *supplement* it with something which is more friendly to computer programs. But you can't supplement it, and you can't print messages such as:

    EXT4-fs error (device sda3): ...
                   ^^^^^^^^^^^

... if you don't have a standardized way of passing the struct super * to a standardized log function. Once you do that, it becomes much easier to do semi-custom things with the information. Someone could write a kernel module that sends out the information in XML. Another person could write a kernel module that sends out the information in protobufs (http://code.google.com/p/protobuf/). Still another person could use that information to more easily translate messages into Japanese, if that is what floats their boat. But if you don't pass the information into a standardized log function, you can't do any of this.

Of course, the body of the message needs to be standardized too. But that's orthogonal to the problem of passing the kernel structure which identifies the object which the log message is about. That part is completely and utterly necessary if you're going to standardize the *first* part of the printable dmesg log, which contains the structured information.

- Ted

#25 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

On Mon, 2012-03-19 at 22:58 -0400, Ted Ts'o wrote:

[good stuff]

Quoted 6 lines

Of course, the body of the message needs to be standardized too. But that's orthogonal to the problem of passing the kernel structure which identifies the object which the log message is about. That part is completely and utterly necessary if you're going to standardize the *first* part of the printable dmesg log, which contains the structured information.

All of what you say is true, but you're neglecting the ability to scrape or notify a particular set of per subsystem messages that pr_<level> could easily provide that a bare printk could not.

#26 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 11 lines · Joe Perches

On Mon, 2012-03-19 at 22:58 -0400, Ted Ts'o wrote:

[good stuff]

Quoted 6 lines

Of course, the body of the message needs to be standardized too. But that's orthogonal to the problem of passing the kernel structure which identifies the object which the log message is about. That part is completely and utterly necessary if you're going to standardize the *first* part of the printable dmesg log, which contains the structured information.

Quoted 4 lines

All of what you say is true, but you're neglecting the ability to scrape or notify a particular set of per subsystem messages that pr_<level> could easily provide that a bare printk could not.

OK. Say I'm a scraper. How do I distinguish between:

pr_info("foo");
printk(KERN_INFO "foo");

Oh my. seems that both result in exactly the same thing ending up in the dmesg buffer, so a scraper can't tell the difference because there isn't any...

pr_<level> isn't currently providing any ability to scrap or notify that can't be done equally well by a printk. Now if you fix things so there's actual advantage to or_<level>, then I'm sure Ted will be willing to listen.

#27 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

From: [email protected]
Date: Tue, 20 Mar 2012 01:46:06 -0400

Quoted 7 lines

OK. Say I'm a scraper. How do I distinguish between:

pr_info("foo");
printk(KERN_INFO "foo");

Oh my. seems that both result in exactly the same thing ending up in the dmesg buffer

No it doesn't result in the same output, read the definitions again.

pr_info can be influenced by pr_fmt, plain printk cannot

#28 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 8 lines · David Miller

From: [email protected]
Date: Tue, 20 Mar 2012 01:46:06 -0400

Quoted 5 lines

OK. Say I'm a scraper. How do I distinguish between:

pr_info("foo");
printk(KERN_INFO "foo");

Quoted 6 lines
Quoted 2 lines

Oh my. seems that both result in exactly the same thing ending up in the dmesg buffer

No it doesn't result in the same output, read the definitions again.

pr_info can be influenced by pr_fmt, plain printk cannot

Ok, but how exactly does one select per-subsystem messages relying only on pr_fmt?

Joe writes: "notify a particular set of per subsystem messages that pr_<level> could easily provide"

Maybe the concept is not explained well enough that we do not follow?

thanks,

Signature · 2 lines
js
suse labs
#29 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 4 lines · Jiri Slaby

Joe writes: "notify a particular set of per subsystem messages that pr_<level> could easily provide"

Maybe the concept is not explained well enough that we do not follow?

Basically, when pr_fmt(fmt) is #defined to something other than "#define pr_fmt(fmt) fmt", it could be used as a mechanism to do more than standardize prefixes.

__builtin_strcmp is pretty useful.

You can do things like:
(I'm just typing, this may have errors)

#define _first_macro_arg(arg1) arg1
#define first_macro_arg(arg1, ...) _first_macro_arg(arg1)

#define pr_info(fmt, ...) \

do {								\
	if (!__builtin_strcmp(first_macro_arg(pr_fmt(fmt)),	\
			      KBUILD_MODNAME ": " fmt)))	\
		advanced_printk(fmt, ##__VA_ARGS__);		\
	else							\
		printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__);	\

while (0)

etc...

Anyway, it's been a couple of years since I prototyped something like that and tested file sizes.

The size delta of an allyesconfig was:

-rwxr-xr-x 1 joe joe 926968471 2010-06-29 10:39 vmlinux.allyesconfig.printk_level_with_module -rwxr-xr-x 1 joe joe 927196127 2010-06-23 02:40 vmlinux.allyesconfig.pr_level

It reduced an allyesconfig by .1%
It wasn't a huge saving.

I'll play with it again and send an rfc
patch in a couple of days.

Maybe I'll prototype a pr_notify mechanism too.

cheers, Joe

#30 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

In reply to David Miller
Quoted 8 lines · David Miller

From: [email protected]
Date: Tue, 20 Mar 2012 01:46:06 -0400

Quoted 5 lines

OK. Say I'm a scraper.  How do I distinguish between:

pr_info("foo");
printk(KERN_INFO "foo");

Quoted 6 lines
Quoted 2 lines

Oh my. seems that both result in exactly the same thing ending up in the dmesg buffer

No it doesn't result in the same output, read the definitions again.

pr_info can be influenced by pr_fmt, plain printk cannot

And for pr_debug(), there's even more, cfr. CONFIG_DYNAMIC_DEBUG.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

maintainer
#31 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

In reply to David Miller
Quoted 11 lines · David Miller
Quoted 7 lines

OK. Say I'm a scraper. How do I distinguish between:

pr_info("foo");
printk(KERN_INFO "foo");

Oh my. seems that both result in exactly the same thing ending up in the dmesg buffer

No it doesn't result in the same output, read the definitions again.

pr_info can be influenced by pr_fmt, plain printk cannot

But if you don't use pr_fmt, it's the same. And even if you do, then pr_info() is the same as printk(KERN_INFO pr_fmt ...). For a device driver where cut and paste reuse happens, sure, it's handy. But for the rest of the kernel, it doesn't really buy you anything.

And as I've said, something like ext4_msg(sb, ...) is **far** more useful. I don't need a 1200-line patch that messes with debug printk's that are #ifdef'ed out.

- Ted

#32 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

In reply to David Miller
Quoted 8 lines · David Miller

From: [email protected]
Date: Tue, 20 Mar 2012 01:46:06 -0400

Quoted 5 lines

OK. Say I'm a scraper. How do I distinguish between:

pr_info("foo");
printk(KERN_INFO "foo");

Quoted 6 lines
Quoted 2 lines

Oh my. seems that both result in exactly the same thing ending up in the dmesg buffer

No it doesn't result in the same output, read the definitions again.

pr_info can be influenced by pr_fmt, plain printk cannot

So if nobody screws around with a #define pr_fmt, you can't tell the difference, and if somebody *does* screw around wiht a #define pr_fmt, you have no good way to know what *this* build of the kernel produces as output without tracking down the actual define in use. Consider Joe Perche's example "you can do things like" - you don't have a good programmatic way to tell what that pr_info() actually outputs, so an actual human is going to have to review that and figure out what needs to be pattern-matched.

Not a situation you want to be in if you're a scraper.

At least the perf subsystem is nice enough to *tell* you "I've added an extra field you may not know about" and provides a mechanism to skip over it. And I can see why Ted doesn't want to have code churn to do a half-assed conversion, when there will just be *more* churn if and when we ever get off our collective butts and deploy a structured solution.

#33 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

In reply to Ted Ts'o
Quoted 4 lines · Ted Ts'o

Yes, but we can't do structured notifications with the current pr_<foo>. So why change literally tens of thousands of callsites when in order to really realize the full promise of structured notifications, we'll have to change them *again*?

Two words: commit count.

Al, expecting a rant from mingo about inexcusable hostility to new duhvelopers any time now...

#34 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

In reply to Ted Ts'o

From: "Ted Ts'o" <[email protected]>
Date: Mon, 19 Mar 2012 11:31:34 -0400

Quoted 3 lines

And in the past we've always allowed for local variations of style; it's never been a strict style nazism. Linus has always given maintainers latitude and has frowned on patch churn for churn's sake.

The variations in opinion and taste are why it's important for us to discuss the interfaces and tweak them so that most people are happy with them and then everyone uses them.

Your story about get_fs() still being around isn't a victory, or an accomplishment, it's just a small failure that we've been too lazy to fix. But it's one that if someone did post a patch for it, would be accepted.

And the thing you call churn, I call development and progress.

#35 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

In reply to Ted Ts'o
Quoted 2 lines · Ted Ts'o

One evidence that this patch is noise is that it doesn't apply cleanly just on top of my current patch set that I plan to send to Linus.

That's crap for evidence.

It's more evident that you don't make
public your own internal patch queue
quickly enough than anything else.

maintainer
#36 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 9 lines · Joe Perches
Quoted 2 lines · Ted Ts'o

One evidence that this patch is noise is that it doesn't apply cleanly just on top of my current patch set that I plan to send to Linus.

That's crap for evidence.

It's more evident that you don't make
public your own internal patch queue
quickly enough than anything else.

It's all in linux-next, under the dev branch. There are a few things which I recently added over the weekend, but it's all been there for quite a while.

Tell you what, when you write your own file system you can develop it however you please.

- Ted

#37 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

In reply to Joe Perches
Quoted 9 lines · Joe Perches
Quoted 2 lines · Ted Ts'o

One evidence that this patch is noise is that it doesn't apply cleanly just on top of my current patch set that I plan to send to Linus.

That's crap for evidence.

It's more evident that you don't make
public your own internal patch queue
quickly enough than anything else.

Please stop repeating that shit. I already explained you the reasons. A couple of weeks ago.

thanks,

Signature · 1 line
js
#38 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 9 lines · Jiri Slaby
Quoted 6 lines · Joe Perches
Quoted 2 lines · Ted Ts'o

One evidence that this patch is noise is that it doesn't apply cleanly just on top of my current patch set that I plan to send to Linus.

It's more evident that you don't make
public your own internal patch queue
quickly enough than anything else.

Please stop repeating that shit. I already explained you the reasons. A couple of weeks ago.

shrug.

I don't quite understand your comment on this subject as git log -i --grep=slaby fs/ext4 doesn't show your name.

As far as I can tell, your "reasons" as you explained it is that you just do not like whitespace or style changes.

Ted seems to tend to defer changes to ext4. And in this case, Ted applied other patches before mine.

The issues at hand are simply normal patch conflict resolution and taste. Ted's free to do what he wants with patches he receives.

Accept/reject/ridicule/ignore/defer, etc... Whatever, it doesn't bother me.

My patch applies cleanly on -next (ext4 #dev) as of the date I sent it. I can not do more than that except to resend it after resolving the conflicts. I see no real good reason to do that currently. There are no patches from you for ext4 between the patch I sent and now.

cheers, Joe

#39 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 11 lines · Joe Perches
Quoted 2 lines · Jiri Slaby

Please stop repeating that shit. I already explained you the reasons. A couple of weeks ago.

I don't quite understand your comment on this subject as git log -i --grep=slaby fs/ext4 doesn't show your name.

do that currently. There are no patches from you for ext4 between the patch I sent and now.

Oh, give us a *break* Joe. JIri's not allowed to comment on the situation merely because he doesn't have an in-flight patch at the moment?

#40 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 11 lines · On Mon, 2012-03-19 at 13:36 -0400, [email protected] wrote:
Quoted 8 lines · Joe Perches
Quoted 2 lines · Jiri Slaby

Please stop repeating that shit. I already explained you the reasons. A couple of weeks ago.

I don't quite understand your comment on this subject as git log -i --grep=slaby fs/ext4 doesn't show your name.
do that currently. There are no patches from you for ext4 between the patch I sent and now.

Oh, give us a *break* Joe. JIri's not allowed to comment on the situation merely because he doesn't have an in-flight patch at the moment?

Of course he's allowed to comment.

I just don't understand his comment as anything
other than general grumpiness.

His "explanation" to me was he as far as I can
tell was he didn't like whitespace only changes.

https://lkml.org/lkml/2012/2/27/266

This stuff ain't whitespace.

cheers, Joe

maintainer
#41 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 3 lines · Joe Perches

This stuff ain't whitespace.

I'll have to respectfully disagree with you. For ext4, as far as I am concerned, changing printk(KERN_INFO, ...) to pr_info(...) is *purely* a whitespace-level change.

- Ted

#42 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 8 lines · Ted Ts'o
Quoted 3 lines · Joe Perches

This stuff ain't whitespace.

I'll have to respectfully disagree with you. For ext4, as far as I am concerned, changing printk(KERN_INFO, ...) to pr_info(...) is *purely* a whitespace-level change.

Joe, if everything did change, what difference would it make?

just being a consistant style doesn't matter much, but if there is some functionality that would be possible with pr_info(...) that would not be possible with printk(KERN_INFO, ...), there may be more reason to change.

David Lang

#43 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 7 lines · On Mon, 2012-03-19 at 18:28 -0700, [email protected] wrote:
Quoted 5 lines · Ted Ts'o
Quoted 2 lines · Joe Perches

This stuff ain't whitespace.

Quoted 1 line

I'll have to respectfully disagree with you. For ext4, as far as I am

concerned, changing printk(KERN_INFO, ...) to pr_info(...) is *purely* a whitespace-level change.

Joe, if everything did change, what difference would it make?

Hi David.

Quoted 1 line

just being a consistant style doesn't matter much,

Perhaps we disagree on the value of consistency. I believe it's a small but measurable effect and it can reduce overall ongoing defect rates.

Is it DoublePlus_important_? No, definitely not.

To me using pr_<level> is a bit like using const or marking sections devinitconst.

It hardly matters, but it's good form and it can free up some working memory in ram limited systems.

Quoted 3 lines

but if there is some
functionality that would be possible with pr_info(...) that would not be possible with printk(KERN_INFO, ...), there may be more reason to change.

Right now, it's just macros over printk so it's pretty trivial. I do intend to convert pr_<level> macros to functions eventually to reduce code size ~.5% overall. That reduction does depend on quantity of CONFIG_<FOO> options enabled of course. Enable everything, I think it's ~.01%. I haven't done it in quite awhile though so that's a guess.

It matters a tiny bit more for flash or ram limited systems.

Some driver optimizations like the rtlwifi reduction in -next commit 481b9606ec might have more of an impact though for those systems.

So, it depends...

cheers, Joe

#44 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

In reply to Ted Ts'o
Quoted 5 lines · Ted Ts'o
Quoted 1 line · Joe Perches

This stuff ain't whitespace.

I'll have to respectfully disagree with you. For ext4, as far as I am concerned, changing printk(KERN_INFO, ...) to pr_info(...) is *purely* a whitespace-level change.

You've a non-standard definition of whitespace.

cheers, Joe

#45 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

In reply to Joe Perches
Quoted 10 lines · Joe Perches
Quoted 9 lines · Jiri Slaby
Quoted 6 lines · Joe Perches
Quoted 2 lines · Ted Ts'o

One evidence that this patch is noise is that it doesn't apply cleanly just on top of my current patch set that I plan to send to Linus.

It's more evident that you don't make
public your own internal patch queue
quickly enough than anything else.

Please stop repeating that shit. I already explained you the reasons. A couple of weeks ago.

...

Quoted 3 lines

As far as I can tell, your "reasons" as you
explained it is that you just do not like
whitespace or style changes.

No, sorry, you misunderstand me. Here, I meant your complain that we are not fast enough with pushing patches. Some of us simply do not push patches as soon as they are written. I personally prefer deferring patches for some time (weeks) to actually give some testing to the changes. Simply because I have bad experiences with patches emerging in the (-next) tree immediately. (And I mean not only my patches.)

While you are pointing to whitespace cleanup of whole subtrees. Yes, I wrote about _whitespace_ cleanup in the thread you are referring to. But this can be generalized to an arbitrary cleanup. I.e. changes which do not change functionality in no way. And that is doubtful with pr_* changes. But as you can see in my post a couple of minutes ago, maybe I am just missing the point of the new interface. (And I hate naming of the pr_* functions.)

And BTW setting the argument that it is a newer interface (that is what ath5k pr_* conversion patches commit log says) is not a good justification at all.

thanks,

Signature · 2 lines
js
suse labs
#46 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 5 lines · Jiri Slaby
Quoted 3 lines · Joe Perches

As far as I can tell, your "reasons" as you
explained it is that you just do not like
whitespace or style changes.

No, sorry, you misunderstand me.

No, sorry, you were simply unclear in your reference.

Quoted 3 lines

And BTW setting the argument that it is a newer interface (that is what ath5k pr_* conversion patches commit log says) is not a good justification at all.

No again, the ath5k pr_ conversion patches are to
standardize prefixes and to reduce code size by
centralizing tests.

https://lkml.org/lkml/2012/3/18/121
https://lkml.org/lkml/2012/3/18/123
https://lkml.org/lkml/2012/3/18/124

I happen to like 30K/10+% code size reductions.

#47 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 8 lines · Joe Perches
Quoted 3 lines · Jiri Slaby

And BTW setting the argument that it is a newer interface (that is what ath5k pr_* conversion patches commit log says) is not a good justification at all.

No again, the ath5k pr_ conversion patches are to
standardize prefixes and to reduce code size by
centralizing tests.

What is the "standard" prefix?

Quoted 5 lines

https://lkml.org/lkml/2012/3/18/121
https://lkml.org/lkml/2012/3/18/123
https://lkml.org/lkml/2012/3/18/124

I happen to like 30K/10+% code size reductions.

Obvious question: what speed reduction does this bring?

thanks,

Signature · 2 lines
js
suse labs
#48 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 6 lines · Jiri Slaby
Quoted 3 lines · Joe Perches

the ath5k pr_ conversion patches are to
standardize prefixes and to reduce code size by
centralizing tests.

What is the "standard" prefix?

KBUILD_MODNAME

Quoted 5 lines
Quoted 4 lines

https://lkml.org/lkml/2012/3/18/121
https://lkml.org/lkml/2012/3/18/123
https://lkml.org/lkml/2012/3/18/124
I happen to like 30K/10+% code size reductions.

Obvious question: what speed reduction does this bring?

Anyone looking for speed impacts in logging
messages is looking in the wrong place.

btw: it's effectively 0.

#49 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 9 lines · Joe Perches
Quoted 6 lines · Jiri Slaby
Quoted 3 lines · Joe Perches

the ath5k pr_ conversion patches are to
standardize prefixes and to reduce code size by
centralizing tests.

What is the "standard" prefix?

KBUILD_MODNAME

Instead, we should switch as many printks to dev_* and similar as possible, right? They are standard and provide a good interface for
extensions: one has a device to work with. This is something what pr_* does not offer. For that reason it's not worth it.

Quoted 8 lines
Quoted 5 lines
Quoted 4 lines

https://lkml.org/lkml/2012/3/18/121
https://lkml.org/lkml/2012/3/18/123
https://lkml.org/lkml/2012/3/18/124
I happen to like 30K/10+% code size reductions.

Obvious question: what speed reduction does this bring?

Anyone looking for speed impacts in logging
messages is looking in the wrong place.

Ok, I finally found some time to look into the changes properly. I was confused by Adrian's note that you moved a debug check to an external function. But that is not true.

Quoted 1 line

btw: it's effectively 0.

Frankly, moving the debug code to a separate function should make the code rather faster. By moving the unlikely code out of the instruction cache.

thanks,
-- js suse labs

#50 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

Quoted 11 lines · Jiri Slaby
Quoted 7 lines · Joe Perches
Quoted 5 lines · Jiri Slaby
Quoted 3 lines · Joe Perches

the ath5k pr_ conversion patches are to
standardize prefixes and to reduce code size by
centralizing tests.

What is the "standard" prefix?

KBUILD_MODNAME

Instead, we should switch as many printks to dev_* and similar as possible, right?

I believe logging message use should be selected to be as specific as possible (from high to low):

o <subsystem>_<level>
o netif_<level>
o netdev_<level>
o dev_<level>
o pr_<level>
o printk(KERN_<LEVEL>

Quoted 3 lines

They are standard and provide a good interface for
extensions: one has a device to work with. This is something what pr_* does not offer.

I completely agree.

I'd like to see some new subsystem prefixes created and used as well. Maybe:

fs_<level> (like Ted's ext4_<foo>) io_<level> (scsi/ide/mtd)
mm_<level>

There are probably a few others like cpu/video/sound that might be useful.

Quoted 3 lines

Frankly, moving the debug code to a separate function should make the code rather faster. By moving the unlikely code out of the instruction cache.

I think it's better still to eliminate debug code altogether via if (0) when reasonable or use dynamic_debug (jump_tables) when overall code size isn't an issue.

cheers, Joe

maintainer
#51 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

In reply to Joe Perches
Quoted 1 line · Joe Perches

Use a more current logging style.

Oh, just for yucks, I did a quick check:

% git grep printk | wc -l
71266
% git grep pr_ | wc -l
29552

(and there are no doubt some false positives when grepping for "pr_")

- Ted

#52 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

From: "Ted Ts'o" <[email protected]>
Date: Mon, 19 Mar 2012 00:55:14 -0400

Quoted 9 lines · Joe Perches
Quoted 1 line

Use a more current logging style.

Oh, just for yucks, I did a quick check:

% git grep printk | wc -l
71266
% git grep pr_ | wc -l
29552

It's up to 30,000 exactly because of the hard work of people like Joe converting the tree over gradually.

If people like you keep resisting these kinds of changes, we'll never get the whole tree using these nice new shorthands.

#53 lore raw

Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>

In reply to Ted Ts'o
Quoted 8 lines · Ted Ts'o
Quoted 1 line · Joe Perches

Use a more current logging style.

Oh, just for yucks, I did a quick check:
% git grep printk | wc -l
71266
% git grep pr_ | wc -l
29552
(and there are no doubt some false positives when grepping for "pr_")

As there are for printk.

I use "\bpr_(emerg|crit|alert|err|notice|warn|warning|info|debug)\b" and "\bprintk\b" or -w

I did write "more current" and not predominate as there are large blocks of historical crud.

You might try the same test on current code in the last couple of years or so.

pr_<foo> was introduced October 2007 by commit 1f7c8234c7a68

$ git log -p --since=2-years-ago | \
  grep -P "^\+.*\bpr_(emerg|crit|alert|err|notice|warn|warning|info|debug)\b" | \
  wc -l
22704
$ git log -p --since=2-years-ago | \
  grep -P "^\+.*\bprintk\b" | \
  wc -l
24899

A lot of that was original submissions of older out-of-tree drivers with printk that were later converted to pr_<level> or dev_<level>, etc.

In an aside:

Gads, I need a faster netbook, that took 10's of minutes to produce...

Any suggestions?

cheers, Joe

#54 lore raw

PATCH 3/9 ext4: Fix indentation

In reply to Joe Perches

Indent a line appropriate.

---
 fs/ext4/extents.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
fs/ext4/extents.c +3 -2
index 4ca1c7e..c39a737 100644
@@ -533,8 +533,9 @@ ext4_ext_binsearch_idx(struct inode *inode,
 
 		chix = ix = EXT_FIRST_INDEX(eh);
 		for (k = 0; k < le16_to_cpu(eh->eh_entries); k++, ix++) {
-		  if (k != 0 &&
-		      le32_to_cpu(ix->ei_block) <= le32_to_cpu(ix[-1].ei_block)) {
+			if (k != 0 &&
+			    le32_to_cpu(ix->ei_block) <=
+			    le32_to_cpu(ix[-1].ei_block)) {
 				pr_debug("k=%d, ix=0x%p, first=0x%p\n",
 					 k, ix, EXT_FIRST_INDEX(eh));
 				pr_debug("%u <= %u\n",
Signature · 1 line
1.7.8.111.gad25c.dirty
maintainer
#55 lore raw

Re: [PATCH 3/9] ext4: Fix indentation

Quoted 3 lines · Joe Perches

Indent a line appropriate.

I'll fix indentation and whitespaces if I need to actually go in and mess with that particular part of the code, but otherwise (since it doesn't apply cleanly standalone anyway), I'm not going to bother...

- Ted

#56 lore raw

Re: [PATCH 3/9] ext4: Fix indentation

Quoted 8 lines · Ted Ts'o
Quoted 3 lines · Joe Perches

Indent a line appropriate.

I'll fix indentation and whitespaces if I need to actually go in and mess with that particular part of the code, but otherwise (since it doesn't apply cleanly standalone anyway), I'm not going to bother...

No worries, it was next to a pr_<level> change and I just happened to notice it.

#57 lore raw

PATCH 4/9 ext4: Add no_printk argument validation, fix fallout

In reply to Joe Perches

Add argument validation to debug functions.
Use ##__VA_ARGS__.

Fix format and argument mismatches.

---
 fs/ext4/ext4.h         |    2 +-
 fs/ext4/ext4_extents.h |    4 ++--
 fs/ext4/inode.c        |    2 +-
 fs/ext4/xattr.c        |   13 ++++++++-----
 4 files changed, 12 insertions(+), 9 deletions(-)
fs/ext4/ext4.h +1 -1
index 30d32e2..2aaf549 100644
@@ -50,7 +50,7 @@
 	pr_debug("DEBUG (%s, %d): %s:" fmt,				\
 		 __FILE__, __LINE__, __func__, ##__VA_ARGS__)
 #else
-#define ext4_debug(f, a...)	do {} while (0)
+#define ext4_debug(fmt, ...)	no_printk(fmt, ##__VA_ARGS__)
 #endif
 
 #define EXT4_ERROR_INODE(inode, fmt, a...) \
fs/ext4/ext4_extents.h +2 -2
index a52db3a..0f58b86 100644
@@ -47,9 +47,9 @@
  */
 #define EXT_DEBUG__
 #ifdef EXT_DEBUG
-#define ext_debug(a...)		printk(a)
+#define ext_debug(fmt, ...)	printk(fmt, ##__VA_ARGS__)
 #else
-#define ext_debug(a...)
+#define ext_debug(fmt, ...)	no_printk(fmt, ##__VA_ARGS__)
 #endif
 
 /*
fs/ext4/inode.c +1 -1
index c67be80..a2a1ae4 100644
@@ -2766,7 +2766,7 @@ static void ext4_end_io_dio(struct kiocb *iocb, loff_t offset,
 		goto out;
 
 	ext_debug("ext4_end_io_dio(): io_end 0x%p "
-		  "for inode %lu, iocb 0x%p, offset %llu, size %llu\n",
+		  "for inode %lu, iocb 0x%p, offset %llu, size %zd\n",
  		  iocb->private, io_end->inode->i_ino, iocb, offset,
 		  size);
 
fs/ext4/xattr.c +8 -5
index a8674d2..5bc718c 100644
@@ -78,8 +78,8 @@ do {									\
 		 (unsigned long)bh->b_blocknr, ##__VA_ARGS__);		\
 } while (0)
 #else
-# define ea_idebug(f...)
-# define ea_bdebug(f...)
+# define ea_idebug(inode, fmt, ...)	no_printk(fmt, ##__VA_ARGS__)
+# define ea_bdebug(bh, fmt, ...)	no_printk(fmt, ##__VA_ARGS__)
 #endif
 
 static void ext4_xattr_cache_insert(struct buffer_head *);
@@ -213,7 +213,8 @@ ext4_xattr_block_get(struct inode *inode, int name_index, const char *name,
 	error = -ENODATA;
 	if (!EXT4_I(inode)->i_file_acl)
 		goto cleanup;
-	ea_idebug(inode, "reading block %u", EXT4_I(inode)->i_file_acl);
+	ea_idebug(inode, "reading block %llu",
+		  (unsigned long long)EXT4_I(inode)->i_file_acl);
 	bh = sb_bread(inode->i_sb, EXT4_I(inode)->i_file_acl);
 	if (!bh)
 		goto cleanup;
@@ -356,7 +357,8 @@ ext4_xattr_block_list(struct dentry *dentry, char *buffer, size_t buffer_size)
 	error = 0;
 	if (!EXT4_I(inode)->i_file_acl)
 		goto cleanup;
-	ea_idebug(inode, "reading block %u", EXT4_I(inode)->i_file_acl);
+	ea_idebug(inode, "reading block %llu",
+		  (unsigned long long)EXT4_I(inode)->i_file_acl);
 	bh = sb_bread(inode->i_sb, EXT4_I(inode)->i_file_acl);
 	error = -EIO;
 	if (!bh)
@@ -828,7 +830,8 @@ inserted:
 			if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)))
 				BUG_ON(block > EXT4_MAX_BLOCK_FILE_PHYS);
 
-			ea_idebug(inode, "creating block %d", block);
+			ea_idebug(inode, "creating block %llu",
+				  (unsigned long long)block);
 
 			new_bh = sb_getblk(sb, block);
 			if (!new_bh) {
Signature · 1 line
1.7.8.111.gad25c.dirty
maintainer Applied
#58 lore raw

Re: [PATCH 4/9] ext4: Add no_printk argument validation, fix fallout

Quoted 6 lines · Joe Perches

Add argument validation to debug functions.
Use ##__VA_ARGS__.

Fix format and argument mismatches.

Applied, thanks.

- Ted

#59 lore raw

PATCH 5/9 ext4: Avoid output message interleaving in ext4_error_<foo>

In reply to Joe Perches

Using pr_cont means that messages from multiple threads may be interleaved. Avoid this by using a single pr_<level> call in ext4_error_inode and ext4_error_file.

---
 fs/ext4/super.c |   24 +++++++++++++++---------
 1 files changed, 15 insertions(+), 9 deletions(-)
fs/ext4/super.c +15 -9
index 4e1d656..ac77a95 100644
@@ -517,11 +517,14 @@ void ext4_error_inode(struct inode *inode, const char *function,
 	va_start(args, fmt);
 	vaf.fmt = fmt;
 	vaf.va = &args;
-	pr_crit("error (device %s): %s:%d: inode #%lu: ",
-		inode->i_sb->s_id, function, line, inode->i_ino);
 	if (block)
-		pr_cont("block %llu: ", block);
-	pr_cont("comm %s: %pV\n", current->comm, &vaf);
+		pr_crit("error (device %s): %s:%d: inode #%lu: block %llu: comm %s: %pV\n",
+			inode->i_sb->s_id, function, line, inode->i_ino,
+			block, current->comm, &vaf);
+	else
+		pr_crit("error (device %s): %s:%d: inode #%lu: comm %s: %pV\n",
+			inode->i_sb->s_id, function, line, inode->i_ino,
+			current->comm, &vaf);
 	va_end(args);
 
 	ext4_handle_error(inode->i_sb);
@@ -543,14 +546,17 @@ void ext4_error_file(struct file *file, const char *function,
 	path = d_path(&(file->f_path), pathname, sizeof(pathname));
 	if (IS_ERR(path))
 		path = "(unknown)";
-	pr_crit("error (device %s): %s:%d: inode #%lu: ",
-		inode->i_sb->s_id, function, line, inode->i_ino);
-	if (block)
-		pr_cont("block %llu: ", block);
 	va_start(args, fmt);
 	vaf.fmt = fmt;
 	vaf.va = &args;
-	pr_cont("comm %s: path %s: %pV\n", current->comm, path, &vaf);
+	if (block)
+		pr_crit("error (device %s): %s:%d: inode #%lu: block %llu: comm %s: path %s: %pV\n",
+			inode->i_sb->s_id, function, line, inode->i_ino,
+			block, current->comm, path, &vaf);
+	else
+		pr_crit("error (device %s): %s:%d: inode #%lu: comm %s: path %s: %pV\n",
+			inode->i_sb->s_id, function, line, inode->i_ino,
+			current->comm, path, &vaf);
 	va_end(args);
 
 	ext4_handle_error(inode->i_sb);
Signature · 1 line
1.7.8.111.gad25c.dirty
maintainer
#60 lore raw

Re: [PATCH 5/9] ext4: Avoid output message interleaving in ext4_error_<foo>

Quoted 5 lines · Joe Perches

Using pr_cont means that messages from multiple threads may be interleaved. Avoid this by using a single pr_<level> call in ext4_error_inode and ext4_error_file.

Applied, (with some changes so the patch would apply cleanly), thanks.

- Ted

#61 lore raw

PATCH 6/9 ext4: Remove redundant "EXT4-fs: " from uses of ext4_msg

In reply to Joe Perches

ext4_msg adds "EXT4-fs: " to the messsage output.
Remove the redundant bits from uses.

---
 fs/ext4/mballoc.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)
fs/ext4/mballoc.c +7 -7
index c26b600..cd4d7c6 100644
@@ -2178,7 +2178,7 @@ int ext4_mb_add_groupinfo(struct super_block *sb, ext4_group_t group,
 			EXT4_DESC_PER_BLOCK_BITS(sb);
 		meta_group_info = kmalloc(metalen, GFP_KERNEL);
 		if (meta_group_info == NULL) {
-			ext4_msg(sb, KERN_ERR, "EXT4-fs: can't allocate mem "
+			ext4_msg(sb, KERN_ERR, "can't allocate mem "
 				 "for a buddy group");
 			goto exit_meta_group_info;
 		}
@@ -2192,7 +2192,7 @@ int ext4_mb_add_groupinfo(struct super_block *sb, ext4_group_t group,
 
 	meta_group_info[i] = kmem_cache_alloc(cachep, GFP_KERNEL);
 	if (meta_group_info[i] == NULL) {
-		ext4_msg(sb, KERN_ERR, "EXT4-fs: can't allocate buddy mem");
+		ext4_msg(sb, KERN_ERR, "can't allocate buddy mem");
 		goto exit_group_info;
 	}
 	memset(meta_group_info[i], 0, kmem_cache_size(cachep));
@@ -3864,11 +3864,11 @@ static void ext4_mb_show_ac(struct ext4_allocation_context *ac)
 	    (EXT4_SB(sb)->s_mount_flags & EXT4_MF_FS_ABORTED))
 		return;
 
-	ext4_msg(ac->ac_sb, KERN_ERR, "EXT4-fs: Can't allocate:"
+	ext4_msg(ac->ac_sb, KERN_ERR, "Can't allocate:"
 			" Allocation context details:");
-	ext4_msg(ac->ac_sb, KERN_ERR, "EXT4-fs: status %d flags %d",
+	ext4_msg(ac->ac_sb, KERN_ERR, "status %d flags %d",
 			ac->ac_status, ac->ac_flags);
-	ext4_msg(ac->ac_sb, KERN_ERR, "EXT4-fs: orig %lu/%lu/%lu@%lu, "
+	ext4_msg(ac->ac_sb, KERN_ERR, "orig %lu/%lu/%lu@%lu, "
 		 	"goal %lu/%lu/%lu@%lu, "
 			"best %lu/%lu/%lu@%lu cr %d",
 			(unsigned long)ac->ac_o_ex.fe_group,
@@ -3884,9 +3884,9 @@ static void ext4_mb_show_ac(struct ext4_allocation_context *ac)
 			(unsigned long)ac->ac_b_ex.fe_len,
 			(unsigned long)ac->ac_b_ex.fe_logical,
 			(int)ac->ac_criteria);
-	ext4_msg(ac->ac_sb, KERN_ERR, "EXT4-fs: %lu scanned, %d found",
+	ext4_msg(ac->ac_sb, KERN_ERR, "%lu scanned, %d found",
 		 ac->ac_ex_scanned, ac->ac_found);
-	ext4_msg(ac->ac_sb, KERN_ERR, "EXT4-fs: groups: ");
+	ext4_msg(ac->ac_sb, KERN_ERR, "groups: ");
 	ngroups = ext4_get_groups_count(sb);
 	for (i = 0; i < ngroups; i++) {
 		struct ext4_group_info *grp = ext4_get_group_info(sb, i);
Signature · 1 line
1.7.8.111.gad25c.dirty
maintainer Applied
#62 lore raw

Re: [PATCH 6/9] ext4: Remove redundant "EXT4-fs: " from uses of ext4_msg

Quoted 4 lines · Joe Perches

ext4_msg adds "EXT4-fs: " to the messsage output.
Remove the redundant bits from uses.

Applied, thanks.

- Ted

#63 lore raw

PATCH 7/9 ext4: Format neatening for easier grep

In reply to Joe Perches

Split strings are difficult to grep so make it easier.

Coalesce strings. Add spaces between words in newly coalesced formats. Remove unnecessary terminating \n's from ext4_msg uses as ext4_msg already adds it. Add terminating \n's to ext4_debug uses. Use %s, __func__ instead of embedded function names.

---
 fs/ext4/ext4_jbd2.c   |    3 +-
 fs/ext4/extents.c     |   85 ++++++-------
 fs/ext4/file.c        |    3 +-
 fs/ext4/fsync.c       |    2 +-
 fs/ext4/inode.c       |   43 +++----
 fs/ext4/mballoc.c     |   93 +++++++-------
 fs/ext4/mmp.c         |   23 ++--
 fs/ext4/move_extent.c |   86 +++++--------
 fs/ext4/namei.c       |    4 +-
 fs/ext4/page-io.c     |   12 +--
 fs/ext4/resize.c      |   22 ++--
 fs/ext4/super.c       |  331 +++++++++++++++++++++++--------------------------
 12 files changed, 321 insertions(+), 386 deletions(-)
fs/ext4/ext4_jbd2.c +1 -2
index aca1790..5365b66 100644
@@ -43,8 +43,7 @@ int __ext4_forget(const char *where, unsigned int line, handle_t *handle,
 	trace_ext4_forget(inode, is_metadata, blocknr);
 	BUFFER_TRACE(bh, "enter");
 
-	jbd_debug(4, "forgetting bh %p: is_metadata = %d, mode %o, "
-		  "data mode %x\n",
+	jbd_debug(4, "forgetting bh %p: is_metadata = %d, mode %o, data mode %x\n",
 		  bh, is_metadata, inode->i_mode,
 		  test_opt(inode->i_sb, DATA_FLAGS));
 
fs/ext4/extents.c +39 -46

142 lines not on this page · raw message

fs/ext4/file.c +1 -2
index cb70f18..a979bcd 100644
@@ -126,8 +126,7 @@ ext4_file_write(struct kiocb *iocb, const struct iovec *iov,
 		/* Warn about this once per day */
 		if (printk_timed_ratelimit(&unaligned_warn_time, 60*60*24*HZ))
 			ext4_msg(inode->i_sb, KERN_WARNING,
-				 "Unaligned AIO/DIO on inode %ld by %s; "
-				 "performance will be poor.",
+				 "Unaligned AIO/DIO on inode %ld by %s; performance will be poor",
 				 inode->i_ino, current->comm);
 		mutex_lock(ext4_aio_mutex(inode));
 		ext4_aiodio_wait(inode);
fs/ext4/fsync.c +1 -1
index bb6c7d8..3f87aca 100644
@@ -46,7 +46,7 @@ static void dump_completed_IO(struct inode * inode)
 		return;
 	}
 
-	ext4_debug("Dump inode %lu completed_io list \n", inode->i_ino);
+	ext4_debug("Dump inode %lu completed_io list\n", inode->i_ino);
 	spin_lock_irqsave(&EXT4_I(inode)->i_completed_io_lock, flags);
 	list_for_each_entry(io, &EXT4_I(inode)->i_completed_io_list, list){
 		cur = &io->list;
fs/ext4/inode.c +20 -23

94 lines not on this page · raw message

fs/ext4/mballoc.c +44 -49

129 lines not on this page · raw message

fs/ext4/mmp.c +10 -13
index ed6548d..1767b2a 100644
@@ -73,8 +73,7 @@ void __dump_mmp_msg(struct super_block *sb, struct mmp_struct *mmp,
 {
 	__ext4_warning(sb, function, line, msg);
 	__ext4_warning(sb, function, line,
-		       "MMP failure info: last update time: %llu, last update "
-		       "node: %s, last update device: %s\n",
+		       "MMP failure info: last update time: %llu, last update node: %s, last update device: %s",
 		       (long long unsigned int) le64_to_cpu(mmp->mmp_time),
 		       mmp->mmp_nodename, mmp->mmp_bdevname);
 }
@@ -133,15 +132,13 @@ static int kmmpd(void *data)
 
 		if (!(le32_to_cpu(es->s_feature_incompat) &
 		    EXT4_FEATURE_INCOMPAT_MMP)) {
-			ext4_warning(sb, "kmmpd being stopped since MMP feature"
-				     " has been disabled.");
+			ext4_warning(sb, "kmmpd being stopped since MMP feature has been disabled");
 			EXT4_SB(sb)->s_mmp_tsk = NULL;
 			goto failed;
 		}
 
 		if (sb->s_flags & MS_RDONLY) {
-			ext4_warning(sb, "kmmpd being stopped since filesystem "
-				     "has been remounted as readonly.");
+			ext4_warning(sb, "kmmpd being stopped since filesystem has been remounted as readonly");
 			EXT4_SB(sb)->s_mmp_tsk = NULL;
 			goto failed;
 		}
@@ -274,11 +271,11 @@ int ext4_multi_mount_protect(struct super_block *sb,
 
 	/* Print MMP interval if more than 20 secs. */
 	if (wait_time > EXT4_MMP_MIN_CHECK_INTERVAL * 4)
-		ext4_warning(sb, "MMP interval %u higher than expected, please"
-			     " wait.\n", wait_time * 2);
+		ext4_warning(sb, "MMP interval %u higher than expected, please wait",
+			     wait_time * 2);
 
 	if (schedule_timeout_interruptible(HZ * wait_time) != 0) {
-		ext4_warning(sb, "MMP startup interrupted, failing mount\n");
+		ext4_warning(sb, "MMP startup interrupted, failing mount");
 		goto failed;
 	}
 
@@ -288,7 +285,7 @@ int ext4_multi_mount_protect(struct super_block *sb,
 	mmp = (struct mmp_struct *)(bh->b_data);
 	if (seq != le32_to_cpu(mmp->mmp_seq)) {
 		dump_mmp_msg(sb, mmp,
-			     "Device is already active on another node.");
+			     "Device is already active on another node");
 		goto failed;
 	}
 
@@ -307,7 +304,7 @@ skip:
 	 * wait for MMP interval and check mmp_seq.
 	 */
 	if (schedule_timeout_interruptible(HZ * wait_time) != 0) {
-		ext4_warning(sb, "MMP startup interrupted, failing mount\n");
+		ext4_warning(sb, "MMP startup interrupted, failing mount");
 		goto failed;
 	}
 
@@ -317,7 +314,7 @@ skip:
 	mmp = (struct mmp_struct *)(bh->b_data);
 	if (seq != le32_to_cpu(mmp->mmp_seq)) {
 		dump_mmp_msg(sb, mmp,
-			     "Device is already active on another node.");
+			     "Device is already active on another node");
 		goto failed;
 	}
 
@@ -338,7 +335,7 @@ skip:
 	if (IS_ERR(EXT4_SB(sb)->s_mmp_tsk)) {
 		EXT4_SB(sb)->s_mmp_tsk = NULL;
 		kfree(mmpd_data);
-		ext4_warning(sb, "Unable to create kmmpd thread for %s.",
+		ext4_warning(sb, "Unable to create kmmpd thread for %s",
 			     sb->s_id);
 		goto failed;
 	}
fs/ext4/move_extent.c +34 -52

164 lines not on this page · raw message

fs/ext4/namei.c +2 -2
index b10ee89..99f86a7 100644
@@ -474,8 +474,8 @@ fail2:
 fail:
 	if (*err == ERR_BAD_DX_DIR)
 		ext4_warning(dir->i_sb,
-			     "Corrupt dir inode %lu, running e2fsck is "
-			     "recommended.", dir->i_ino);
+			     "Corrupt dir inode %lu, running e2fsck is recommended.",
+			     dir->i_ino);
 	return NULL;
 }
 
fs/ext4/page-io.c +4 -8
index 09c9d4b..a5afbad 100644
@@ -95,16 +95,13 @@ int ext4_end_io_nolock(ext4_io_end_t *io)
 	ssize_t size = io->size;
 	int ret = 0;
 
-	ext4_debug("ext4_end_io_nolock: io 0x%p from inode %lu,list->next 0x%p,"
-		   "list->prev 0x%p\n",
-		   io, inode->i_ino, io->list.next, io->list.prev);
+	ext4_debug("%s: io 0x%p from inode %lu,list->next 0x%p,list->prev 0x%p\n",
+		   __func__, io, inode->i_ino, io->list.next, io->list.prev);
 
 	ret = ext4_convert_unwritten_extents(inode, offset, size);
 	if (ret < 0) {
 		ext4_msg(inode->i_sb, KERN_EMERG,
-			 "failed to convert unwritten extents to written "
-			 "extents -- potential data loss!  "
-			 "(inode %lu, offset %llu, size %zd, error %d)",
+			 "failed to convert unwritten extents to written extents -- potential data loss!  (inode %lu, offset %llu, size %zd, error %d)",
 			 inode->i_ino, offset, size, ret);
 	}
 
@@ -241,8 +238,7 @@ static void ext4_end_bio(struct bio *bio, int error)
 
 	if (error) {
 		io_end->flag |= EXT4_IO_END_ERROR;
-		ext4_warning(inode->i_sb, "I/O error writing to inode %lu "
-			     "(offset %llu size %ld starting block %llu)",
+		ext4_warning(inode->i_sb, "I/O error writing to inode %lu (offset %llu size %ld starting block %llu)",
 			     inode->i_ino,
 			     (unsigned long long) io_end->offset,
 			     (long) io_end->size,
fs/ext4/resize.c +8 -14
index 83884e8..ed3a306 100644
@@ -29,8 +29,7 @@ int ext4_resize_begin(struct super_block *sb)
 	 * with error, because it can destroy the filesystem easily.
 	 */
 	if (EXT4_SB(sb)->s_mount_state & EXT4_ERROR_FS) {
-		ext4_warning(sb, "There are errors in the filesystem, "
-			     "so online resizing is not allowed\n");
+		ext4_warning(sb, "There are errors in the filesystem, so online resizing is not allowed");
 		return -EPERM;
 	}
 
@@ -104,13 +103,11 @@ static int verify_group_input(struct super_block *sb,
 		ext4_warning(sb, "Block bitmap same as inode bitmap (%llu)",
 			     (unsigned long long)input->block_bitmap);
 	else if (inside(input->block_bitmap, input->inode_table, itend))
-		ext4_warning(sb, "Block bitmap (%llu) in inode table "
-			     "(%llu-%llu)",
+		ext4_warning(sb, "Block bitmap (%llu) in inode table (%llu-%llu)",
 			     (unsigned long long)input->block_bitmap,
 			     (unsigned long long)input->inode_table, itend - 1);
 	else if (inside(input->inode_bitmap, input->inode_table, itend))
-		ext4_warning(sb, "Inode bitmap (%llu) in inode table "
-			     "(%llu-%llu)",
+		ext4_warning(sb, "Inode bitmap (%llu) in inode table (%llu-%llu)",
 			     (unsigned long long)input->inode_bitmap,
 			     (unsigned long long)input->inode_table, itend - 1);
 	else if (inside(input->block_bitmap, start, metaend))
@@ -123,8 +120,7 @@ static int verify_group_input(struct super_block *sb,
 			     start, metaend - 1);
 	else if (inside(input->inode_table, start, metaend) ||
 		 inside(itend - 1, start, metaend))
-		ext4_warning(sb, "Inode table (%llu-%llu) overlaps GDT table "
-			     "(%llu-%llu)",
+		ext4_warning(sb, "Inode table (%llu-%llu) overlaps GDT table (%llu-%llu)",
 			     (unsigned long long)input->inode_table,
 			     itend - 1, start, metaend - 1);
 	else
@@ -648,8 +644,7 @@ static int verify_reserved_gdb(struct super_block *sb,
 	while ((grp = ext4_list_backups(sb, &three, &five, &seven)) < end) {
 		if (le32_to_cpu(*p++) !=
 		    grp * EXT4_BLOCKS_PER_GROUP(sb) + blk){
-			ext4_warning(sb, "reserved GDT %llu"
-				     " missing grp %d (%llu)",
+			ext4_warning(sb, "reserved GDT %llu missing grp %d (%llu)",
 				     blk, grp,
 				     grp *
 				     (ext4_fsblk_t)EXT4_BLOCKS_PER_GROUP(sb) +
@@ -861,8 +856,7 @@ static int reserve_backup_gdb(handle_t *handle, struct inode *inode,
 	/* Get each reserved primary GDT block and verify it holds backups */
 	for (res = 0; res < reserved_gdb; res++, blk++) {
 		if (le32_to_cpu(*data) != blk) {
-			ext4_warning(sb, "reserved block %llu"
-				     " not at offset %ld",
+			ext4_warning(sb, "reserved block %llu not at offset %ld",
 				     blk,
 				     (long)(data - (__le32 *)dind->b_data));
 			err = -EINVAL;
@@ -1009,8 +1003,8 @@ static void update_backups(struct super_block *sb,
 	 */
 exit_err:
 	if (err) {
-		ext4_warning(sb, "can't update backup for group %u (err %d), "
-			     "forcing fsck on next reboot", group, err);
+		ext4_warning(sb, "can't update backup for group %u (err %d), forcing fsck on next reboot",
+			     group, err);
 		sbi->s_mount_state &= ~EXT4_VALID_FS;
 		sbi->s_es->s_state &= cpu_to_le16(~EXT4_VALID_FS);
 		mark_buffer_dirty(sbi->s_sbh);
fs/ext4/super.c +157 -174

769 lines not on this page · raw message

Signature · 1 line
1.7.8.111.gad25c.dirty
maintainer
#64 lore raw

Re: [PATCH 7/9] ext4: Format neatening for easier grep

I've removed the terminating \n from ext4_msg and ext4_error (from another patch) and created a separate patch just for that.

The rest just causes way too much code churn. NACK.

- Ted

Quoted 1,670 lines · Joe Perches

Split strings are difficult to grep so make it easier.

Coalesce strings. Add spaces between words in newly coalesced formats. Remove unnecessary terminating \n's from ext4_msg uses as ext4_msg already adds it. Add terminating \n's to ext4_debug uses. Use %s, __func__ instead of embedded function names.

---
 fs/ext4/ext4_jbd2.c   |    3 +-
 fs/ext4/extents.c     |   85 ++++++-------
 fs/ext4/file.c        |    3 +-
 fs/ext4/fsync.c       |    2 +-
 fs/ext4/inode.c       |   43 +++----
 fs/ext4/mballoc.c     |   93 +++++++-------
 fs/ext4/mmp.c         |   23 ++--
 fs/ext4/move_extent.c |   86 +++++--------
 fs/ext4/namei.c       |    4 +-
 fs/ext4/page-io.c     |   12 +--
 fs/ext4/resize.c      |   22 ++--
 fs/ext4/super.c       |  331 +++++++++++++++++++++++--------------------------
 12 files changed, 321 insertions(+), 386 deletions(-)
fs/ext4/ext4_jbd2.c +1 -2
index aca1790..5365b66 100644
@@ -43,8 +43,7 @@ int __ext4_forget(const char *where, unsigned int line, handle_t *handle,
 	trace_ext4_forget(inode, is_metadata, blocknr);
 	BUFFER_TRACE(bh, "enter");
 
-	jbd_debug(4, "forgetting bh %p: is_metadata = %d, mode %o, "
-		  "data mode %x\n",
+	jbd_debug(4, "forgetting bh %p: is_metadata = %d, mode %o, data mode %x\n",
 		  bh, is_metadata, inode->i_mode,
 		  test_opt(inode->i_sb, DATA_FLAGS));
 
fs/ext4/extents.c +39 -46
index c39a737..70b9419 100644
@@ -760,9 +760,8 @@ static int ext4_ext_insert_index(handle_t *handle, struct inode *inode,
 	len = EXT_LAST_INDEX(curp->p_hdr) - ix + 1;
 	BUG_ON(len < 0);
 	if (len > 0) {
-		ext_debug("insert new index %d: "
-				"move %d indices from 0x%p to 0x%p\n",
-				logical, len, ix, ix + 1);
+		ext_debug("insert new index %d: move %d indices from 0x%p to 0x%p\n",
+			  logical, len, ix, ix + 1);
 		memmove(ix + 1, ix, len * sizeof(struct ext4_extent_idx));
 	}
 
@@ -822,14 +821,12 @@ static int ext4_ext_split(handle_t *handle, struct inode *inode,
 	}
 	if (path[depth].p_ext != EXT_MAX_EXTENT(path[depth].p_hdr)) {
 		border = path[depth].p_ext[1].ee_block;
-		ext_debug("leaf will be split."
-				" next leaf starts at %d\n",
-				  le32_to_cpu(border));
+		ext_debug("leaf will be split. next leaf starts at %d\n",
+			  le32_to_cpu(border));
 	} else {
 		border = newext->ee_block;
-		ext_debug("leaf will be added."
-				" next leaf starts at %d\n",
-				le32_to_cpu(border));
+		ext_debug("leaf will be added. next leaf starts at %d\n",
+			  le32_to_cpu(border));
 	}
 
 	/*
@@ -1771,34 +1768,31 @@ has_space:
 		if (le32_to_cpu(newext->ee_block)
 			   > le32_to_cpu(nearex->ee_block)) {
 			/* Insert after */
-			ext_debug("insert %u:%llu:[%d]%d before: "
-					"nearest %p\n",
-					le32_to_cpu(newext->ee_block),
-					ext4_ext_pblock(newext),
-					ext4_ext_is_uninitialized(newext),
-					ext4_ext_get_actual_len(newext),
-					nearex);
+			ext_debug("insert %u:%llu:[%d]%d before: nearest %p\n",
+				  le32_to_cpu(newext->ee_block),
+				  ext4_ext_pblock(newext),
+				  ext4_ext_is_uninitialized(newext),
+				  ext4_ext_get_actual_len(newext),
+				  nearex);
 			nearex++;
 		} else {
 			/* Insert before */
 			BUG_ON(newext->ee_block == nearex->ee_block);
-			ext_debug("insert %u:%llu:[%d]%d after: "
-					"nearest %p\n",
-					le32_to_cpu(newext->ee_block),
-					ext4_ext_pblock(newext),
-					ext4_ext_is_uninitialized(newext),
-					ext4_ext_get_actual_len(newext),
-					nearex);
+			ext_debug("insert %u:%llu:[%d]%d after: nearest %p\n",
+				  le32_to_cpu(newext->ee_block),
+				  ext4_ext_pblock(newext),
+				  ext4_ext_is_uninitialized(newext),
+				  ext4_ext_get_actual_len(newext),
+				  nearex);
 		}
 		len = EXT_LAST_EXTENT(eh) - nearex + 1;
 		if (len > 0) {
-			ext_debug("insert %u:%llu:[%d]%d: "
-					"move %d extents from 0x%p to 0x%p\n",
-					le32_to_cpu(newext->ee_block),
-					ext4_ext_pblock(newext),
-					ext4_ext_is_uninitialized(newext),
-					ext4_ext_get_actual_len(newext),
-					len, nearex, nearex + 1);
+			ext_debug("insert %u:%llu:[%d]%d: move %d extents from 0x%p to 0x%p\n",
+				  le32_to_cpu(newext->ee_block),
+				  ext4_ext_pblock(newext),
+				  ext4_ext_is_uninitialized(newext),
+				  ext4_ext_get_actual_len(newext),
+				  len, nearex, nearex + 1);
 			memmove(nearex + 1, nearex,
 				len * sizeof(struct ext4_extent));
 		}
@@ -2754,8 +2748,8 @@ static int ext4_split_extent_at(handle_t *handle,
 	unsigned int ee_len, depth;
 	int err = 0;
 
-	ext_debug("ext4_split_extents_at: inode %lu, logical"
-		"block %llu\n", inode->i_ino, (unsigned long long)split);
+	ext_debug("ext4_split_extents_at: inode %lu, logicalblock %llu\n",
+		  inode->i_ino, (unsigned long long)split);
 
 	ext4_ext_show_leaf(inode, path);
 
@@ -2936,9 +2930,9 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
 	int err = 0;
 	int split_flag = 0;
 
-	ext_debug("ext4_ext_convert_to_initialized: inode %lu, logical"
-		"block %llu, max_blocks %u\n", inode->i_ino,
-		(unsigned long long)map->m_lblk, map->m_len);
+	ext_debug("%s: inode %lu, logicalblock %llu, max_blocks %u\n",
+		  __func__, inode->i_ino,
+		  (unsigned long long)map->m_lblk, map->m_len);
 
 	eof_block = (inode->i_size + inode->i_sb->s_blocksize - 1) >>
 		inode->i_sb->s_blocksize_bits;
@@ -3143,9 +3137,9 @@ static int ext4_split_unwritten_extents(handle_t *handle,
 	unsigned int ee_len;
 	int split_flag = 0, depth;
 
-	ext_debug("ext4_split_unwritten_extents: inode %lu, logical"
-		"block %llu, max_blocks %u\n", inode->i_ino,
-		(unsigned long long)map->m_lblk, map->m_len);
+	ext_debug("%s: inode %lu, logicalblock %llu, max_blocks %u\n",
+		  __func__, inode->i_ino,
+		  (unsigned long long)map->m_lblk, map->m_len);
 
 	eof_block = (inode->i_size + inode->i_sb->s_blocksize - 1) >>
 		inode->i_sb->s_blocksize_bits;
@@ -3178,10 +3172,10 @@ static int ext4_convert_unwritten_extents_endio(handle_t *handle,
 	depth = ext_depth(inode);
 	ex = path[depth].p_ext;
 
-	ext_debug("ext4_convert_unwritten_extents_endio: inode %lu, logical"
-		"block %llu, max_blocks %u\n", inode->i_ino,
-		(unsigned long long)le32_to_cpu(ex->ee_block),
-		ext4_ext_get_actual_len(ex));
+	ext_debug("%s: inode %lu, logicalblock %llu, max_blocks %u\n",
+		  __func__, inode->i_ino,
+		  (unsigned long long)le32_to_cpu(ex->ee_block),
+		  ext4_ext_get_actual_len(ex));
 
 	err = ext4_ext_get_access(handle, inode, path + depth);
 	if (err)
@@ -3458,10 +3452,9 @@ ext4_ext_handle_uninitialized_extents(handle_t *handle, struct inode *inode,
 	int err = 0;
 	ext4_io_end_t *io = EXT4_I(inode)->cur_aio_dio;
 
-	ext_debug("ext4_ext_handle_uninitialized_extents: inode %lu, logical "
-		  "block %llu, max_blocks %u, flags %x, allocated %u\n",
-		  inode->i_ino, (unsigned long long)map->m_lblk, map->m_len,
-		  flags, allocated);
+	ext_debug("%s: inode %lu, logical block %llu, max_blocks %u, flags %x, allocated %u\n",
+		  __func__, inode->i_ino, (unsigned long long)map->m_lblk,
+		  map->m_len, flags, allocated);
 	ext4_ext_show_leaf(inode, path);
 
 	trace_ext4_ext_handle_uninitialized_extents(inode, map, allocated,
fs/ext4/file.c +1 -2
index cb70f18..a979bcd 100644
@@ -126,8 +126,7 @@ ext4_file_write(struct kiocb *iocb, const struct iovec *iov,
 		/* Warn about this once per day */
 		if (printk_timed_ratelimit(&unaligned_warn_time, 60*60*24*HZ))
 			ext4_msg(inode->i_sb, KERN_WARNING,
-				 "Unaligned AIO/DIO on inode %ld by %s; "
-				 "performance will be poor.",
+				 "Unaligned AIO/DIO on inode %ld by %s; performance will be poor",
 				 inode->i_ino, current->comm);
 		mutex_lock(ext4_aio_mutex(inode));
 		ext4_aiodio_wait(inode);
fs/ext4/fsync.c +1 -1
index bb6c7d8..3f87aca 100644
@@ -46,7 +46,7 @@ static void dump_completed_IO(struct inode * inode)
 		return;
 	}
 
-	ext4_debug("Dump inode %lu completed_io list \n", inode->i_ino);
+	ext4_debug("Dump inode %lu completed_io list\n", inode->i_ino);
 	spin_lock_irqsave(&EXT4_I(inode)->i_completed_io_lock, flags);
 	list_for_each_entry(io, &EXT4_I(inode)->i_completed_io_list, list){
 		cur = &io->list;
fs/ext4/inode.c +20 -23
index a2a1ae4..2fbfee2 100644
@@ -273,8 +273,8 @@ void ext4_da_update_reserve_space(struct inode *inode,
 	spin_lock(&ei->i_block_reservation_lock);
 	trace_ext4_da_update_reserve_space(inode, used, quota_claim);
 	if (unlikely(used > ei->i_reserved_data_blocks)) {
-		ext4_msg(inode->i_sb, KERN_NOTICE, "%s: ino %lu, used %d "
-			 "with only %d reserved data blocks\n",
+		ext4_msg(inode->i_sb, KERN_NOTICE,
+			 "%s: ino %lu, used %d with only %d reserved data blocks",
 			 __func__, inode->i_ino, used,
 			 ei->i_reserved_data_blocks);
 		WARN_ON(1);
@@ -472,9 +472,9 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
 	int retval;
 
 	map->m_flags = 0;
-	ext_debug("ext4_map_blocks(): inode %lu, flag %d, max_blocks %u,"
-		  "logical block %lu\n", inode->i_ino, flags, map->m_len,
-		  (unsigned long) map->m_lblk);
+	ext_debug("%s: inode %lu, flag %d, max_blocks %u, logical block %lu\n",
+		  __func__, inode->i_ino, flags, map->m_len,
+		  (unsigned long)map->m_lblk);
 	/*
 	 * Try to see if we can get the block without requesting a new
 	 * file system block.
@@ -1165,9 +1165,9 @@ static void ext4_da_release_space(struct inode *inode, int to_free)
 		 * function is called from invalidate page, it's
 		 * harmless to return without any action.
 		 */
-		ext4_msg(inode->i_sb, KERN_NOTICE, "ext4_da_release_space: "
-			 "ino %lu, to_free %d with only %d reserved "
-			 "data blocks\n", inode->i_ino, to_free,
+		ext4_msg(inode->i_sb, KERN_NOTICE,
+			 "%s: ino %lu, to_free %d with only %d reserved data blocks",
+			 __func__, inode->i_ino, to_free,
 			 ei->i_reserved_data_blocks);
 		WARN_ON(1);
 		to_free = ei->i_reserved_data_blocks;
@@ -1532,13 +1532,12 @@ static void mpage_da_map_and_submit(struct mpage_da_data *mpd)
 		 */
 		if (!(EXT4_SB(sb)->s_mount_flags & EXT4_MF_FS_ABORTED)) {
 			ext4_msg(sb, KERN_CRIT,
-				 "delayed block allocation failed for inode %lu "
-				 "at logical offset %llu with max blocks %zd "
-				 "with error %d", mpd->inode->i_ino,
+				 "delayed block allocation failed for inode %lu at logical offset %llu with max blocks %zd with error %d",
+				 mpd->inode->i_ino,
 				 (unsigned long long) next,
 				 mpd->b_size >> mpd->inode->i_blkbits, err);
 			ext4_msg(sb, KERN_CRIT,
-				"This should not happen!! Data will be lost\n");
+				"This should not happen!! Data will be lost");
 			if (err == -ENOSPC)
 				ext4_print_free_blocks(mpd->inode);
 		}
@@ -1689,9 +1688,9 @@ static int ext4_da_map_blocks(struct inode *inode, sector_t iblock,
 		invalid_block = ~0;
 
 	map->m_flags = 0;
-	ext_debug("ext4_da_map_blocks(): inode %lu, max_blocks %u,"
-		  "logical block %lu\n", inode->i_ino, map->m_len,
-		  (unsigned long) map->m_lblk);
+	ext_debug("%s: inode %lu, max_blocks %u, logical block %lu\n",
+		  __func__, inode->i_ino, map->m_len,
+		  (unsigned long)map->m_lblk);
 	/*
 	 * Try to see if we can get the block without requesting a new
 	 * file system block.
@@ -2274,9 +2273,9 @@ retry:
 		handle = ext4_journal_start(inode, needed_blocks);
 		if (IS_ERR(handle)) {
 			ret = PTR_ERR(handle);
-			ext4_msg(inode->i_sb, KERN_CRIT, "%s: jbd2_start: "
-			       "%ld pages, ino %lu; err %d", __func__,
-				wbc->nr_to_write, inode->i_ino, ret);
+			ext4_msg(inode->i_sb, KERN_CRIT,
+				 "%s: jbd2_start: %ld pages, ino %lu; err %d",
+				 __func__, wbc->nr_to_write, inode->i_ino, ret);
 			blk_finish_plug(&plug);
 			goto out_writepages;
 		}
@@ -2765,9 +2764,8 @@ static void ext4_end_io_dio(struct kiocb *iocb, loff_t offset,
 	if (!io_end || !size)
 		goto out;
 
-	ext_debug("ext4_end_io_dio(): io_end 0x%p "
-		  "for inode %lu, iocb 0x%p, offset %llu, size %zd\n",
- 		  iocb->private, io_end->inode->i_ino, iocb, offset,
+	ext_debug("%s: io_end 0x%p for inode %lu, iocb 0x%p, offset %llu, size %zd\n",
+		  __func__, iocb->private, io_end->inode->i_ino, iocb, offset,
 		  size);
 
 	iocb->private = NULL;
@@ -4445,8 +4443,7 @@ int ext4_mark_inode_dirty(handle_t *handle, struct inode *inode)
 				if (mnt_count !=
 					le16_to_cpu(sbi->s_es->s_mnt_count)) {
 					ext4_warning(inode->i_sb,
-					"Unable to expand inode %lu. Delete"
-					" some EAs or run e2fsck.",
+					"Unable to expand inode %lu. Delete some EAs or run e2fsck.",
 					inode->i_ino);
 					mnt_count =
 					  le16_to_cpu(sbi->s_es->s_mnt_count);
fs/ext4/mballoc.c +44 -49
index cd4d7c6..70b22fb 100644
@@ -499,9 +499,7 @@ static void mb_cmp_bitmaps(struct ext4_buddy *e4b, void *bitmap)
 		for (i = 0; i < e4b->bd_sb->s_blocksize; i++) {
 			if (b1[i] != b2[i]) {
 				ext4_msg(e4b->bd_sb, KERN_ERR,
-					 "corruption in group %u "
-					 "at byte %u(%u): %x in copy != %x "
-					 "on disk/prealloc",
+					 "corruption in group %u at byte %u(%u): %x in copy != %x on disk/prealloc",
 					 e4b->bd_group, i, i * 8, b1[i], b2[i]);
 				BUG();
 			}
@@ -2178,8 +2176,8 @@ int ext4_mb_add_groupinfo(struct super_block *sb, ext4_group_t group,
 			EXT4_DESC_PER_BLOCK_BITS(sb);
 		meta_group_info = kmalloc(metalen, GFP_KERNEL);
 		if (meta_group_info == NULL) {
-			ext4_msg(sb, KERN_ERR, "can't allocate mem "
-				 "for a buddy group");
+			ext4_msg(sb, KERN_ERR,
+				 "can't allocate mem for a buddy group");
 			goto exit_meta_group_info;
 		}
 		sbi->s_group_info[group >> EXT4_DESC_PER_BLOCK_BITS(sb)] =
@@ -2541,26 +2539,25 @@ int ext4_mb_release(struct super_block *sb)
 		iput(sbi->s_buddy_cache);
 	if (sbi->s_mb_stats) {
 		ext4_msg(sb, KERN_INFO,
-		       "mballoc: %u blocks %u reqs (%u success)",
-				atomic_read(&sbi->s_bal_allocated),
-				atomic_read(&sbi->s_bal_reqs),
-				atomic_read(&sbi->s_bal_success));
+			 "mballoc: %u blocks %u reqs (%u success)",
+			 atomic_read(&sbi->s_bal_allocated),
+			 atomic_read(&sbi->s_bal_reqs),
+			 atomic_read(&sbi->s_bal_success));
 		ext4_msg(sb, KERN_INFO,
-		      "mballoc: %u extents scanned, %u goal hits, "
-				"%u 2^N hits, %u breaks, %u lost",
-				atomic_read(&sbi->s_bal_ex_scanned),
-				atomic_read(&sbi->s_bal_goals),
-				atomic_read(&sbi->s_bal_2orders),
-				atomic_read(&sbi->s_bal_breaks),
-				atomic_read(&sbi->s_mb_lost_chunks));
+			 "mballoc: %u extents scanned, %u goal hits, %u 2^N hits, %u breaks, %u lost",
+			 atomic_read(&sbi->s_bal_ex_scanned),
+			 atomic_read(&sbi->s_bal_goals),
+			 atomic_read(&sbi->s_bal_2orders),
+			 atomic_read(&sbi->s_bal_breaks),
+			 atomic_read(&sbi->s_mb_lost_chunks));
 		ext4_msg(sb, KERN_INFO,
-		       "mballoc: %lu generated and it took %Lu",
-				sbi->s_mb_buddies_generated,
-				sbi->s_mb_generation_time);
+			 "mballoc: %lu generated and it took %Lu",
+			 sbi->s_mb_buddies_generated,
+			 sbi->s_mb_generation_time);
 		ext4_msg(sb, KERN_INFO,
-		       "mballoc: %u preallocated, %u discarded",
-				atomic_read(&sbi->s_mb_preallocated),
-				atomic_read(&sbi->s_mb_discarded));
+			 "mballoc: %u preallocated, %u discarded",
+			 atomic_read(&sbi->s_mb_preallocated),
+			 atomic_read(&sbi->s_mb_discarded));
 	}
 
 	free_percpu(sbi->s_locality_groups);
@@ -3864,29 +3861,28 @@ static void ext4_mb_show_ac(struct ext4_allocation_context *ac)
 	    (EXT4_SB(sb)->s_mount_flags & EXT4_MF_FS_ABORTED))
 		return;
 
-	ext4_msg(ac->ac_sb, KERN_ERR, "Can't allocate:"
-			" Allocation context details:");
+	ext4_msg(ac->ac_sb, KERN_ERR,
+		 "Can't allocate: Allocation context details:");
 	ext4_msg(ac->ac_sb, KERN_ERR, "status %d flags %d",
-			ac->ac_status, ac->ac_flags);
-	ext4_msg(ac->ac_sb, KERN_ERR, "orig %lu/%lu/%lu@%lu, "
-		 	"goal %lu/%lu/%lu@%lu, "
-			"best %lu/%lu/%lu@%lu cr %d",
-			(unsigned long)ac->ac_o_ex.fe_group,
-			(unsigned long)ac->ac_o_ex.fe_start,
-			(unsigned long)ac->ac_o_ex.fe_len,
-			(unsigned long)ac->ac_o_ex.fe_logical,
-			(unsigned long)ac->ac_g_ex.fe_group,
-			(unsigned long)ac->ac_g_ex.fe_start,
-			(unsigned long)ac->ac_g_ex.fe_len,
-			(unsigned long)ac->ac_g_ex.fe_logical,
-			(unsigned long)ac->ac_b_ex.fe_group,
-			(unsigned long)ac->ac_b_ex.fe_start,
-			(unsigned long)ac->ac_b_ex.fe_len,
-			(unsigned long)ac->ac_b_ex.fe_logical,
-			(int)ac->ac_criteria);
+		 ac->ac_status, ac->ac_flags);
+	ext4_msg(ac->ac_sb, KERN_ERR,
+		 "orig %lu/%lu/%lu@%lu, goal %lu/%lu/%lu@%lu, best %lu/%lu/%lu@%lu cr %d",
+		 (unsigned long)ac->ac_o_ex.fe_group,
+		 (unsigned long)ac->ac_o_ex.fe_start,
+		 (unsigned long)ac->ac_o_ex.fe_len,
+		 (unsigned long)ac->ac_o_ex.fe_logical,
+		 (unsigned long)ac->ac_g_ex.fe_group,
+		 (unsigned long)ac->ac_g_ex.fe_start,
+		 (unsigned long)ac->ac_g_ex.fe_len,
+		 (unsigned long)ac->ac_g_ex.fe_logical,
+		 (unsigned long)ac->ac_b_ex.fe_group,
+		 (unsigned long)ac->ac_b_ex.fe_start,
+		 (unsigned long)ac->ac_b_ex.fe_len,
+		 (unsigned long)ac->ac_b_ex.fe_logical,
+		 (int)ac->ac_criteria);
 	ext4_msg(ac->ac_sb, KERN_ERR, "%lu scanned, %d found",
 		 ac->ac_ex_scanned, ac->ac_found);
-	ext4_msg(ac->ac_sb, KERN_ERR, "groups: ");
+	ext4_msg(ac->ac_sb, KERN_ERR, "groups:");
 	ngroups = ext4_get_groups_count(sb);
 	for (i = 0; i < ngroups; i++) {
 		struct ext4_group_info *grp = ext4_get_group_info(sb, i);
@@ -4017,13 +4013,12 @@ ext4_mb_initialize_context(struct ext4_allocation_context *ac,
 	 * locality group. this is a policy, actually */
 	ext4_mb_group_or_file(ac);
 
-	mb_debug(1, "init ac: %u blocks @ %u, goal %u, flags %x, 2^%d, "
-			"left: %u/%u, right %u/%u to %swritable\n",
-			(unsigned) ar->len, (unsigned) ar->logical,
-			(unsigned) ar->goal, ac->ac_flags, ac->ac_2order,
-			(unsigned) ar->lleft, (unsigned) ar->pleft,
-			(unsigned) ar->lright, (unsigned) ar->pright,
-			atomic_read(&ar->inode->i_writecount) ? "" : "non-");
+	mb_debug(1, "init ac: %u blocks @ %u, goal %u, flags %x, 2^%d, left: %u/%u, right %u/%u to %swritable\n",
+		 (unsigned) ar->len, (unsigned) ar->logical,
+		 (unsigned) ar->goal, ac->ac_flags, ac->ac_2order,
+		 (unsigned) ar->lleft, (unsigned) ar->pleft,
+		 (unsigned) ar->lright, (unsigned) ar->pright,
+		 atomic_read(&ar->inode->i_writecount) ? "" : "non-");
 	return 0;
 
 }
fs/ext4/mmp.c +10 -13
index ed6548d..1767b2a 100644
@@ -73,8 +73,7 @@ void __dump_mmp_msg(struct super_block *sb, struct mmp_struct *mmp,
 {
 	__ext4_warning(sb, function, line, msg);
 	__ext4_warning(sb, function, line,
-		       "MMP failure info: last update time: %llu, last update "
-		       "node: %s, last update device: %s\n",
+		       "MMP failure info: last update time: %llu, last update node: %s, last update device: %s",
 		       (long long unsigned int) le64_to_cpu(mmp->mmp_time),
 		       mmp->mmp_nodename, mmp->mmp_bdevname);
 }
@@ -133,15 +132,13 @@ static int kmmpd(void *data)
 
 		if (!(le32_to_cpu(es->s_feature_incompat) &
 		    EXT4_FEATURE_INCOMPAT_MMP)) {
-			ext4_warning(sb, "kmmpd being stopped since MMP feature"
-				     " has been disabled.");
+			ext4_warning(sb, "kmmpd being stopped since MMP feature has been disabled");
 			EXT4_SB(sb)->s_mmp_tsk = NULL;
 			goto failed;
 		}
 
 		if (sb->s_flags & MS_RDONLY) {
-			ext4_warning(sb, "kmmpd being stopped since filesystem "
-				     "has been remounted as readonly.");
+			ext4_warning(sb, "kmmpd being stopped since filesystem has been remounted as readonly");
 			EXT4_SB(sb)->s_mmp_tsk = NULL;
 			goto failed;
 		}
@@ -274,11 +271,11 @@ int ext4_multi_mount_protect(struct super_block *sb,
 
 	/* Print MMP interval if more than 20 secs. */
 	if (wait_time > EXT4_MMP_MIN_CHECK_INTERVAL * 4)
-		ext4_warning(sb, "MMP interval %u higher than expected, please"
-			     " wait.\n", wait_time * 2);
+		ext4_warning(sb, "MMP interval %u higher than expected, please wait",
+			     wait_time * 2);
 
 	if (schedule_timeout_interruptible(HZ * wait_time) != 0) {
-		ext4_warning(sb, "MMP startup interrupted, failing mount\n");
+		ext4_warning(sb, "MMP startup interrupted, failing mount");
 		goto failed;
 	}
 
@@ -288,7 +285,7 @@ int ext4_multi_mount_protect(struct super_block *sb,
 	mmp = (struct mmp_struct *)(bh->b_data);
 	if (seq != le32_to_cpu(mmp->mmp_seq)) {
 		dump_mmp_msg(sb, mmp,
-			     "Device is already active on another node.");
+			     "Device is already active on another node");
 		goto failed;
 	}
 
@@ -307,7 +304,7 @@ skip:
 	 * wait for MMP interval and check mmp_seq.
 	 */
 	if (schedule_timeout_interruptible(HZ * wait_time) != 0) {
-		ext4_warning(sb, "MMP startup interrupted, failing mount\n");
+		ext4_warning(sb, "MMP startup interrupted, failing mount");
 		goto failed;
 	}
 
@@ -317,7 +314,7 @@ skip:
 	mmp = (struct mmp_struct *)(bh->b_data);
 	if (seq != le32_to_cpu(mmp->mmp_seq)) {
 		dump_mmp_msg(sb, mmp,
-			     "Device is already active on another node.");
+			     "Device is already active on another node");
 		goto failed;
 	}
 
@@ -338,7 +335,7 @@ skip:
 	if (IS_ERR(EXT4_SB(sb)->s_mmp_tsk)) {
 		EXT4_SB(sb)->s_mmp_tsk = NULL;
 		kfree(mmpd_data);
-		ext4_warning(sb, "Unable to create kmmpd thread for %s.",
+		ext4_warning(sb, "Unable to create kmmpd thread for %s",
 			     sb->s_id);
 		goto failed;
 	}
fs/ext4/move_extent.c +34 -52
index c5826c6..ce52881 100644
@@ -952,8 +952,7 @@ mext_check_arguments(struct inode *orig_inode,
 	unsigned int blocksize = 1 << blkbits;
 
 	if (donor_inode->i_mode & (S_ISUID|S_ISGID)) {
-		ext4_debug("ext4 move extent: suid or sgid is set"
-			   " to donor file [ino:orig %lu, donor %lu]\n",
+		ext4_debug("ext4 move extent: suid or sgid is set to donor file [ino:orig %lu, donor %lu]\n",
 			   orig_inode->i_ino, donor_inode->i_ino);
 		return -EINVAL;
 	}
@@ -963,28 +962,26 @@ mext_check_arguments(struct inode *orig_inode,
 
 	/* Ext4 move extent does not support swapfile */
 	if (IS_SWAPFILE(orig_inode) || IS_SWAPFILE(donor_inode)) {
-		ext4_debug("ext4 move extent: The argument files should "
-			"not be swapfile [ino:orig %lu, donor %lu]\n",
-			orig_inode->i_ino, donor_inode->i_ino);
+		ext4_debug("ext4 move extent: The argument files should not be swapfile [ino:orig %lu, donor %lu]\n",
+			   orig_inode->i_ino, donor_inode->i_ino);
 		return -EINVAL;
 	}
 
 	/* Files should be in the same ext4 FS */
 	if (orig_inode->i_sb != donor_inode->i_sb) {
-		ext4_debug("ext4 move extent: The argument files "
-			"should be in same FS [ino:orig %lu, donor %lu]\n",
-			orig_inode->i_ino, donor_inode->i_ino);
+		ext4_debug("ext4 move extent: The argument files should be in same FS [ino:orig %lu, donor %lu]\n",
+			   orig_inode->i_ino, donor_inode->i_ino);
 		return -EINVAL;
 	}
 
 	/* Ext4 move extent supports only extent based file */
 	if (!(ext4_test_inode_flag(orig_inode, EXT4_INODE_EXTENTS))) {
-		ext4_debug("ext4 move extent: orig file is not extents "
-			"based file [ino:orig %lu]\n", orig_inode->i_ino);
+		ext4_debug("ext4 move extent: orig file is not extents based file [ino:orig %lu]\n",
+			   orig_inode->i_ino);
 		return -EOPNOTSUPP;
 	} else if (!(ext4_test_inode_flag(donor_inode, EXT4_INODE_EXTENTS))) {
-		ext4_debug("ext4 move extent: donor file is not extents "
-			"based file [ino:donor %lu]\n", donor_inode->i_ino);
+		ext4_debug("ext4 move extent: donor file is not extents based file [ino:donor %lu]\n",
+			   donor_inode->i_ino);
 		return -EOPNOTSUPP;
 	}
 
@@ -995,9 +992,8 @@ mext_check_arguments(struct inode *orig_inode,
 
 	/* Start offset should be same */
 	if (orig_start != donor_start) {
-		ext4_debug("ext4 move extent: orig and donor's start "
-			"offset are not same [ino:orig %lu, donor %lu]\n",
-			orig_inode->i_ino, donor_inode->i_ino);
+		ext4_debug("ext4 move extent: orig and donor's start offset are not same [ino:orig %lu, donor %lu]\n",
+			   orig_inode->i_ino, donor_inode->i_ino);
 		return -EINVAL;
 	}
 
@@ -1005,9 +1001,9 @@ mext_check_arguments(struct inode *orig_inode,
 	    (donor_start >= EXT_MAX_BLOCKS) ||
 	    (*len > EXT_MAX_BLOCKS) ||
 	    (orig_start + *len >= EXT_MAX_BLOCKS))  {
-		ext4_debug("ext4 move extent: Can't handle over [%u] blocks "
-			"[ino:orig %lu, donor %lu]\n", EXT_MAX_BLOCKS,
-			orig_inode->i_ino, donor_inode->i_ino);
+		ext4_debug("ext4 move extent: Can't handle over [%u] blocks [ino:orig %lu, donor %lu]\n",
+			   EXT_MAX_BLOCKS,
+			   orig_inode->i_ino, donor_inode->i_ino);
 		return -EINVAL;
 	}
 
@@ -1015,51 +1011,40 @@ mext_check_arguments(struct inode *orig_inode,
 		donor_blocks = (donor_inode->i_size + blocksize - 1) >> blkbits;
 		/* TODO: eliminate this artificial restriction */
 		if (orig_start >= donor_blocks) {
-			ext4_debug("ext4 move extent: orig start offset "
-			"[%llu] should be less than donor file blocks "
-			"[%u] [ino:orig %lu, donor %lu]\n",
-			orig_start, donor_blocks,
-			orig_inode->i_ino, donor_inode->i_ino);
+			ext4_debug("ext4 move extent: orig start offset [%llu] should be less than donor file blocks [%u] [ino:orig %lu, donor %lu]\n",
+				   orig_start, donor_blocks,
+				   orig_inode->i_ino, donor_inode->i_ino);
 			return -EINVAL;
 		}
 
 		/* TODO: eliminate this artificial restriction */
 		if (orig_start + *len > donor_blocks) {
-			ext4_debug("ext4 move extent: End offset [%llu] should "
-				"be less than donor file blocks [%u]."
-				"So adjust length from %llu to %llu "
-				"[ino:orig %lu, donor %lu]\n",
-				orig_start + *len, donor_blocks,
-				*len, donor_blocks - orig_start,
-				orig_inode->i_ino, donor_inode->i_ino);
+			ext4_debug("ext4 move extent: End offset [%llu] should be less than donor file blocks [%u].So adjust length from %llu to %llu [ino:orig %lu, donor %lu]\n",
+				   orig_start + *len, donor_blocks,
+				   *len, donor_blocks - orig_start,
+				   orig_inode->i_ino, donor_inode->i_ino);
 			*len = donor_blocks - orig_start;
 		}
 	} else {
 		orig_blocks = (orig_inode->i_size + blocksize - 1) >> blkbits;
 		if (orig_start >= orig_blocks) {
-			ext4_debug("ext4 move extent: start offset [%llu] "
-				"should be less than original file blocks "
-				"[%u] [ino:orig %lu, donor %lu]\n",
-				 orig_start, orig_blocks,
-				orig_inode->i_ino, donor_inode->i_ino);
+			ext4_debug("ext4 move extent: start offset [%llu] should be less than original file blocks [%u] [ino:orig %lu, donor %lu]\n",
+				   orig_start, orig_blocks,
+				   orig_inode->i_ino, donor_inode->i_ino);
 			return -EINVAL;
 		}
 
 		if (orig_start + *len > orig_blocks) {
-			ext4_debug("ext4 move extent: Adjust length "
-				"from %llu to %llu. Because it should be "
-				"less than original file blocks "
-				"[ino:orig %lu, donor %lu]\n",
-				*len, orig_blocks - orig_start,
-				orig_inode->i_ino, donor_inode->i_ino);
+			ext4_debug("ext4 move extent: Adjust length from %llu to %llu. Because it should be less than original file blocks [ino:orig %lu, donor %lu]\n",
+				   *len, orig_blocks - orig_start,
+				   orig_inode->i_ino, donor_inode->i_ino);
 			*len = orig_blocks - orig_start;
 		}
 	}
 
 	if (!*len) {
-		ext4_debug("ext4 move extent: len should not be 0 "
-			"[ino:orig %lu, donor %lu]\n", orig_inode->i_ino,
-			donor_inode->i_ino);
+		ext4_debug("ext4 move extent: len should not be 0 [ino:orig %lu, donor %lu]\n",
+			   orig_inode->i_ino, donor_inode->i_ino);
 		return -EINVAL;
 	}
 
@@ -1195,17 +1180,15 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp,
 
 	/* orig and donor should be different file */
 	if (orig_inode->i_ino == donor_inode->i_ino) {
-		ext4_debug("ext4 move extent: The argument files should not "
-			"be same file [ino:orig %lu, donor %lu]\n",
-			orig_inode->i_ino, donor_inode->i_ino);
+		ext4_debug("ext4 move extent: The argument files should not be same file [ino:orig %lu, donor %lu]\n",
+			   orig_inode->i_ino, donor_inode->i_ino);
 		return -EINVAL;
 	}
 
 	/* Regular file check */
 	if (!S_ISREG(orig_inode->i_mode) || !S_ISREG(donor_inode->i_mode)) {
-		ext4_debug("ext4 move extent: The argument files should be "
-			"regular file [ino:orig %lu, donor %lu]\n",
-			orig_inode->i_ino, donor_inode->i_ino);
+		ext4_debug("ext4 move extent: The argument files should be regular file [ino:orig %lu, donor %lu]\n",
+			   orig_inode->i_ino, donor_inode->i_ino);
 		return -EINVAL;
 	}
 
@@ -1270,8 +1253,7 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp,
 
 	/* No blocks within the specified range. */
 	if (le32_to_cpu(ext_cur->ee_block) > block_end) {
-		ext4_debug("ext4 move extent: The specified range of file "
-							"may be the hole\n");
+		ext4_debug("ext4 move extent: The specified range of file may be the hole\n");
 		ret1 = -EINVAL;
 		goto out;
 	}
fs/ext4/namei.c +2 -2
index b10ee89..99f86a7 100644
@@ -474,8 +474,8 @@ fail2:
 fail:
 	if (*err == ERR_BAD_DX_DIR)
 		ext4_warning(dir->i_sb,
-			     "Corrupt dir inode %lu, running e2fsck is "
-			     "recommended.", dir->i_ino);
+			     "Corrupt dir inode %lu, running e2fsck is recommended.",
+			     dir->i_ino);
 	return NULL;
 }
 
fs/ext4/page-io.c +4 -8
index 09c9d4b..a5afbad 100644
@@ -95,16 +95,13 @@ int ext4_end_io_nolock(ext4_io_end_t *io)
 	ssize_t size = io->size;
 	int ret = 0;
 
-	ext4_debug("ext4_end_io_nolock: io 0x%p from inode %lu,list->next 0x%p,"
-		   "list->prev 0x%p\n",
-		   io, inode->i_ino, io->list.next, io->list.prev);
+	ext4_debug("%s: io 0x%p from inode %lu,list->next 0x%p,list->prev 0x%p\n",
+		   __func__, io, inode->i_ino, io->list.next, io->list.prev);
 
 	ret = ext4_convert_unwritten_extents(inode, offset, size);
 	if (ret < 0) {
 		ext4_msg(inode->i_sb, KERN_EMERG,
-			 "failed to convert unwritten extents to written "
-			 "extents -- potential data loss!  "
-			 "(inode %lu, offset %llu, size %zd, error %d)",
+			 "failed to convert unwritten extents to written extents -- potential data loss!  (inode %lu, offset %llu, size %zd, error %d)",
 			 inode->i_ino, offset, size, ret);
 	}
 
@@ -241,8 +238,7 @@ static void ext4_end_bio(struct bio *bio, int error)
 
 	if (error) {
 		io_end->flag |= EXT4_IO_END_ERROR;
-		ext4_warning(inode->i_sb, "I/O error writing to inode %lu "
-			     "(offset %llu size %ld starting block %llu)",
+		ext4_warning(inode->i_sb, "I/O error writing to inode %lu (offset %llu size %ld starting block %llu)",
 			     inode->i_ino,
 			     (unsigned long long) io_end->offset,
 			     (long) io_end->size,
fs/ext4/resize.c +8 -14
index 83884e8..ed3a306 100644
@@ -29,8 +29,7 @@ int ext4_resize_begin(struct super_block *sb)
 	 * with error, because it can destroy the filesystem easily.
 	 */
 	if (EXT4_SB(sb)->s_mount_state & EXT4_ERROR_FS) {
-		ext4_warning(sb, "There are errors in the filesystem, "
-			     "so online resizing is not allowed\n");
+		ext4_warning(sb, "There are errors in the filesystem, so online resizing is not allowed");
 		return -EPERM;
 	}
 
@@ -104,13 +103,11 @@ static int verify_group_input(struct super_block *sb,
 		ext4_warning(sb, "Block bitmap same as inode bitmap (%llu)",
 			     (unsigned long long)input->block_bitmap);
 	else if (inside(input->block_bitmap, input->inode_table, itend))
-		ext4_warning(sb, "Block bitmap (%llu) in inode table "
-			     "(%llu-%llu)",
+		ext4_warning(sb, "Block bitmap (%llu) in inode table (%llu-%llu)",
 			     (unsigned long long)input->block_bitmap,
 			     (unsigned long long)input->inode_table, itend - 1);
 	else if (inside(input->inode_bitmap, input->inode_table, itend))
-		ext4_warning(sb, "Inode bitmap (%llu) in inode table "
-			     "(%llu-%llu)",
+		ext4_warning(sb, "Inode bitmap (%llu) in inode table (%llu-%llu)",
 			     (unsigned long long)input->inode_bitmap,
 			     (unsigned long long)input->inode_table, itend - 1);
 	else if (inside(input->block_bitmap, start, metaend))
@@ -123,8 +120,7 @@ static int verify_group_input(struct super_block *sb,
 			     start, metaend - 1);
 	else if (inside(input->inode_table, start, metaend) ||
 		 inside(itend - 1, start, metaend))
-		ext4_warning(sb, "Inode table (%llu-%llu) overlaps GDT table "
-			     "(%llu-%llu)",
+		ext4_warning(sb, "Inode table (%llu-%llu) overlaps GDT table (%llu-%llu)",
 			     (unsigned long long)input->inode_table,
 			     itend - 1, start, metaend - 1);
 	else
@@ -648,8 +644,7 @@ static int verify_reserved_gdb(struct super_block *sb,
 	while ((grp = ext4_list_backups(sb, &three, &five, &seven)) < end) {
 		if (le32_to_cpu(*p++) !=
 		    grp * EXT4_BLOCKS_PER_GROUP(sb) + blk){
-			ext4_warning(sb, "reserved GDT %llu"
-				     " missing grp %d (%llu)",
+			ext4_warning(sb, "reserved GDT %llu missing grp %d (%llu)",
 				     blk, grp,
 				     grp *
 				     (ext4_fsblk_t)EXT4_BLOCKS_PER_GROUP(sb) +
@@ -861,8 +856,7 @@ static int reserve_backup_gdb(handle_t *handle, struct inode *inode,
 	/* Get each reserved primary GDT block and verify it holds backups */
 	for (res = 0; res < reserved_gdb; res++, blk++) {
 		if (le32_to_cpu(*data) != blk) {
-			ext4_warning(sb, "reserved block %llu"
-				     " not at offset %ld",
+			ext4_warning(sb, "reserved block %llu not at offset %ld",
 				     blk,
 				     (long)(data - (__le32 *)dind->b_data));
 			err = -EINVAL;
@@ -1009,8 +1003,8 @@ static void update_backups(struct super_block *sb,
 	 */
 exit_err:
 	if (err) {
-		ext4_warning(sb, "can't update backup for group %u (err %d), "
-			     "forcing fsck on next reboot", group, err);
+		ext4_warning(sb, "can't update backup for group %u (err %d), forcing fsck on next reboot",
+			     group, err);
 		sbi->s_mount_state &= ~EXT4_VALID_FS;
 		sbi->s_es->s_state &= cpu_to_le16(~EXT4_VALID_FS);
 		mark_buffer_dirty(sbi->s_sbh);
fs/ext4/super.c +157 -174
index ac77a95..cade67b 100644
@@ -738,8 +738,7 @@ void ext4_update_dynamic_rev(struct super_block *sb)
 		return;
 
 	ext4_warning(sb,
-		     "updating to rev %d because of new feature flag, "
-		     "running e2fsck is recommended",
+		     "updating to rev %d because of new feature flag, running e2fsck is recommended",
 		     EXT4_DYNAMIC_REV);
 
 	es->s_first_ino = cpu_to_le32(EXT4_GOOD_OLD_FIRST_INO);
@@ -1297,27 +1296,26 @@ static int set_qf_name(struct super_block *sb, int qtype, substring_t *args)
 	if (sb_any_quota_loaded(sb) &&
 		!sbi->s_qf_names[qtype]) {
 		ext4_msg(sb, KERN_ERR,
-			"Cannot change journaled "
-			"quota options when quota turned on");
+			 "Cannot change journaled quota options when quota turned on");
 		return 0;
 	}
 	qname = match_strdup(args);
 	if (!qname) {
 		ext4_msg(sb, KERN_ERR,
-			"Not enough memory for storing quotafile name");
+			 "Not enough memory for storing quotafile name");
 		return 0;
 	}
 	if (sbi->s_qf_names[qtype] &&
 		strcmp(sbi->s_qf_names[qtype], qname)) {
 		ext4_msg(sb, KERN_ERR,
-			"%s quota file already specified", QTYPE2NAME(qtype));
+			 "%s quota file already specified", QTYPE2NAME(qtype));
 		kfree(qname);
 		return 0;
 	}
 	sbi->s_qf_names[qtype] = qname;
 	if (strchr(sbi->s_qf_names[qtype], '/')) {
 		ext4_msg(sb, KERN_ERR,
-			"quotafile must be on filesystem root");
+			 "quotafile must be on filesystem root");
 		kfree(sbi->s_qf_names[qtype]);
 		sbi->s_qf_names[qtype] = NULL;
 		return 0;
@@ -1333,8 +1331,8 @@ static int clear_qf_name(struct super_block *sb, int qtype)
 
 	if (sb_any_quota_loaded(sb) &&
 		sbi->s_qf_names[qtype]) {
-		ext4_msg(sb, KERN_ERR, "Cannot change journaled quota options"
-			" when quota turned on");
+		ext4_msg(sb, KERN_ERR,
+			 "Cannot change journaled quota options when quota turned on");
 		return 0;
 	}
 	/*
@@ -1494,8 +1492,8 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
 		if (m->flags & MOPT_CLEAR_ERR)
 			clear_opt(sb, ERRORS_MASK);
 		if (token == Opt_noquota && sb_any_quota_loaded(sb)) {
-			ext4_msg(sb, KERN_ERR, "Cannot change quota "
-				 "options when quota turned on");
+			ext4_msg(sb, KERN_ERR,
+				 "Cannot change quota options when quota turned on");
 			return -1;
 		}
 
@@ -1516,8 +1514,7 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
 				return -1;
 			if (arg && !is_power_of_2(arg)) {
 				ext4_msg(sb, KERN_ERR,
-					 "EXT4-fs: inode_readahead_blks"
-					 " must be a power of 2");
+					 "inode_readahead_blks must be a power of 2");
 				return -1;
 			}
 			sbi->s_inode_readahead_blks = arg;
@@ -1535,7 +1532,7 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
 				else if (test_opt(sb, DATA_FLAGS) !=
 					 m->mount_opt) {
 					ext4_msg(sb, KERN_ERR,
-					 "Cannot change data mode on remount");
+						 "Cannot change data mode on remount");
 					return -1;
 				}
 			} else {
@@ -1558,9 +1555,8 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
 		} else if (m->flags & MOPT_QFMT) {
 			if (sb_any_quota_loaded(sb) &&
 			    sbi->s_jquota_fmt != m->mount_opt) {
-				ext4_msg(sb, KERN_ERR, "Cannot "
-					 "change journaled quota options "
-					 "when quota turned on");
+				ext4_msg(sb, KERN_ERR,
+					 "Cannot change journaled quota options when quota turned on");
 				return -1;
 			}
 			sbi->s_jquota_fmt = m->mount_opt;
@@ -1583,8 +1579,8 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
 		}
 		return 1;
 	}
-	ext4_msg(sb, KERN_ERR, "Unrecognized mount option \"%s\" "
-		 "or missing value", opt);
+	ext4_msg(sb, KERN_ERR,
+		 "Unrecognized mount option \"%s\" or missing value", opt);
 	return -1;
 }
 
@@ -1623,21 +1619,20 @@ static int parse_options(char *options, struct super_block *sb,
 			clear_opt(sb, GRPQUOTA);
 
 		if (test_opt(sb, GRPQUOTA) || test_opt(sb, USRQUOTA)) {
-			ext4_msg(sb, KERN_ERR, "old and new quota "
-					"format mixing");
+			ext4_msg(sb, KERN_ERR,
+				 "old and new quota format mixing");
 			return 0;
 		}
 
 		if (!sbi->s_jquota_fmt) {
-			ext4_msg(sb, KERN_ERR, "journaled quota format "
-					"not specified");
+			ext4_msg(sb, KERN_ERR,
+				 "journaled quota format not specified");
 			return 0;
 		}
 	} else {
 		if (sbi->s_jquota_fmt) {
-			ext4_msg(sb, KERN_ERR, "journaled quota format "
-					"specified with no journaling "
-					"enabled");
+			ext4_msg(sb, KERN_ERR,
+				 "journaled quota format specified with no journaling enabled");
 			return 0;
 		}
 	}
@@ -1806,31 +1801,28 @@ static int ext4_setup_super(struct super_block *sb, struct ext4_super_block *es,
 	int res = 0;
 
 	if (le32_to_cpu(es->s_rev_level) > EXT4_MAX_SUPP_REV) {
-		ext4_msg(sb, KERN_ERR, "revision level too high, "
-			 "forcing read-only mode");
+		ext4_msg(sb, KERN_ERR,
+			 "revision level too high, forcing read-only mode");
 		res = MS_RDONLY;
 	}
 	if (read_only)
 		goto done;
 	if (!(sbi->s_mount_state & EXT4_VALID_FS))
-		ext4_msg(sb, KERN_WARNING, "warning: mounting unchecked fs, "
-			 "running e2fsck is recommended");
+		ext4_msg(sb, KERN_WARNING,
+			 "warning: mounting unchecked fs, running e2fsck is recommended");
 	else if ((sbi->s_mount_state & EXT4_ERROR_FS))
 		ext4_msg(sb, KERN_WARNING,
-			 "warning: mounting fs with errors, "
-			 "running e2fsck is recommended");
+			 "warning: mounting fs with errors, running e2fsck is recommended");
 	else if ((__s16) le16_to_cpu(es->s_max_mnt_count) > 0 &&
 		 le16_to_cpu(es->s_mnt_count) >=
 		 (unsigned short) (__s16) le16_to_cpu(es->s_max_mnt_count))
 		ext4_msg(sb, KERN_WARNING,
-			 "warning: maximal mount count reached, "
-			 "running e2fsck is recommended");
+			 "warning: maximal mount count reached, running e2fsck is recommended");
 	else if (le32_to_cpu(es->s_checkinterval) &&
 		(le32_to_cpu(es->s_lastcheck) +
 			le32_to_cpu(es->s_checkinterval) <= get_seconds()))
 		ext4_msg(sb, KERN_WARNING,
-			 "warning: checktime reached, "
-			 "running e2fsck is recommended");
+			 "warning: checktime reached, running e2fsck is recommended");
 	if (!sbi->s_journal)
 		es->s_state &= cpu_to_le16(~EXT4_VALID_FS);
 	if (!(__s16) le16_to_cpu(es->s_max_mnt_count))
@@ -1954,7 +1946,7 @@ static int ext4_check_descriptors(struct super_block *sb,
 	if (EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_FLEX_BG))
 		flexbg_flag = 1;
 
-	ext4_debug("Checking group descriptors");
+	ext4_debug("Checking group descriptors\n");
 
 	for (i = 0; i < sbi->s_groups_count; i++) {
 		struct ext4_group_desc *gdp = ext4_get_group_desc(sb, i, NULL);
@@ -1971,32 +1963,33 @@ static int ext4_check_descriptors(struct super_block *sb,
 
 		block_bitmap = ext4_block_bitmap(sb, gdp);
 		if (block_bitmap < first_block || block_bitmap > last_block) {
-			ext4_msg(sb, KERN_ERR, "ext4_check_descriptors: "
-			       "Block bitmap for group %u not in group "
-			       "(block %llu)!", i, block_bitmap);
+			ext4_msg(sb, KERN_ERR,
+				 "%s: Block bitmap for group %u not in group (block %llu)!",
+				 __func__, i, block_bitmap);
 			return 0;
 		}
 		inode_bitmap = ext4_inode_bitmap(sb, gdp);
 		if (inode_bitmap < first_block || inode_bitmap > last_block) {
-			ext4_msg(sb, KERN_ERR, "ext4_check_descriptors: "
-			       "Inode bitmap for group %u not in group "
-			       "(block %llu)!", i, inode_bitmap);
+			ext4_msg(sb, KERN_ERR,
+				 "%s: Inode bitmap for group %u not in group (block %llu)!",
+				 __func__, i, inode_bitmap);
 			return 0;
 		}
 		inode_table = ext4_inode_table(sb, gdp);
 		if (inode_table < first_block ||
 		    inode_table + sbi->s_itb_per_group - 1 > last_block) {
-			ext4_msg(sb, KERN_ERR, "ext4_check_descriptors: "
-			       "Inode table for group %u not in group "
-			       "(block %llu)!", i, inode_table);
+			ext4_msg(sb, KERN_ERR,
+				 "%s: Inode table for group %u not in group (block %llu)!",
+				 __func__, i, inode_table);
 			return 0;
 		}
 		ext4_lock_group(sb, i);
 		if (!ext4_group_desc_csum_verify(sbi, i, gdp)) {
-			ext4_msg(sb, KERN_ERR, "ext4_check_descriptors: "
-				 "Checksum for group %u failed (%u!=%u)",
-				 i, le16_to_cpu(ext4_group_desc_csum(sbi, i,
-				     gdp)), le16_to_cpu(gdp->bg_checksum));
+			ext4_msg(sb, KERN_ERR,
+				 "%s: Checksum for group %u failed (%u!=%u)",
+				 __func__, i,
+				 le16_to_cpu(ext4_group_desc_csum(sbi, i, gdp)),
+				 le16_to_cpu(gdp->bg_checksum));
 			if (!(sb->s_flags & MS_RDONLY)) {
 				ext4_unlock_group(sb, i);
 				return 0;
@@ -2046,22 +2039,21 @@ static void ext4_orphan_cleanup(struct super_block *sb,
 	}
 
 	if (bdev_read_only(sb->s_bdev)) {
-		ext4_msg(sb, KERN_ERR, "write access "
-			"unavailable, skipping orphan cleanup");
+		ext4_msg(sb, KERN_ERR,
+			 "write access unavailable, skipping orphan cleanup");
 		return;
 	}
 
 	/* Check if feature set would not allow a r/w mount */
 	if (!ext4_feature_set_ok(sb, 0)) {
-		ext4_msg(sb, KERN_INFO, "Skipping orphan cleanup due to "
-			 "unknown ROCOMPAT features");
+		ext4_msg(sb, KERN_INFO,
+			 "Skipping orphan cleanup due to unknown ROCOMPAT features");
 		return;
 	}
 
 	if (EXT4_SB(sb)->s_mount_state & EXT4_ERROR_FS) {
 		if (es->s_last_orphan)
-			jbd_debug(1, "Errors on filesystem, "
-				  "clearing orphan list.\n");
+			jbd_debug(1, "Errors on filesystem, clearing orphan list.\n");
 		es->s_last_orphan = 0;
 		jbd_debug(1, "Skipping orphan recovery on fs with errors.\n");
 		return;
@@ -2080,8 +2072,8 @@ static void ext4_orphan_cleanup(struct super_block *sb,
 			int ret = ext4_quota_on_mount(sb, i);
 			if (ret < 0)
 				ext4_msg(sb, KERN_ERR,
-					"Cannot turn on journaled "
-					"quota: error %d", ret);
+					"Cannot turn on journaled quota: error %d",
+					 ret);
 		}
 	}
 #endif
@@ -2099,16 +2091,16 @@ static void ext4_orphan_cleanup(struct super_block *sb,
 		dquot_initialize(inode);
 		if (inode->i_nlink) {
 			ext4_msg(sb, KERN_DEBUG,
-				"%s: truncating inode %lu to %lld bytes",
-				__func__, inode->i_ino, inode->i_size);
+				 "%s: truncating inode %lu to %lld bytes",
+				 __func__, inode->i_ino, inode->i_size);
 			jbd_debug(2, "truncating inode %lu to %lld bytes\n",
 				  inode->i_ino, inode->i_size);
 			ext4_truncate(inode);
 			nr_truncates++;
 		} else {
 			ext4_msg(sb, KERN_DEBUG,
-				"%s: deleting unreferenced inode %lu",
-				__func__, inode->i_ino);
+				 "%s: deleting unreferenced inode %lu",
+				 __func__, inode->i_ino);
 			jbd_debug(2, "deleting unreferenced inode %lu\n",
 				  inode->i_ino);
 			nr_orphans++;
@@ -2120,10 +2112,10 @@ static void ext4_orphan_cleanup(struct super_block *sb,
 
 	if (nr_orphans)
 		ext4_msg(sb, KERN_INFO, "%d orphan inode%s deleted",
-		       PLURAL(nr_orphans));
+			 PLURAL(nr_orphans));
 	if (nr_truncates)
 		ext4_msg(sb, KERN_INFO, "%d truncate%s cleaned up",
-		       PLURAL(nr_truncates));
+			 PLURAL(nr_truncates));
 #ifdef CONFIG_QUOTA
 	/* Turn quotas off */
 	for (i = 0; i < MAXQUOTAS; i++) {
@@ -2529,10 +2521,9 @@ static int ext4_feature_set_ok(struct super_block *sb, int readonly)
 {
 	if (EXT4_HAS_INCOMPAT_FEATURE(sb, ~EXT4_FEATURE_INCOMPAT_SUPP)) {
 		ext4_msg(sb, KERN_ERR,
-			"Couldn't mount because of "
-			"unsupported optional features (%x)",
-			(le32_to_cpu(EXT4_SB(sb)->s_es->s_feature_incompat) &
-			~EXT4_FEATURE_INCOMPAT_SUPP));
+			 "Couldn't mount because of unsupported optional features (%x)",
+			 (le32_to_cpu(EXT4_SB(sb)->s_es->s_feature_incompat) &
+			  ~EXT4_FEATURE_INCOMPAT_SUPP));
 		return 0;
 	}
 
@@ -2541,10 +2532,10 @@ static int ext4_feature_set_ok(struct super_block *sb, int readonly)
 
 	/* Check that feature set is OK for a read-write mount */
 	if (EXT4_HAS_RO_COMPAT_FEATURE(sb, ~EXT4_FEATURE_RO_COMPAT_SUPP)) {
-		ext4_msg(sb, KERN_ERR, "couldn't mount RDWR because of "
-			 "unsupported optional features (%x)",
+		ext4_msg(sb, KERN_ERR,
+			 "couldn't mount RDWR because of unsupported optional features (%x)",
 			 (le32_to_cpu(EXT4_SB(sb)->s_es->s_feature_ro_compat) &
-				~EXT4_FEATURE_RO_COMPAT_SUPP));
+			  ~EXT4_FEATURE_RO_COMPAT_SUPP));
 		return 0;
 	}
 	/*
@@ -2553,17 +2544,15 @@ static int ext4_feature_set_ok(struct super_block *sb, int readonly)
 	 */
 	if (EXT4_HAS_RO_COMPAT_FEATURE(sb, EXT4_FEATURE_RO_COMPAT_HUGE_FILE)) {
 		if (sizeof(blkcnt_t) < sizeof(u64)) {
-			ext4_msg(sb, KERN_ERR, "Filesystem with huge files "
-				 "cannot be mounted RDWR without "
-				 "CONFIG_LBDAF");
+			ext4_msg(sb, KERN_ERR,
+				 "Filesystem with huge files cannot be mounted RDWR without CONFIG_LBDAF");
 			return 0;
 		}
 	}
 	if (EXT4_HAS_RO_COMPAT_FEATURE(sb, EXT4_FEATURE_RO_COMPAT_BIGALLOC) &&
 	    !EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_EXTENTS)) {
 		ext4_msg(sb, KERN_ERR,
-			 "Can't support bigalloc feature without "
-			 "extents feature\n");
+			 "Can't support bigalloc feature without extents feature");
 		return 0;
 	}
 	return 1;
@@ -3105,13 +3094,13 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	if (test_opt(sb, DATA_FLAGS) == EXT4_MOUNT_JOURNAL_DATA) {
 		pr_warn_once("Warning: mounting with data=journal disables delayed allocation and O_DIRECT support!\n");
 		if (test_opt2(sb, EXPLICIT_DELALLOC)) {
-			ext4_msg(sb, KERN_ERR, "can't mount with "
-				 "both data=journal and delalloc");
+			ext4_msg(sb, KERN_ERR,
+				 "can't mount with both data=journal and delalloc");
 			goto failed_mount;
 		}
 		if (test_opt(sb, DIOREAD_NOLOCK)) {
-			ext4_msg(sb, KERN_ERR, "can't mount with "
-				 "both data=journal and delalloc");
+			ext4_msg(sb, KERN_ERR,
+				 "can't mount with both data=journal and delalloc");
 			goto failed_mount;
 		}
 		if (test_opt(sb, DELALLOC))
@@ -3121,8 +3110,8 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	blocksize = BLOCK_SIZE << le32_to_cpu(es->s_log_block_size);
 	if (test_opt(sb, DIOREAD_NOLOCK)) {
 		if (blocksize < PAGE_SIZE) {
-			ext4_msg(sb, KERN_ERR, "can't mount with "
-				 "dioread_nolock if block size != PAGE_SIZE");
+			ext4_msg(sb, KERN_ERR,
+				 "can't mount with dioread_nolock if block size != PAGE_SIZE");
 			goto failed_mount;
 		}
 	}
@@ -3135,27 +3124,26 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	     EXT4_HAS_RO_COMPAT_FEATURE(sb, ~0U) ||
 	     EXT4_HAS_INCOMPAT_FEATURE(sb, ~0U)))
 		ext4_msg(sb, KERN_WARNING,
-		       "feature flags set on rev 0 fs, "
-		       "running e2fsck is recommended");
+			 "feature flags set on rev 0 fs, running e2fsck is recommended");
 
 	if (IS_EXT2_SB(sb)) {
 		if (ext2_feature_set_ok(sb))
-			ext4_msg(sb, KERN_INFO, "mounting ext2 file system "
-				 "using the ext4 subsystem");
+			ext4_msg(sb, KERN_INFO,
+				 "mounting ext2 file system using the ext4 subsystem");
 		else {
-			ext4_msg(sb, KERN_ERR, "couldn't mount as ext2 due "
-				 "to feature incompatibilities");
+			ext4_msg(sb, KERN_ERR,
+				 "couldn't mount as ext2 due to feature incompatibilities");
 			goto failed_mount;
 		}
 	}
 
 	if (IS_EXT3_SB(sb)) {
 		if (ext3_feature_set_ok(sb))
-			ext4_msg(sb, KERN_INFO, "mounting ext3 file system "
-				 "using the ext4 subsystem");
+			ext4_msg(sb, KERN_INFO,
+				 "mounting ext3 file system using the ext4 subsystem");
 		else {
-			ext4_msg(sb, KERN_ERR, "couldn't mount as ext3 due "
-				 "to feature incompatibilities");
+			ext4_msg(sb, KERN_ERR,
+				 "couldn't mount as ext3 due to feature incompatibilities");
 			goto failed_mount;
 		}
 	}
@@ -3171,7 +3159,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	if (blocksize < EXT4_MIN_BLOCK_SIZE ||
 	    blocksize > EXT4_MAX_BLOCK_SIZE) {
 		ext4_msg(sb, KERN_ERR,
-		       "Unsupported filesystem blocksize %d", blocksize);
+			 "Unsupported filesystem blocksize %d", blocksize);
 		goto failed_mount;
 	}
 
@@ -3179,7 +3167,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 		/* Validate the filesystem blocksize */
 		if (!sb_set_blocksize(sb, blocksize)) {
 			ext4_msg(sb, KERN_ERR, "bad block size %d",
-					blocksize);
+				 blocksize);
 			goto failed_mount;
 		}
 
@@ -3189,14 +3177,14 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 		bh = sb_bread(sb, logical_sb_block);
 		if (!bh) {
 			ext4_msg(sb, KERN_ERR,
-			       "Can't read superblock on 2nd try");
+				 "Can't read superblock on 2nd try");
 			goto failed_mount;
 		}
 		es = (struct ext4_super_block *)(((char *)bh->b_data) + offset);
 		sbi->s_es = es;
 		if (es->s_magic != cpu_to_le16(EXT4_SUPER_MAGIC)) {
 			ext4_msg(sb, KERN_ERR,
-			       "Magic mismatch, very weird!");
+				 "Magic mismatch, very weird!");
 			goto failed_mount;
 		}
 	}
@@ -3217,8 +3205,8 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 		    (!is_power_of_2(sbi->s_inode_size)) ||
 		    (sbi->s_inode_size > blocksize)) {
 			ext4_msg(sb, KERN_ERR,
-			       "unsupported inode size: %d",
-			       sbi->s_inode_size);
+				 "unsupported inode size: %d",
+				 sbi->s_inode_size);
 			goto failed_mount;
 		}
 		if (sbi->s_inode_size > EXT4_GOOD_OLD_INODE_SIZE)
@@ -3231,8 +3219,8 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 		    sbi->s_desc_size > EXT4_MAX_DESC_SIZE ||
 		    !is_power_of_2(sbi->s_desc_size)) {
 			ext4_msg(sb, KERN_ERR,
-			       "unsupported descriptor size %lu",
-			       sbi->s_desc_size);
+				 "unsupported descriptor size %lu",
+				 sbi->s_desc_size);
 			goto failed_mount;
 		}
 	} else
@@ -3277,8 +3265,8 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	if (has_bigalloc) {
 		if (clustersize < blocksize) {
 			ext4_msg(sb, KERN_ERR,
-				 "cluster size (%d) smaller than "
-				 "block size (%d)", clustersize, blocksize);
+				 "cluster size (%d) smaller than block size (%d)",
+				 clustersize, blocksize);
 			goto failed_mount;
 		}
 		sbi->s_cluster_bits = le32_to_cpu(es->s_log_cluster_size) -
@@ -3293,17 +3281,16 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 		}
 		if (sbi->s_blocks_per_group !=
 		    (sbi->s_clusters_per_group * (clustersize / blocksize))) {
-			ext4_msg(sb, KERN_ERR, "blocks per group (%lu) and "
-				 "clusters per group (%lu) inconsistent",
+			ext4_msg(sb, KERN_ERR,
+				 "blocks per group (%lu) and clusters per group (%lu) inconsistent",
 				 sbi->s_blocks_per_group,
 				 sbi->s_clusters_per_group);
 			goto failed_mount;
 		}
 	} else {
 		if (clustersize != blocksize) {
-			ext4_warning(sb, "fragment/cluster size (%d) != "
-				     "block size (%d)", clustersize,
-				     blocksize);
+			ext4_warning(sb, "fragment/cluster size (%d) != block size (%d)",
+				     clustersize, blocksize);
 			clustersize = blocksize;
 		}
 		if (sbi->s_blocks_per_group > blocksize * 8) {
@@ -3319,8 +3306,8 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 
 	if (sbi->s_inodes_per_group > blocksize * 8) {
 		ext4_msg(sb, KERN_ERR,
-		       "#inodes per group too big: %lu",
-		       sbi->s_inodes_per_group);
+			 "#inodes per group too big: %lu",
+			 sbi->s_inodes_per_group);
 		goto failed_mount;
 	}
 
@@ -3331,8 +3318,8 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	err = generic_check_addressable(sb->s_blocksize_bits,
 					ext4_blocks_count(es));
 	if (err) {
-		ext4_msg(sb, KERN_ERR, "filesystem"
-			 " too large to mount safely on this system");
+		ext4_msg(sb, KERN_ERR,
+			 "filesystem too large to mount safely on this system");
 		if (sizeof(sector_t) < 8)
 			ext4_msg(sb, KERN_WARNING, "CONFIG_LBDAF not enabled");
 		ret = err;
@@ -3345,8 +3332,8 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	/* check blocks count against device size */
 	blocks_count = sb->s_bdev->bd_inode->i_size >> sb->s_blocksize_bits;
 	if (blocks_count && ext4_blocks_count(es) > blocks_count) {
-		ext4_msg(sb, KERN_WARNING, "bad geometry: block count %llu "
-		       "exceeds size of device (%llu blocks)",
+		ext4_msg(sb, KERN_WARNING,
+			 "bad geometry: block count %llu exceeds size of device (%llu blocks)",
 		       ext4_blocks_count(es), blocks_count);
 		goto failed_mount;
 	}
@@ -3356,8 +3343,8 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	 * of the filesystem.
 	 */
 	if (le32_to_cpu(es->s_first_data_block) >= ext4_blocks_count(es)) {
-		ext4_msg(sb, KERN_WARNING, "bad geometry: first data "
-			 "block %u is beyond end of filesystem (%llu)",
+		ext4_msg(sb, KERN_WARNING,
+			 "bad geometry: first data block %u is beyond end of filesystem (%llu)",
 			 le32_to_cpu(es->s_first_data_block),
 			 ext4_blocks_count(es));
 		goto failed_mount;
@@ -3367,12 +3354,12 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 			EXT4_BLOCKS_PER_GROUP(sb) - 1);
 	do_div(blocks_count, EXT4_BLOCKS_PER_GROUP(sb));
 	if (blocks_count > ((uint64_t)1<<32) - EXT4_DESC_PER_BLOCK(sb)) {
-		ext4_msg(sb, KERN_WARNING, "groups count too large: %u "
-		       "(block count %llu, first data block %u, "
-		       "blocks per group %lu)", sbi->s_groups_count,
-		       ext4_blocks_count(es),
-		       le32_to_cpu(es->s_first_data_block),
-		       EXT4_BLOCKS_PER_GROUP(sb));
+		ext4_msg(sb, KERN_WARNING,
+			 "groups count too large: %u (block count %llu, first data block %u, blocks per group %lu)",
+			 sbi->s_groups_count,
+			 ext4_blocks_count(es),
+			 le32_to_cpu(es->s_first_data_block),
+			 EXT4_BLOCKS_PER_GROUP(sb));
 		goto failed_mount;
 	}
 	sbi->s_groups_count = blocks_count;
@@ -3402,7 +3389,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 		sbi->s_group_desc[i] = sb_bread(sb, block);
 		if (!sbi->s_group_desc[i]) {
 			ext4_msg(sb, KERN_ERR,
-			       "can't read group descriptor %d", i);
+				 "can't read group descriptor %d", i);
 			db_count = i;
 			goto failed_mount2;
 		}
@@ -3414,8 +3401,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	if (EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_FLEX_BG))
 		if (!ext4_fill_flex_info(sb)) {
 			ext4_msg(sb, KERN_ERR,
-			       "unable to initialize "
-			       "flex_bg meta info!");
+				 "unable to initialize flex_bg meta info!");
 			goto failed_mount2;
 		}
 
@@ -3489,8 +3475,8 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 			goto failed_mount3;
 	} else if (test_opt(sb, NOLOAD) && !(sb->s_flags & MS_RDONLY) &&
 	      EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_RECOVER)) {
-		ext4_msg(sb, KERN_ERR, "required journal recovery "
-		       "suppressed and not mounted read-only");
+		ext4_msg(sb, KERN_ERR,
+			 "required journal recovery suppressed and not mounted read-only");
 		goto failed_mount_wq;
 	} else {
 		clear_opt(sb, DATA_FLAGS);
@@ -3540,8 +3526,8 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	case EXT4_MOUNT_WRITEBACK_DATA:
 		if (!jbd2_journal_check_available_features
 		    (sbi->s_journal, 0, 0, JBD2_FEATURE_INCOMPAT_REVOKE)) {
-			ext4_msg(sb, KERN_ERR, "Journal does not support "
-			       "requested data journaling mode");
+			ext4_msg(sb, KERN_ERR,
+				 "Journal does not support requested data journaling mode");
 			goto failed_mount_wq;
 		}
 	default:
@@ -3622,14 +3608,14 @@ no_journal:
 							sbi->s_inode_size) {
 		sbi->s_want_extra_isize = sizeof(struct ext4_inode) -
 						       EXT4_GOOD_OLD_INODE_SIZE;
-		ext4_msg(sb, KERN_INFO, "required extra inode space not"
-			 "available");
+		ext4_msg(sb, KERN_INFO,
+			 "required extra inode space not available");
 	}
 
 	err = ext4_setup_system_zone(sb);
 	if (err) {
-		ext4_msg(sb, KERN_ERR, "failed to initialize system "
-			 "zone (%d)", err);
+		ext4_msg(sb, KERN_ERR, "failed to initialize system zone (%d)",
+			 err);
 		goto failed_mount4a;
 	}
 
@@ -3669,8 +3655,8 @@ no_journal:
 	} else
 		descr = "out journal";
 
-	ext4_msg(sb, KERN_INFO, "mounted filesystem with%s. "
-		 "Opts: %s%s%s", descr, sbi->s_es->s_mount_opts,
+	ext4_msg(sb, KERN_INFO, "mounted filesystem with%s. Opts: %s%s%s",
+		 descr, sbi->s_es->s_mount_opts,
 		 *sbi->s_es->s_mount_opts ? "; " : "", orig_data);
 
 	if (es->s_error_count)
@@ -3827,7 +3813,7 @@ static journal_t *ext4_get_dev_journal(struct super_block *sb,
 	hblock = bdev_logical_block_size(bdev);
 	if (blocksize < hblock) {
 		ext4_msg(sb, KERN_ERR,
-			"blocksize too small for journal device");
+			 "blocksize too small for journal device");
 		goto out_bdev;
 	}
 
@@ -3835,8 +3821,8 @@ static journal_t *ext4_get_dev_journal(struct super_block *sb,
 	offset = EXT4_MIN_BLOCK_SIZE % blocksize;
 	set_blocksize(bdev, blocksize);
 	if (!(bh = __bread(bdev, sb_block, blocksize))) {
-		ext4_msg(sb, KERN_ERR, "couldn't read superblock of "
-		       "external journal");
+		ext4_msg(sb, KERN_ERR,
+			 "couldn't read superblock of external journal");
 		goto out_bdev;
 	}
 
@@ -3844,8 +3830,7 @@ static journal_t *ext4_get_dev_journal(struct super_block *sb,
 	if ((le16_to_cpu(es->s_magic) != EXT4_SUPER_MAGIC) ||
 	    !(le32_to_cpu(es->s_feature_incompat) &
 	      EXT4_FEATURE_INCOMPAT_JOURNAL_DEV)) {
-		ext4_msg(sb, KERN_ERR, "external journal has "
-					"bad superblock");
+		ext4_msg(sb, KERN_ERR, "external journal has bad superblock");
 		brelse(bh);
 		goto out_bdev;
 	}
@@ -3874,9 +3859,9 @@ static journal_t *ext4_get_dev_journal(struct super_block *sb,
 		goto out_journal;
 	}
 	if (be32_to_cpu(journal->j_superblock->s_nr_users) != 1) {
-		ext4_msg(sb, KERN_ERR, "External journal has more than one "
-					"user (unsupported) - %d",
-			be32_to_cpu(journal->j_superblock->s_nr_users));
+		ext4_msg(sb, KERN_ERR,
+			 "External journal has more than one user (unsupported) - %d",
+			 be32_to_cpu(journal->j_superblock->s_nr_users));
 		goto out_journal;
 	}
 	EXT4_SB(sb)->journal_bdev = bdev;
@@ -3904,8 +3889,8 @@ static int ext4_load_journal(struct super_block *sb,
 
 	if (journal_devnum &&
 	    journal_devnum != le32_to_cpu(es->s_journal_dev)) {
-		ext4_msg(sb, KERN_INFO, "external journal device major/minor "
-			"numbers have changed");
+		ext4_msg(sb, KERN_INFO,
+			 "external journal device major/minor numbers have changed");
 		journal_dev = new_decode_dev(journal_devnum);
 	} else
 		journal_dev = new_decode_dev(le32_to_cpu(es->s_journal_dev));
@@ -3919,21 +3904,21 @@ static int ext4_load_journal(struct super_block *sb,
 	 */
 	if (EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_RECOVER)) {
 		if (sb->s_flags & MS_RDONLY) {
-			ext4_msg(sb, KERN_INFO, "INFO: recovery "
-					"required on readonly filesystem");
+			ext4_msg(sb, KERN_INFO,
+				 "INFO: recovery required on readonly filesystem");
 			if (really_read_only) {
-				ext4_msg(sb, KERN_ERR, "write access "
-					"unavailable, cannot proceed");
+				ext4_msg(sb, KERN_ERR,
+					 "write access unavailable, cannot proceed");
 				return -EROFS;
 			}
-			ext4_msg(sb, KERN_INFO, "write access will "
-			       "be enabled during recovery");
+			ext4_msg(sb, KERN_INFO,
+				 "write access will be enabled during recovery");
 		}
 	}
 
 	if (journal_inum && journal_dev) {
-		ext4_msg(sb, KERN_ERR, "filesystem has both journal "
-		       "and inode journals!");
+		ext4_msg(sb, KERN_ERR,
+			 "filesystem has both journal and inode journals!");
 		return -EINVAL;
 	}
 
@@ -3999,8 +3984,8 @@ static int ext4_commit_super(struct super_block *sb, int sync)
 		 * be remapped.  Nothing we can do but to retry the
 		 * write and hope for the best.
 		 */
-		ext4_msg(sb, KERN_ERR, "previous I/O error to "
-		       "superblock detected");
+		ext4_msg(sb, KERN_ERR,
+			 "previous I/O error to superblock detected");
 		clear_buffer_write_io_error(sbh);
 		set_buffer_uptodate(sbh);
 	}
@@ -4040,8 +4025,8 @@ static int ext4_commit_super(struct super_block *sb, int sync)
 
 		error = buffer_write_io_error(sbh);
 		if (error) {
-			ext4_msg(sb, KERN_ERR, "I/O error while writing "
-			       "superblock");
+			ext4_msg(sb, KERN_ERR,
+				 "I/O error while writing superblock");
 			clear_buffer_write_io_error(sbh);
 			set_buffer_uptodate(sbh);
 		}
@@ -4103,9 +4088,9 @@ static void ext4_clear_journal_err(struct super_block *sb,
 		char nbuf[16];
 
 		errstr = ext4_decode_error(sb, j_errno, nbuf);
-		ext4_warning(sb, "Filesystem error recorded "
-			     "from previous mount: %s", errstr);
-		ext4_warning(sb, "Marking fs in need of filesystem check.");
+		ext4_warning(sb, "Filesystem error recorded from previous mount: %s",
+			     errstr);
+		ext4_warning(sb, "Marking fs in need of filesystem check");
 
 		EXT4_SB(sb)->s_mount_state |= EXT4_ERROR_FS;
 		es->s_state |= cpu_to_le16(EXT4_ERROR_FS);
@@ -4328,9 +4313,10 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
 
 				if (!ext4_group_desc_csum_verify(sbi, g, gdp)) {
 					ext4_msg(sb, KERN_ERR,
-	       "ext4_remount: Checksum for group %u failed (%u!=%u)",
-		g, le16_to_cpu(ext4_group_desc_csum(sbi, g, gdp)),
-					       le16_to_cpu(gdp->bg_checksum));
+						 "%s: Checksum for group %u failed (%u!=%u)",
+						 __func__, g,
+						 le16_to_cpu(ext4_group_desc_csum(sbi, g, gdp)),
+						 le16_to_cpu(gdp->bg_checksum));
 					err = -EINVAL;
 					goto restore_opts;
 				}
@@ -4342,10 +4328,8 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
 			 * require a full umount/remount for now.
 			 */
 			if (es->s_last_orphan) {
-				ext4_msg(sb, KERN_WARNING, "Couldn't "
-				       "remount RDWR because of unprocessed "
-				       "orphan inode list.  Please "
-				       "umount/remount instead");
+				ext4_msg(sb, KERN_WARNING,
+					 "Couldn't remount RDWR because of unprocessed orphan inode list.  Please umount/remount instead.");
 				err = -EINVAL;
 				goto restore_opts;
 			}
@@ -4632,8 +4616,7 @@ static int ext4_quota_on(struct super_block *sb, int type, int format_id,
 		/* Quotafile not in fs root? */
 		if (path->dentry->d_parent != sb->s_root)
 			ext4_msg(sb, KERN_WARNING,
-				"Quota file not on filesystem root. "
-				"Journaled quota will not work");
+				"Quota file not on filesystem root. Journaled quota will not work.");
 	}
 
 	/*
@@ -4735,9 +4718,9 @@ static ssize_t ext4_quota_write(struct super_block *sb, int type,
 	handle_t *handle = journal_current_handle();
 
 	if (EXT4_SB(sb)->s_journal && !handle) {
-		ext4_msg(sb, KERN_WARNING, "Quota write (off=%llu, len=%llu)"
-			" cancelled because transaction is not started",
-			(unsigned long long)off, (unsigned long long)len);
+		ext4_msg(sb, KERN_WARNING,
+			 "Quota write (off=%llu, len=%llu) cancelled because transaction is not started",
+			 (unsigned long long)off, (unsigned long long)len);
 		return -EIO;
 	}
 	/*
@@ -4745,9 +4728,9 @@ static ssize_t ext4_quota_write(struct super_block *sb, int type,
 	 * then it is impossible to cross a block boundary.
 	 */
 	if (sb->s_blocksize - offset < len) {
-		ext4_msg(sb, KERN_WARNING, "Quota write (off=%llu, len=%llu)"
-			" cancelled because not block aligned",
-			(unsigned long long)off, (unsigned long long)len);
+		ext4_msg(sb, KERN_WARNING,
+			 "Quota write (off=%llu, len=%llu) cancelled because not block aligned",
+			 (unsigned long long)off, (unsigned long long)len);
 		return -EIO;
 	}
 
Signature · 1 line
1.7.8.111.gad25c.dirty
#65 lore raw

Re: [PATCH 7/9] ext4: Format neatening for easier grep

Quoted 2 lines · Ted Ts'o

I've removed the terminating \n from ext4_msg and ext4_error (from another patch) and created a separate patch just for that.

If you did the patch correctly to avoid possible message interleaving by adding '\n' to all the ext4_msg and ext4_error uses, the patch would be just about the same size as the patch I sent.

#66 lore raw

PATCH 8/9 ext4: Neaten ext4_error uses

In reply to Joe Perches

ext4_error appends a newline to the output. Uses of ext4_error should not end in a newline.

Remove terminating newlines from ext4_error uses. Coalesce formats. Add missing space between words in newly coalesced format. Align arguments.

---
 fs/ext4/balloc.c      |   15 ++++++---------
 fs/ext4/ialloc.c      |   15 ++++++---------
 fs/ext4/mballoc.c     |   25 ++++++++++++-------------
 fs/ext4/move_extent.c |    8 ++++----
 4 files changed, 28 insertions(+), 35 deletions(-)
fs/ext4/balloc.c +6 -9
index a5cd777..82f468f 100644
@@ -254,8 +254,8 @@ struct ext4_group_desc * ext4_get_group_desc(struct super_block *sb,
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
 
 	if (block_group >= ngroups) {
-		ext4_error(sb, "block_group >= groups_count - block_group = %u,"
-			   " groups_count = %u", block_group, ngroups);
+		ext4_error(sb, "block_group >= groups_count - block_group = %u, groups_count = %u",
+			   block_group, ngroups);
 
 		return NULL;
 	}
@@ -263,8 +263,7 @@ struct ext4_group_desc * ext4_get_group_desc(struct super_block *sb,
 	group_desc = block_group >> EXT4_DESC_PER_BLOCK_BITS(sb);
 	offset = block_group & (EXT4_DESC_PER_BLOCK(sb) - 1);
 	if (!sbi->s_group_desc[group_desc]) {
-		ext4_error(sb, "Group descriptor not loaded - "
-			   "block_group = %u, group_desc = %u, desc = %u",
+		ext4_error(sb, "Group descriptor not loaded - block_group = %u, group_desc = %u, desc = %u",
 			   block_group, group_desc, offset);
 		return NULL;
 	}
@@ -324,7 +323,7 @@ static int ext4_valid_block_bitmap(struct super_block *sb,
 
 err_out:
 	ext4_error(sb, "Invalid block bitmap - block_group = %d, block = %llu",
-			block_group, bitmap_blk);
+		   block_group, bitmap_blk);
 	return 0;
 }
 /**
@@ -350,8 +349,7 @@ ext4_read_block_bitmap_nowait(struct super_block *sb, ext4_group_t block_group)
 	bitmap_blk = ext4_block_bitmap(sb, desc);
 	bh = sb_getblk(sb, bitmap_blk);
 	if (unlikely(!bh)) {
-		ext4_error(sb, "Cannot get buffer for block bitmap - "
-			   "block_group = %u, block_bitmap = %llu",
+		ext4_error(sb, "Cannot get buffer for block bitmap - block_group = %u, block_bitmap = %llu",
 			   block_group, bitmap_blk);
 		return NULL;
 	}
@@ -407,8 +405,7 @@ int ext4_wait_block_bitmap(struct super_block *sb, ext4_group_t block_group,
 		return 1;
 	wait_on_buffer(bh);
 	if (!buffer_uptodate(bh)) {
-		ext4_error(sb, "Cannot read block bitmap - "
-			   "block_group = %u, block_bitmap = %llu",
+		ext4_error(sb, "Cannot read block bitmap - block_group = %u, block_bitmap = %llu",
 			   block_group, (unsigned long long) bh->b_blocknr);
 		return 1;
 	}
fs/ext4/ialloc.c +6 -9
index 024466a..29cbd9f 100644
@@ -124,9 +124,8 @@ ext4_read_inode_bitmap(struct super_block *sb, ext4_group_t block_group)
 	bitmap_blk = ext4_inode_bitmap(sb, desc);
 	bh = sb_getblk(sb, bitmap_blk);
 	if (unlikely(!bh)) {
-		ext4_error(sb, "Cannot read inode bitmap - "
-			    "block_group = %u, inode_bitmap = %llu",
-			    block_group, bitmap_blk);
+		ext4_error(sb, "Cannot read inode bitmap - block_group = %u, inode_bitmap = %llu",
+			   block_group, bitmap_blk);
 		return NULL;
 	}
 	if (bitmap_uptodate(bh))
@@ -168,8 +167,7 @@ ext4_read_inode_bitmap(struct super_block *sb, ext4_group_t block_group)
 	wait_on_buffer(bh);
 	if (!buffer_uptodate(bh)) {
 		put_bh(bh);
-		ext4_error(sb, "Cannot read inode bitmap - "
-			   "block_group = %u, inode_bitmap = %llu",
+		ext4_error(sb, "Cannot read inode bitmap - block_group = %u, inode_bitmap = %llu",
 			   block_group, bitmap_blk);
 		return NULL;
 	}
@@ -690,8 +688,8 @@ repeat_in_this_group:
 			continue;
 		}
 		if (group == 0 && (ino+1) < EXT4_FIRST_INO(sb)) {
-			ext4_error(sb, "reserved inode found cleared - "
-				   "inode=%lu", ino + 1);
+			ext4_error(sb, "reserved inode found cleared - inode=%lu",
+				   ino + 1);
 			continue;
 		}
 		ext4_lock_group(sb, group);
@@ -1101,8 +1099,7 @@ int ext4_init_inode_table(struct super_block *sb, ext4_group_t group,
 
 	if ((used_blks < 0) || (used_blks > sbi->s_itb_per_group)) {
 		ext4_error(sb, "Something is wrong with group %u\n"
-			   "Used itable blocks: %d"
-			   "itable unused count: %u\n",
+			   "Used itable blocks: %d itable unused count: %u",
 			   group, used_blks,
 			   ext4_itable_unused_count(sb, gdp));
 		ret = 1;
fs/ext4/mballoc.c +12 -13
index 70b22fb..185cbac 100644
@@ -2758,8 +2758,8 @@ ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac,
 
 	len = EXT4_C2B(sbi, ac->ac_b_ex.fe_len);
 	if (!ext4_data_block_valid(sbi, block, len)) {
-		ext4_error(sb, "Allocating blocks %llu-%llu which overlap "
-			   "fs metadata\n", block, block+len);
+		ext4_error(sb, "Allocating blocks %llu-%llu which overlap fs metadata",
+			   block, block + len);
 		/* File system mounted not to panic on error
 		 * Fix the bitmap and repeat the block allocation
 		 * We leak some of the blocks here.
@@ -3826,14 +3826,14 @@ repeat:
 		err = ext4_mb_load_buddy(sb, group, &e4b);
 		if (err) {
 			ext4_error(sb, "Error loading buddy information for %u",
-					group);
+				   group);
 			continue;
 		}
 
 		bitmap_bh = ext4_read_block_bitmap(sb, group);
 		if (bitmap_bh == NULL) {
 			ext4_error(sb, "Error reading block bitmap for %u",
-					group);
+				   group);
 			ext4_mb_unload_buddy(&e4b);
 			continue;
 		}
@@ -4082,7 +4082,7 @@ ext4_mb_discard_lg_preallocations(struct super_block *sb,
 		ext4_get_group_no_and_offset(sb, pa->pa_pstart, &group, NULL);
 		if (ext4_mb_load_buddy(sb, group, &e4b)) {
 			ext4_error(sb, "Error loading buddy information for %u",
-					group);
+				   group);
 			continue;
 		}
 		ext4_lock_group(sb, group);
@@ -4488,8 +4488,8 @@ void ext4_free_blocks(handle_t *handle, struct inode *inode,
 	sbi = EXT4_SB(sb);
 	if (!(flags & EXT4_FREE_BLOCKS_VALIDATED) &&
 	    !ext4_data_block_valid(sbi, block, count)) {
-		ext4_error(sb, "Freeing blocks not in datazone - "
-			   "block = %llu, count = %lu", block, count);
+		ext4_error(sb, "Freeing blocks not in datazone - block = %llu, count = %lu",
+			   block, count);
 		goto error_return;
 	}
 
@@ -4587,8 +4587,8 @@ do_more:
 	    in_range(block + count - 1, ext4_inode_table(sb, gdp),
 		     EXT4_SB(sb)->s_itb_per_group)) {
 
-		ext4_error(sb, "Freeing blocks in system zone - "
-			   "Block = %llu, count = %lu", block, count);
+		ext4_error(sb, "Freeing blocks in system zone - Block = %llu, count = %lu",
+			   block, count);
 		/* err = 0. ext4_std_error should be a no op */
 		goto error_return;
 	}
@@ -4748,8 +4748,7 @@ int ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
 	    in_range(block, ext4_inode_table(sb, desc), sbi->s_itb_per_group) ||
 	    in_range(block + count - 1, ext4_inode_table(sb, desc),
 		     sbi->s_itb_per_group)) {
-		ext4_error(sb, "Adding blocks in system zones - "
-			   "Block = %llu, count = %lu",
+		ext4_error(sb, "Adding blocks in system zones - Block = %llu, count = %lu",
 			   block, count);
 		err = -EINVAL;
 		goto error_return;
@@ -4892,8 +4891,8 @@ ext4_trim_all_free(struct super_block *sb, ext4_group_t group,
 
 	ret = ext4_mb_load_buddy(sb, group, &e4b);
 	if (ret) {
-		ext4_error(sb, "Error in loading buddy "
-				"information for %u", group);
+		ext4_error(sb, "Error in loading buddy information for %u",
+			   group);
 		return ret;
 	}
 	bitmap = e4b.bd_bitmap;
fs/ext4/move_extent.c +4 -4
index ce52881..4558376 100644
@@ -153,13 +153,13 @@ mext_check_null_inode(struct inode *inode1, struct inode *inode2,
 
 	if (inode1 == NULL) {
 		__ext4_error(inode2->i_sb, function, line,
-			"Both inodes should not be NULL: "
-			"inode1 NULL inode2 %lu", inode2->i_ino);
+			     "Both inodes should not be NULL: inode1 NULL inode2 %lu",
+			     inode2->i_ino);
 		ret = -EIO;
 	} else if (inode2 == NULL) {
 		__ext4_error(inode1->i_sb, function, line,
-			"Both inodes should not be NULL: "
-			"inode1 %lu inode2 NULL", inode1->i_ino);
+			     "Both inodes should not be NULL: inode1 %lu inode2 NULL",
+			     inode1->i_ino);
 		ret = -EIO;
 	}
 	return ret;
Signature · 1 line
1.7.8.111.gad25c.dirty
#67 lore raw

PATCH 9/9 ext4: Rename ext4_warning to ext4_warn and ext4_error to ext4_err

In reply to Joe Perches

Make the logging macros more like the current kernel logging style.

Neaten the macro definitions.
Align arguments and 80 column wrapping.

---
 fs/ext4/balloc.c   |   22 ++++----
 fs/ext4/ext4.h     |    8 ++--
 fs/ext4/ialloc.c   |   32 ++++++------
 fs/ext4/indirect.c |    6 +-
 fs/ext4/inode.c    |   20 ++++----
 fs/ext4/mballoc.c  |   44 +++++++++---------
 fs/ext4/mmp.c      |   30 ++++++------
 fs/ext4/namei.c    |   66 +++++++++++++-------------
 fs/ext4/page-io.c  |   12 ++--
 fs/ext4/resize.c   |  133 +++++++++++++++++++++++++--------------------------
 fs/ext4/super.c    |   20 ++++----
 11 files changed, 194 insertions(+), 199 deletions(-)
fs/ext4/balloc.c +11 -11
index 82f468f..c77446c 100644
@@ -171,7 +171,7 @@ void ext4_init_block_bitmap(struct super_block *sb, struct buffer_head *bh,
 	/* If checksum is bad mark all blocks used to prevent allocation
 	 * essentially implementing a per-group read-only flag. */
 	if (!ext4_group_desc_csum_verify(sbi, block_group, gdp)) {
-		ext4_error(sb, "Checksum bad for group %u", block_group);
+		ext4_err(sb, "Checksum bad for group %u", block_group);
 		ext4_free_group_clusters_set(sb, gdp, 0);
 		ext4_free_inodes_set(sb, gdp, 0);
 		ext4_itable_unused_set(sb, gdp, 0);
@@ -254,8 +254,8 @@ struct ext4_group_desc * ext4_get_group_desc(struct super_block *sb,
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
 
 	if (block_group >= ngroups) {
-		ext4_error(sb, "block_group >= groups_count - block_group = %u, groups_count = %u",
-			   block_group, ngroups);
+		ext4_err(sb, "block_group >= groups_count - block_group = %u, groups_count = %u",
+			 block_group, ngroups);
 
 		return NULL;
 	}
@@ -263,8 +263,8 @@ struct ext4_group_desc * ext4_get_group_desc(struct super_block *sb,
 	group_desc = block_group >> EXT4_DESC_PER_BLOCK_BITS(sb);
 	offset = block_group & (EXT4_DESC_PER_BLOCK(sb) - 1);
 	if (!sbi->s_group_desc[group_desc]) {
-		ext4_error(sb, "Group descriptor not loaded - block_group = %u, group_desc = %u, desc = %u",
-			   block_group, group_desc, offset);
+		ext4_err(sb, "Group descriptor not loaded - block_group = %u, group_desc = %u, desc = %u",
+			 block_group, group_desc, offset);
 		return NULL;
 	}
 
@@ -322,8 +322,8 @@ static int ext4_valid_block_bitmap(struct super_block *sb,
 		return 1;
 
 err_out:
-	ext4_error(sb, "Invalid block bitmap - block_group = %d, block = %llu",
-		   block_group, bitmap_blk);
+	ext4_err(sb, "Invalid block bitmap - block_group = %d, block = %llu",
+		 block_group, bitmap_blk);
 	return 0;
 }
 /**
@@ -349,8 +349,8 @@ ext4_read_block_bitmap_nowait(struct super_block *sb, ext4_group_t block_group)
 	bitmap_blk = ext4_block_bitmap(sb, desc);
 	bh = sb_getblk(sb, bitmap_blk);
 	if (unlikely(!bh)) {
-		ext4_error(sb, "Cannot get buffer for block bitmap - block_group = %u, block_bitmap = %llu",
-			   block_group, bitmap_blk);
+		ext4_err(sb, "Cannot get buffer for block bitmap - block_group = %u, block_bitmap = %llu",
+			 block_group, bitmap_blk);
 		return NULL;
 	}
 
@@ -405,8 +405,8 @@ int ext4_wait_block_bitmap(struct super_block *sb, ext4_group_t block_group,
 		return 1;
 	wait_on_buffer(bh);
 	if (!buffer_uptodate(bh)) {
-		ext4_error(sb, "Cannot read block bitmap - block_group = %u, block_bitmap = %llu",
-			   block_group, (unsigned long long) bh->b_blocknr);
+		ext4_err(sb, "Cannot read block bitmap - block_group = %u, block_bitmap = %llu",
+			 block_group, (unsigned long long) bh->b_blocknr);
 		return 1;
 	}
 	clear_buffer_new(bh);
fs/ext4/ext4.h +4 -4
index 2aaf549..348df50 100644
@@ -1960,8 +1960,8 @@ extern void ext4_kvfree(void *ptr);
 extern __printf(4, 5)
 void __ext4_error(struct super_block *, const char *, unsigned int,
 		  const char *, ...);
-#define ext4_error(sb, message...)	__ext4_error(sb, __func__,	\
-						     __LINE__, ## message)
+#define ext4_err(sb, fmt, ...)	\
+	__ext4_error(sb, __func__, __LINE__, fmt, ##__VA_ARGS__)
 extern __printf(5, 6)
 void ext4_error_inode(struct inode *, const char *, unsigned int, ext4_fsblk_t,
 		      const char *, ...);
@@ -1978,8 +1978,8 @@ void __ext4_abort(struct super_block *, const char *, unsigned int,
 extern __printf(4, 5)
 void __ext4_warning(struct super_block *, const char *, unsigned int,
 		    const char *, ...);
-#define ext4_warning(sb, message...)	__ext4_warning(sb, __func__, \
-						       __LINE__, ## message)
+#define ext4_warn(sb, fmt, ...)						\
+	__ext4_warning(sb, __func__, __LINE__, fmt, ##__VA_ARGS__)
 extern __printf(3, 4)
 void ext4_msg(struct super_block *, const char *, const char *, ...);
 extern void __dump_mmp_msg(struct super_block *, struct mmp_struct *mmp,
fs/ext4/ialloc.c +16 -16
index 29cbd9f..f4522eb 100644
@@ -79,7 +79,7 @@ static unsigned ext4_init_inode_bitmap(struct super_block *sb,
 	/* If checksum is bad mark all blocks and inodes use to prevent
 	 * allocation, essentially implementing a per-group read-only flag. */
 	if (!ext4_group_desc_csum_verify(sbi, block_group, gdp)) {
-		ext4_error(sb, "Checksum bad for group %u", block_group);
+		ext4_err(sb, "Checksum bad for group %u", block_group);
 		ext4_free_group_clusters_set(sb, gdp, 0);
 		ext4_free_inodes_set(sb, gdp, 0);
 		ext4_itable_unused_set(sb, gdp, 0);
@@ -124,8 +124,8 @@ ext4_read_inode_bitmap(struct super_block *sb, ext4_group_t block_group)
 	bitmap_blk = ext4_inode_bitmap(sb, desc);
 	bh = sb_getblk(sb, bitmap_blk);
 	if (unlikely(!bh)) {
-		ext4_error(sb, "Cannot read inode bitmap - block_group = %u, inode_bitmap = %llu",
-			   block_group, bitmap_blk);
+		ext4_err(sb, "Cannot read inode bitmap - block_group = %u, inode_bitmap = %llu",
+			 block_group, bitmap_blk);
 		return NULL;
 	}
 	if (bitmap_uptodate(bh))
@@ -167,8 +167,8 @@ ext4_read_inode_bitmap(struct super_block *sb, ext4_group_t block_group)
 	wait_on_buffer(bh);
 	if (!buffer_uptodate(bh)) {
 		put_bh(bh);
-		ext4_error(sb, "Cannot read inode bitmap - block_group = %u, inode_bitmap = %llu",
-			   block_group, bitmap_blk);
+		ext4_err(sb, "Cannot read inode bitmap - block_group = %u, inode_bitmap = %llu",
+			 block_group, bitmap_blk);
 		return NULL;
 	}
 	return bh;
@@ -239,7 +239,7 @@ void ext4_free_inode(handle_t *handle, struct inode *inode)
 
 	es = EXT4_SB(sb)->s_es;
 	if (ino < EXT4_FIRST_INO(sb) || ino > le32_to_cpu(es->s_inodes_count)) {
-		ext4_error(sb, "reserved or nonexistent inode %lu", ino);
+		ext4_err(sb, "reserved or nonexistent inode %lu", ino);
 		goto error_return;
 	}
 	block_group = (ino - 1) / EXT4_INODES_PER_GROUP(sb);
@@ -294,7 +294,7 @@ out:
 			fatal = err;
 		ext4_mark_super_dirty(sb);
 	} else
-		ext4_error(sb, "bit already cleared for inode %lu", ino);
+		ext4_err(sb, "bit already cleared for inode %lu", ino);
 
 error_return:
 	brelse(bitmap_bh);
@@ -688,8 +688,8 @@ repeat_in_this_group:
 			continue;
 		}
 		if (group == 0 && (ino+1) < EXT4_FIRST_INO(sb)) {
-			ext4_error(sb, "reserved inode found cleared - inode=%lu",
-				   ino + 1);
+			ext4_err(sb, "reserved inode found cleared - inode=%lu",
+				 ino + 1);
 			continue;
 		}
 		ext4_lock_group(sb, group);
@@ -922,7 +922,7 @@ struct inode *ext4_orphan_get(struct super_block *sb, unsigned long ino)
 
 	/* Error cases - e2fsck has already cleaned up for us */
 	if (ino > max_ino) {
-		ext4_warning(sb, "bad orphan ino %lu!  e2fsck was run?", ino);
+		ext4_warn(sb, "bad orphan ino %lu!  e2fsck was run?", ino);
 		goto error;
 	}
 
@@ -930,7 +930,7 @@ struct inode *ext4_orphan_get(struct super_block *sb, unsigned long ino)
 	bit = (ino - 1) % EXT4_INODES_PER_GROUP(sb);
 	bitmap_bh = ext4_read_inode_bitmap(sb, block_group);
 	if (!bitmap_bh) {
-		ext4_warning(sb, "inode bitmap error for orphan %lu", ino);
+		ext4_warn(sb, "inode bitmap error for orphan %lu", ino);
 		goto error;
 	}
 
@@ -962,7 +962,7 @@ iget_failed:
 	err = PTR_ERR(inode);
 	inode = NULL;
 bad_orphan:
-	ext4_warning(sb, "bad orphan inode %lu!  e2fsck was run?", ino);
+	ext4_warn(sb, "bad orphan inode %lu!  e2fsck was run?", ino);
 	pr_notice("ext4_test_bit(bit=%d, block=%llu) = %d\n",
 		  bit, (unsigned long long)bitmap_bh->b_blocknr,
 		  ext4_test_bit(bit, bitmap_bh->b_data));
@@ -1098,10 +1098,10 @@ int ext4_init_inode_table(struct super_block *sb, ext4_group_t group,
 			    sbi->s_inodes_per_block);
 
 	if ((used_blks < 0) || (used_blks > sbi->s_itb_per_group)) {
-		ext4_error(sb, "Something is wrong with group %u\n"
-			   "Used itable blocks: %d itable unused count: %u",
-			   group, used_blks,
-			   ext4_itable_unused_count(sb, gdp));
+		ext4_err(sb, "Something is wrong with group %u\n"
+			 "Used itable blocks: %d itable unused count: %u",
+			 group, used_blks,
+			 ext4_itable_unused_count(sb, gdp));
 		ret = 1;
 		goto err_out;
 	}
fs/ext4/indirect.c +3 -3
index bd02261..db428a0 100644
@@ -101,9 +101,9 @@ static int ext4_block_to_path(struct inode *inode,
 		offsets[n++] = i_block & (ptrs - 1);
 		final = ptrs;
 	} else {
-		ext4_warning(inode->i_sb, "block %lu > max in inode %lu",
-			     i_block + direct_blocks +
-			     indirect_blocks + double_blocks, inode->i_ino);
+		ext4_warn(inode->i_sb, "block %lu > max in inode %lu",
+			  i_block + direct_blocks +
+			  indirect_blocks + double_blocks, inode->i_ino);
 	}
 	if (boundary)
 		*boundary = final - 1 - (i_block & (ptrs - 1));
fs/ext4/inode.c +10 -10
index 2fbfee2..29e2f29 100644
@@ -185,8 +185,8 @@ void ext4_evict_inode(struct inode *inode)
 	inode->i_size = 0;
 	err = ext4_mark_inode_dirty(handle, inode);
 	if (err) {
-		ext4_warning(inode->i_sb,
-			     "couldn't mark inode dirty (err %d)", err);
+		ext4_warn(inode->i_sb,
+			  "couldn't mark inode dirty (err %d)", err);
 		goto stop_handle;
 	}
 	if (inode->i_blocks)
@@ -203,8 +203,8 @@ void ext4_evict_inode(struct inode *inode)
 		if (err > 0)
 			err = ext4_journal_restart(handle, 3);
 		if (err != 0) {
-			ext4_warning(inode->i_sb,
-				     "couldn't extend journal (err %d)", err);
+			ext4_warn(inode->i_sb,
+				  "couldn't extend journal (err %d)", err);
 		stop_handle:
 			ext4_journal_stop(handle);
 			ext4_orphan_del(NULL, inode);
@@ -1578,9 +1578,9 @@ static void mpage_da_map_and_submit(struct mpage_da_data *mpd)
 		ext4_update_i_disksize(mpd->inode, disksize);
 		err = ext4_mark_inode_dirty(handle, mpd->inode);
 		if (err)
-			ext4_error(mpd->inode->i_sb,
-				   "Failed to mark inode %lu dirty",
-				   mpd->inode->i_ino);
+			ext4_err(mpd->inode->i_sb,
+				 "Failed to mark inode %lu dirty",
+				 mpd->inode->i_ino);
 	}
 
 submit_io:
@@ -4442,9 +4442,9 @@ int ext4_mark_inode_dirty(handle_t *handle, struct inode *inode)
 						     EXT4_STATE_NO_EXPAND);
 				if (mnt_count !=
 					le16_to_cpu(sbi->s_es->s_mnt_count)) {
-					ext4_warning(inode->i_sb,
-					"Unable to expand inode %lu. Delete some EAs or run e2fsck.",
-					inode->i_ino);
+					ext4_warn(inode->i_sb,
+						  "Unable to expand inode %lu. Delete some EAs or run e2fsck.",
+						  inode->i_ino);
 					mnt_count =
 					  le16_to_cpu(sbi->s_es->s_mnt_count);
 				}
fs/ext4/mballoc.c +22 -22

114 lines not on this page · raw message

fs/ext4/mmp.c +14 -16
index 1767b2a..9aba1b4 100644
@@ -53,8 +53,7 @@ static int read_mmp_block(struct super_block *sb, struct buffer_head **bh,
 		}
 	}
 	if (!*bh) {
-		ext4_warning(sb, "Error while reading MMP block %llu",
-			     mmp_block);
+		ext4_warn(sb, "Error while reading MMP block %llu", mmp_block);
 		return -EIO;
 	}
 
@@ -126,19 +125,19 @@ static int kmmpd(void *data)
 		 */
 		if (retval) {
 			if ((failed_writes % 60) == 0)
-				ext4_error(sb, "Error writing to MMP block");
+				ext4_err(sb, "Error writing to MMP block");
 			failed_writes++;
 		}
 
 		if (!(le32_to_cpu(es->s_feature_incompat) &
 		    EXT4_FEATURE_INCOMPAT_MMP)) {
-			ext4_warning(sb, "kmmpd being stopped since MMP feature has been disabled");
+			ext4_warn(sb, "kmmpd being stopped since MMP feature has been disabled");
 			EXT4_SB(sb)->s_mmp_tsk = NULL;
 			goto failed;
 		}
 
 		if (sb->s_flags & MS_RDONLY) {
-			ext4_warning(sb, "kmmpd being stopped since filesystem has been remounted as readonly");
+			ext4_warn(sb, "kmmpd being stopped since filesystem has been remounted as readonly");
 			EXT4_SB(sb)->s_mmp_tsk = NULL;
 			goto failed;
 		}
@@ -160,8 +159,8 @@ static int kmmpd(void *data)
 
 			retval = read_mmp_block(sb, &bh_check, mmp_block);
 			if (retval) {
-				ext4_error(sb, "error reading MMP data: %d",
-					   retval);
+				ext4_err(sb, "error reading MMP data: %d",
+					 retval);
 
 				EXT4_SB(sb)->s_mmp_tsk = NULL;
 				goto failed;
@@ -175,7 +174,7 @@ static int kmmpd(void *data)
 					     "Error while updating MMP info. "
 					     "The filesystem seems to have been"
 					     " multiply mounted.");
-				ext4_error(sb, "abort");
+				ext4_err(sb, "abort");
 				goto failed;
 			}
 			put_bh(bh_check);
@@ -237,7 +236,7 @@ int ext4_multi_mount_protect(struct super_block *sb,
 
 	if (mmp_block < le32_to_cpu(es->s_first_data_block) ||
 	    mmp_block >= ext4_blocks_count(es)) {
-		ext4_warning(sb, "Invalid MMP block in superblock");
+		ext4_warn(sb, "Invalid MMP block in superblock");
 		goto failed;
 	}
 
@@ -271,11 +270,11 @@ int ext4_multi_mount_protect(struct super_block *sb,
 
 	/* Print MMP interval if more than 20 secs. */
 	if (wait_time > EXT4_MMP_MIN_CHECK_INTERVAL * 4)
-		ext4_warning(sb, "MMP interval %u higher than expected, please wait",
-			     wait_time * 2);
+		ext4_warn(sb, "MMP interval %u higher than expected, please wait",
+			  wait_time * 2);
 
 	if (schedule_timeout_interruptible(HZ * wait_time) != 0) {
-		ext4_warning(sb, "MMP startup interrupted, failing mount");
+		ext4_warn(sb, "MMP startup interrupted, failing mount");
 		goto failed;
 	}
 
@@ -304,7 +303,7 @@ skip:
 	 * wait for MMP interval and check mmp_seq.
 	 */
 	if (schedule_timeout_interruptible(HZ * wait_time) != 0) {
-		ext4_warning(sb, "MMP startup interrupted, failing mount");
+		ext4_warn(sb, "MMP startup interrupted, failing mount");
 		goto failed;
 	}
 
@@ -320,7 +319,7 @@ skip:
 
 	mmpd_data = kmalloc(sizeof(struct mmpd_data), GFP_KERNEL);
 	if (!mmpd_data) {
-		ext4_warning(sb, "not enough memory for mmpd_data");
+		ext4_warn(sb, "not enough memory for mmpd_data");
 		goto failed;
 	}
 	mmpd_data->sb = sb;
@@ -335,8 +334,7 @@ skip:
 	if (IS_ERR(EXT4_SB(sb)->s_mmp_tsk)) {
 		EXT4_SB(sb)->s_mmp_tsk = NULL;
 		kfree(mmpd_data);
-		ext4_warning(sb, "Unable to create kmmpd thread for %s",
-			     sb->s_id);
+		ext4_warn(sb, "Unable to create kmmpd thread for %s", sb->s_id);
 		goto failed;
 	}
 
fs/ext4/namei.c +33 -33

150 lines not on this page · raw message

fs/ext4/page-io.c +6 -6
index a5afbad..b1b492a 100644
@@ -238,12 +238,12 @@ static void ext4_end_bio(struct bio *bio, int error)
 
 	if (error) {
 		io_end->flag |= EXT4_IO_END_ERROR;
-		ext4_warning(inode->i_sb, "I/O error writing to inode %lu (offset %llu size %ld starting block %llu)",
-			     inode->i_ino,
-			     (unsigned long long) io_end->offset,
-			     (long) io_end->size,
-			     (unsigned long long)
-			     bi_sector >> (inode->i_blkbits - 9));
+		ext4_warn(inode->i_sb, "I/O error writing to inode %lu (offset %llu size %ld starting block %llu)",
+			  inode->i_ino,
+			  (unsigned long long) io_end->offset,
+			  (long) io_end->size,
+			  (unsigned long long)
+			  bi_sector >> (inode->i_blkbits - 9));
 	}
 
 	if (!(io_end->flag & EXT4_IO_END_UNWRITTEN)) {
fs/ext4/resize.c +65 -68

288 lines not on this page · raw message

fs/ext4/super.c +10 -10
index cade67b..18e68f1 100644
@@ -621,7 +621,7 @@ void __ext4_std_error(struct super_block *sb, const char *function,
 }
 
 /*
- * ext4_abort is a much stronger failure handler than ext4_error.  The
+ * ext4_abort is a much stronger failure handler than ext4_err.  The
  * abort function may be used to deal with unrecoverable failures such
  * as journal IO errors or ENOMEM at a critical moment in log management.
  *
@@ -737,9 +737,9 @@ void ext4_update_dynamic_rev(struct super_block *sb)
 	if (le32_to_cpu(es->s_rev_level) > EXT4_GOOD_OLD_REV)
 		return;
 
-	ext4_warning(sb,
-		     "updating to rev %d because of new feature flag, running e2fsck is recommended",
-		     EXT4_DYNAMIC_REV);
+	ext4_warn(sb,
+		  "updating to rev %d because of new feature flag, running e2fsck is recommended",
+		  EXT4_DYNAMIC_REV);
 
 	es->s_first_ino = cpu_to_le32(EXT4_GOOD_OLD_FIRST_INO);
 	es->s_inode_size = cpu_to_le16(EXT4_GOOD_OLD_INODE_SIZE);
@@ -3289,8 +3289,8 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 		}
 	} else {
 		if (clustersize != blocksize) {
-			ext4_warning(sb, "fragment/cluster size (%d) != block size (%d)",
-				     clustersize, blocksize);
+			ext4_warn(sb, "fragment/cluster size (%d) != block size (%d)",
+				  clustersize, blocksize);
 			clustersize = blocksize;
 		}
 		if (sbi->s_blocks_per_group > blocksize * 8) {
@@ -4080,7 +4080,7 @@ static void ext4_clear_journal_err(struct super_block *sb,
 
 	/*
 	 * Now check for any error status which may have been recorded in the
-	 * journal by a prior ext4_error() or ext4_abort()
+	 * journal by a prior ext4_err() or ext4_abort()
 	 */
 
 	j_errno = jbd2_journal_errno(journal);
@@ -4088,9 +4088,9 @@ static void ext4_clear_journal_err(struct super_block *sb,
 		char nbuf[16];
 
 		errstr = ext4_decode_error(sb, j_errno, nbuf);
-		ext4_warning(sb, "Filesystem error recorded from previous mount: %s",
-			     errstr);
-		ext4_warning(sb, "Marking fs in need of filesystem check");
+		ext4_warn(sb, "Filesystem error recorded from previous mount: %s",
+			  errstr);
+		ext4_warn(sb, "Marking fs in need of filesystem check");
 
 		EXT4_SB(sb)->s_mount_state |= EXT4_ERROR_FS;
 		es->s_state |= cpu_to_le16(EXT4_ERROR_FS);
Signature · 1 line
1.7.8.111.gad25c.dirty
maintainer
#68 lore raw

Re: [PATCH 9/9] ext4: Rename ext4_warning to ext4_warn and ext4_error to ext4_err

Quoted 6 lines · Joe Perches

Make the logging macros more like the current kernel logging style.

Neaten the macro definitions.
Align arguments and 80 column wrapping.

Code churn. NACK.

- Ted