Patchwork [3/5] ext4: use ext4_msg() instead of printk in mballoc

login
register
mail settings
Submitter Theodore Ts'o
Date Aug. 1, 2011, 1:13 p.m.
Message ID <1312204398-12460-4-git-send-email-tytso@mit.edu>
Download mbox | patch
Permalink /patch/107732/
State Accepted
Headers show

Comments

Theodore Ts'o - Aug. 1, 2011, 1:13 p.m.
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/mballoc.c |   79 ++++++++++++++++++++++++++++-------------------------
 1 files changed, 42 insertions(+), 37 deletions(-)
Andreas Dilger - Aug. 2, 2011, 3:12 a.m.
On 2011-08-01, at 7:13 AM, Theodore Ts'o <tytso@mit.edu> wrote:
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> ---
> fs/ext4/mballoc.c |   79 ++++++++++++++++++++++++++++-------------------------
> 1 files changed, 42 insertions(+), 37 deletions(-)

Hi Ted, thanks for updating this patch set.  One minor issue below. 

> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index d5021e8..262fa47 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -493,10 +493,11 @@ static void mb_cmp_bitmaps(struct ext4_buddy *e4b, void *bitmap)
>        b2 = (unsigned char *) bitmap;
>        for (i = 0; i < e4b->bd_sb->s_blocksize; i++) {
>            if (b1[i] != b2[i]) {
> -                printk(KERN_ERR "corruption in group %u "
> -                       "at byte %u(%u): %x in copy != %x "
> -                       "on disk/prealloc\n",
> -                       e4b->bd_group, i, i * 8, 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",
> +                     e4b->bd_group, i, i * 8, b1[i], b2[i]);
>                BUG();
>            }
>        }
> @@ -2224,8 +2225,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) {
> -            printk(KERN_ERR "EXT4-fs: can't allocate mem for a "
> -                   "buddy group\n");
> +            ext4_msg(sb, KERN_ERR, "EXT4-fs: can't allocate mem "
> +                 "for a buddy group");

I don't think ext4_msg() format strings should still have "Ext4-fs:" at the beginning. Isn't that the whole point of using ext4_msg()?

>            goto exit_meta_group_info;
>        }
>        sbi->s_group_info[group >> EXT4_DESC_PER_BLOCK_BITS(sb)] =
> @@ -2238,7 +2239,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) {
> -        printk(KERN_ERR "EXT4-fs: can't allocate buddy mem\n");
> +        ext4_msg(sb, KERN_ERR, "EXT4-fs: can't allocate buddy mem");

Ditto. 

>        goto exit_group_info;
>    }
>    memset(meta_group_info[i], 0, kmem_cache_size(cachep));
> @@ -2333,12 +2334,12 @@ static int ext4_mb_init_backend(struct super_block *sb)
>     * So a two level scheme suffices for now. */
>    sbi->s_group_info = ext4_kvzalloc(array_size, GFP_KERNEL);
>    if (sbi->s_group_info == NULL) {
> -        printk(KERN_ERR "EXT4-fs: can't allocate buddy meta group\n");
> +        ext4_msg(sb, KERN_ERR, "can't allocate buddy meta group");
>        return -ENOMEM;
>    }
>    sbi->s_buddy_cache = new_inode(sb);
>    if (sbi->s_buddy_cache == NULL) {
> -        printk(KERN_ERR "EXT4-fs: can't get new inode\n");
> +        ext4_msg(sb, KERN_ERR, "can't get new inode");
>        goto err_freesgi;
>    }
>    sbi->s_buddy_cache->i_ino = get_next_ino();
> @@ -2346,8 +2347,7 @@ static int ext4_mb_init_backend(struct super_block *sb)
>    for (i = 0; i < ngroups; i++) {
>        desc = ext4_get_group_desc(sb, i, NULL);
>        if (desc == NULL) {
> -            printk(KERN_ERR
> -                "EXT4-fs: can't read descriptor %u\n", i);
> +            ext4_msg(sb, KERN_ERR, "can't read descriptor %u", i);
>            goto err_freebuddy;
>        }
>        if (ext4_mb_add_groupinfo(sb, i, desc) != 0)
> @@ -2411,7 +2411,8 @@ static int ext4_groupinfo_create_slab(size_t size)
> 
>    mutex_unlock(&ext4_grpinfo_slab_create_mutex);
>    if (!cachep) {
> -        printk(KERN_EMERG "EXT4: no memory for groupinfo slab cache\n");
> +        printk(KERN_EMERG,
> +               "EXT4-fs: no memory for groupinfo slab cache\n");

This one didn't actually get changed to ext4_msg(). 

>        return -ENOMEM;
>    }
> 
> @@ -2566,25 +2567,25 @@ int ext4_mb_release(struct super_block *sb)
>    if (sbi->s_buddy_cache)
>        iput(sbi->s_buddy_cache);
>    if (sbi->s_mb_stats) {
> -        printk(KERN_INFO
> -               "EXT4-fs: mballoc: %u blocks %u reqs (%u success)\n",
> +        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));
> -        printk(KERN_INFO
> -              "EXT4-fs: mballoc: %u extents scanned, %u goal hits, "
> -                "%u 2^N hits, %u breaks, %u lost\n",
> +        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));
> -        printk(KERN_INFO
> -               "EXT4-fs: mballoc: %lu generated and it took %Lu\n",
> +        ext4_msg(sb, KERN_INFO,
> +               "mballoc: %lu generated and it took %Lu",
>                sbi->s_mb_buddies_generated,
>                sbi->s_mb_generation_time);
> -        printk(KERN_INFO
> -               "EXT4-fs: mballoc: %u preallocated, %u discarded\n",
> +        ext4_msg(sb, KERN_INFO,
> +               "mballoc: %u preallocated, %u discarded",
>                atomic_read(&sbi->s_mb_preallocated),
>                atomic_read(&sbi->s_mb_discarded));
>    }
> @@ -3024,9 +3025,10 @@ ext4_mb_normalize_request(struct ext4_allocation_context *ac,
> 
>    if (start + size <= ac->ac_o_ex.fe_logical &&
>            start > ac->ac_o_ex.fe_logical) {
> -        printk(KERN_ERR "start %lu, size %lu, fe_logical %lu\n",
> -            (unsigned long) start, (unsigned long) size,
> -            (unsigned long) ac->ac_o_ex.fe_logical);
> +        ext4_msg(ac->ac_sb, KERN_ERR,
> +             "start %lu, size %lu, fe_logical %lu",
> +             (unsigned long) start, (unsigned long) size,
> +             (unsigned long) ac->ac_o_ex.fe_logical);
>    }
>    BUG_ON(start + size <= ac->ac_o_ex.fe_logical &&
>            start > ac->ac_o_ex.fe_logical);
> @@ -3607,10 +3609,11 @@ ext4_mb_release_inode_pa(struct ext4_buddy *e4b, struct buffer_head *bitmap_bh,
>        bit = next + 1;
>    }
>    if (free != pa->pa_free) {
> -        printk(KERN_CRIT "pa %p: logic %lu, phys. %lu, len %lu\n",
> -            pa, (unsigned long) pa->pa_lstart,
> -            (unsigned long) pa->pa_pstart,
> -            (unsigned long) pa->pa_len);
> +        ext4_msg(e4b->bd_sb, KERN_CRIT,
> +             "pa %p: logic %lu, phys. %lu, len %lu",
> +             pa, (unsigned long) pa->pa_lstart,
> +             (unsigned long) pa->pa_pstart,
> +             (unsigned long) pa->pa_len);
>        ext4_grp_locked_error(sb, group, 0, 0, "free %u, pa_free %u",
>                    free, pa->pa_free);
>        /*
> @@ -3798,7 +3801,8 @@ repeat:
>             * use preallocation while we're discarding it */
>            spin_unlock(&pa->pa_lock);
>            spin_unlock(&ei->i_prealloc_lock);
> -            printk(KERN_ERR "uh-oh! used pa while discarding\n");
> +            ext4_msg(sb, KERN_ERR,
> +                 "uh-oh! used pa while discarding");
>            WARN_ON(1);
>            schedule_timeout_uninterruptible(HZ);
>            goto repeat;
> @@ -3875,12 +3879,13 @@ static void ext4_mb_show_ac(struct ext4_allocation_context *ac)
>        (EXT4_SB(sb)->s_mount_flags & EXT4_MF_FS_ABORTED))
>        return;
> 
> -    printk(KERN_ERR "EXT4-fs: Can't allocate:"
> -            " Allocation context details:\n");
> -    printk(KERN_ERR "EXT4-fs: status %d flags %d\n",
> +    ext4_msg(ac->ac_sb, KERN_ERR, "EXT4-fs: Can't allocate:"

Ditto. 

> +            " Allocation context details:");
> +    ext4_msg(ac->ac_sb, KERN_ERR, "EXT4-fs: status %d flags %d",
>            ac->ac_status, ac->ac_flags);
> -    printk(KERN_ERR "EXT4-fs: orig %lu/%lu/%lu@%lu, goal %lu/%lu/%lu@%lu, "
> -            "best %lu/%lu/%lu@%lu cr %d\n",
> +    ext4_msg(ac->ac_sb, KERN_ERR, "EXT4-fs: orig %lu/%lu/%lu@%lu, "

Here too. 

> +            "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,
> @@ -3894,9 +3899,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);
> -    printk(KERN_ERR "EXT4-fs: %lu scanned, %d found\n", ac->ac_ex_scanned,
> -        ac->ac_found);
> -    printk(KERN_ERR "EXT4-fs: groups: \n");
> +    ext4_msg(ac->ac_sb, KERN_ERR, "EXT4-fs: %lu scanned, %d found",

...

> +         ac->ac_ex_scanned, ac->ac_found);
> +    ext4_msg(ac->ac_sb, KERN_ERR, "EXT4-fs: groups: ");
>    ngroups = ext4_get_groups_count(sb);
>    for (i = 0; i < ngroups; i++) {
>        struct ext4_group_info *grp = ext4_get_group_info(sb, i);
> -- 
> 1.7.4.1.22.gec8e1.dirty
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index d5021e8..262fa47 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -493,10 +493,11 @@  static void mb_cmp_bitmaps(struct ext4_buddy *e4b, void *bitmap)
 		b2 = (unsigned char *) bitmap;
 		for (i = 0; i < e4b->bd_sb->s_blocksize; i++) {
 			if (b1[i] != b2[i]) {
-				printk(KERN_ERR "corruption in group %u "
-				       "at byte %u(%u): %x in copy != %x "
-				       "on disk/prealloc\n",
-				       e4b->bd_group, i, i * 8, 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",
+					 e4b->bd_group, i, i * 8, b1[i], b2[i]);
 				BUG();
 			}
 		}
@@ -2224,8 +2225,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) {
-			printk(KERN_ERR "EXT4-fs: can't allocate mem for a "
-			       "buddy group\n");
+			ext4_msg(sb, KERN_ERR, "EXT4-fs: can't allocate mem "
+				 "for a buddy group");
 			goto exit_meta_group_info;
 		}
 		sbi->s_group_info[group >> EXT4_DESC_PER_BLOCK_BITS(sb)] =
@@ -2238,7 +2239,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) {
-		printk(KERN_ERR "EXT4-fs: can't allocate buddy mem\n");
+		ext4_msg(sb, KERN_ERR, "EXT4-fs: can't allocate buddy mem");
 		goto exit_group_info;
 	}
 	memset(meta_group_info[i], 0, kmem_cache_size(cachep));
@@ -2333,12 +2334,12 @@  static int ext4_mb_init_backend(struct super_block *sb)
 	 * So a two level scheme suffices for now. */
 	sbi->s_group_info = ext4_kvzalloc(array_size, GFP_KERNEL);
 	if (sbi->s_group_info == NULL) {
-		printk(KERN_ERR "EXT4-fs: can't allocate buddy meta group\n");
+		ext4_msg(sb, KERN_ERR, "can't allocate buddy meta group");
 		return -ENOMEM;
 	}
 	sbi->s_buddy_cache = new_inode(sb);
 	if (sbi->s_buddy_cache == NULL) {
-		printk(KERN_ERR "EXT4-fs: can't get new inode\n");
+		ext4_msg(sb, KERN_ERR, "can't get new inode");
 		goto err_freesgi;
 	}
 	sbi->s_buddy_cache->i_ino = get_next_ino();
@@ -2346,8 +2347,7 @@  static int ext4_mb_init_backend(struct super_block *sb)
 	for (i = 0; i < ngroups; i++) {
 		desc = ext4_get_group_desc(sb, i, NULL);
 		if (desc == NULL) {
-			printk(KERN_ERR
-				"EXT4-fs: can't read descriptor %u\n", i);
+			ext4_msg(sb, KERN_ERR, "can't read descriptor %u", i);
 			goto err_freebuddy;
 		}
 		if (ext4_mb_add_groupinfo(sb, i, desc) != 0)
@@ -2411,7 +2411,8 @@  static int ext4_groupinfo_create_slab(size_t size)
 
 	mutex_unlock(&ext4_grpinfo_slab_create_mutex);
 	if (!cachep) {
-		printk(KERN_EMERG "EXT4: no memory for groupinfo slab cache\n");
+		printk(KERN_EMERG,
+		       "EXT4-fs: no memory for groupinfo slab cache\n");
 		return -ENOMEM;
 	}
 
@@ -2566,25 +2567,25 @@  int ext4_mb_release(struct super_block *sb)
 	if (sbi->s_buddy_cache)
 		iput(sbi->s_buddy_cache);
 	if (sbi->s_mb_stats) {
-		printk(KERN_INFO
-		       "EXT4-fs: mballoc: %u blocks %u reqs (%u success)\n",
+		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));
-		printk(KERN_INFO
-		      "EXT4-fs: mballoc: %u extents scanned, %u goal hits, "
-				"%u 2^N hits, %u breaks, %u lost\n",
+		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));
-		printk(KERN_INFO
-		       "EXT4-fs: mballoc: %lu generated and it took %Lu\n",
+		ext4_msg(sb, KERN_INFO,
+		       "mballoc: %lu generated and it took %Lu",
 				sbi->s_mb_buddies_generated,
 				sbi->s_mb_generation_time);
-		printk(KERN_INFO
-		       "EXT4-fs: mballoc: %u preallocated, %u discarded\n",
+		ext4_msg(sb, KERN_INFO,
+		       "mballoc: %u preallocated, %u discarded",
 				atomic_read(&sbi->s_mb_preallocated),
 				atomic_read(&sbi->s_mb_discarded));
 	}
@@ -3024,9 +3025,10 @@  ext4_mb_normalize_request(struct ext4_allocation_context *ac,
 
 	if (start + size <= ac->ac_o_ex.fe_logical &&
 			start > ac->ac_o_ex.fe_logical) {
-		printk(KERN_ERR "start %lu, size %lu, fe_logical %lu\n",
-			(unsigned long) start, (unsigned long) size,
-			(unsigned long) ac->ac_o_ex.fe_logical);
+		ext4_msg(ac->ac_sb, KERN_ERR,
+			 "start %lu, size %lu, fe_logical %lu",
+			 (unsigned long) start, (unsigned long) size,
+			 (unsigned long) ac->ac_o_ex.fe_logical);
 	}
 	BUG_ON(start + size <= ac->ac_o_ex.fe_logical &&
 			start > ac->ac_o_ex.fe_logical);
@@ -3607,10 +3609,11 @@  ext4_mb_release_inode_pa(struct ext4_buddy *e4b, struct buffer_head *bitmap_bh,
 		bit = next + 1;
 	}
 	if (free != pa->pa_free) {
-		printk(KERN_CRIT "pa %p: logic %lu, phys. %lu, len %lu\n",
-			pa, (unsigned long) pa->pa_lstart,
-			(unsigned long) pa->pa_pstart,
-			(unsigned long) pa->pa_len);
+		ext4_msg(e4b->bd_sb, KERN_CRIT,
+			 "pa %p: logic %lu, phys. %lu, len %lu",
+			 pa, (unsigned long) pa->pa_lstart,
+			 (unsigned long) pa->pa_pstart,
+			 (unsigned long) pa->pa_len);
 		ext4_grp_locked_error(sb, group, 0, 0, "free %u, pa_free %u",
 					free, pa->pa_free);
 		/*
@@ -3798,7 +3801,8 @@  repeat:
 			 * use preallocation while we're discarding it */
 			spin_unlock(&pa->pa_lock);
 			spin_unlock(&ei->i_prealloc_lock);
-			printk(KERN_ERR "uh-oh! used pa while discarding\n");
+			ext4_msg(sb, KERN_ERR,
+				 "uh-oh! used pa while discarding");
 			WARN_ON(1);
 			schedule_timeout_uninterruptible(HZ);
 			goto repeat;
@@ -3875,12 +3879,13 @@  static void ext4_mb_show_ac(struct ext4_allocation_context *ac)
 	    (EXT4_SB(sb)->s_mount_flags & EXT4_MF_FS_ABORTED))
 		return;
 
-	printk(KERN_ERR "EXT4-fs: Can't allocate:"
-			" Allocation context details:\n");
-	printk(KERN_ERR "EXT4-fs: status %d flags %d\n",
+	ext4_msg(ac->ac_sb, KERN_ERR, "EXT4-fs: Can't allocate:"
+			" Allocation context details:");
+	ext4_msg(ac->ac_sb, KERN_ERR, "EXT4-fs: status %d flags %d",
 			ac->ac_status, ac->ac_flags);
-	printk(KERN_ERR "EXT4-fs: orig %lu/%lu/%lu@%lu, goal %lu/%lu/%lu@%lu, "
-			"best %lu/%lu/%lu@%lu cr %d\n",
+	ext4_msg(ac->ac_sb, KERN_ERR, "EXT4-fs: 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,
@@ -3894,9 +3899,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);
-	printk(KERN_ERR "EXT4-fs: %lu scanned, %d found\n", ac->ac_ex_scanned,
-		ac->ac_found);
-	printk(KERN_ERR "EXT4-fs: groups: \n");
+	ext4_msg(ac->ac_sb, KERN_ERR, "EXT4-fs: %lu scanned, %d found",
+		 ac->ac_ex_scanned, ac->ac_found);
+	ext4_msg(ac->ac_sb, KERN_ERR, "EXT4-fs: groups: ");
 	ngroups = ext4_get_groups_count(sb);
 	for (i = 0; i < ngroups; i++) {
 		struct ext4_group_info *grp = ext4_get_group_info(sb, i);