* Re: [PATCH] Make some ext3 kernel messages useful by showing device [not found] <1212951761.32756.4.camel@localhost> @ 2008-06-09 9:59 ` Andrew Morton 2008-06-09 13:38 ` Kasper Sandberg 2008-06-09 13:43 ` Eric Sandeen 0 siblings, 2 replies; 6+ messages in thread From: Andrew Morton @ 2008-06-09 9:59 UTC (permalink / raw) To: Kasper Sandberg; +Cc: LKML Mailinglist, linux-ext4 On Sun, 08 Jun 2008 21:02:41 +0200 Kasper Sandberg <lkml@metanurb.dk> wrote: > Hello. > > Some of the ext3 warnings in super.c are not really as useful as they > can be, for instance the "EXT3-fs warning: maximal mount count reached, > running e2fsck is recommended" message, does not tell which device it > actually is. Seems sensible. > Below is patch(both inlined, and in attached form, since i dont trust my > mailer), That really tricks me. Because the resulting file applies nicely with `patch --dry-run' but doesn't apply with plain old `patch'. Inlined is preferred, attached is grumpily accepted, but please avoid duplicating the patch. > to a patch which fixes that particular message, and a few more. > I could look at the rest if anyones interrested? We like to keep ext3 and ext4 in sync as much as poss, please. > Oh, and i dont really know if this is nessecary, but: > Signed-off-by: Kasper Sandberg <lkml@metanurb.dk> It is very much preferred, thanks. (patch retained for linux-ext4 review) > > --- super.c.orig 2008-06-08 20:49:26.153047364 +0200 > +++ super.c 2008-06-08 20:45:20.812047463 +0200 > @@ -1188,31 +1188,31 @@ > int res = 0; > > if (le32_to_cpu(es->s_rev_level) > EXT3_MAX_SUPP_REV) { > - printk (KERN_ERR "EXT3-fs warning: revision level too high, " > - "forcing read-only mode\n"); > + printk (KERN_ERR "EXT3-fs on %s warning: revision level too high, " > + "forcing read-only mode\n", sb->s_id); > res = MS_RDONLY; > } > if (read_only) > return res; > if (!(sbi->s_mount_state & EXT3_VALID_FS)) > - printk (KERN_WARNING "EXT3-fs warning: mounting unchecked fs, " > - "running e2fsck is recommended\n"); > + printk (KERN_WARNING "EXT3-fs on %s warning: mounting unchecked fs, " > + "running e2fsck is recommended\n", sb->s_id); > else if ((sbi->s_mount_state & EXT3_ERROR_FS)) > printk (KERN_WARNING > - "EXT3-fs warning: mounting fs with errors, " > - "running e2fsck is recommended\n"); > + "EXT3-fs on %s warning: mounting fs with errors, " > + "running e2fsck is recommended\n", sb->s_id); > 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)) > printk (KERN_WARNING > - "EXT3-fs warning: maximal mount count reached, " > - "running e2fsck is recommended\n"); > + "EXT3-fs on %s warning: maximal mount count reached, " > + "running e2fsck is recommended\n", sb->s_id); > else if (le32_to_cpu(es->s_checkinterval) && > (le32_to_cpu(es->s_lastcheck) + > le32_to_cpu(es->s_checkinterval) <= get_seconds())) > printk (KERN_WARNING > - "EXT3-fs warning: checktime reached, " > - "running e2fsck is recommended\n"); > + "EXT3-fs on %s warning: checktime reached, " > + "running e2fsck is recommended\n", sb->s_id); > #if 0 > /* @@@ We _will_ want to clear the valid bit if we find > inconsistencies, to force a fsck at reboot. But for > @@ -1339,8 +1339,8 @@ > } > > if (bdev_read_only(sb->s_bdev)) { > - printk(KERN_ERR "EXT3-fs: write access " > - "unavailable, skipping orphan cleanup.\n"); > + printk(KERN_ERR "EXT3-fs on %s: write access " > + "unavailable, skipping orphan cleanup.\n", sb->s_id); > return; > } > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Make some ext3 kernel messages useful by showing device 2008-06-09 9:59 ` [PATCH] Make some ext3 kernel messages useful by showing device Andrew Morton @ 2008-06-09 13:38 ` Kasper Sandberg 2008-06-09 20:48 ` Andrew Morton 2008-06-09 13:43 ` Eric Sandeen 1 sibling, 1 reply; 6+ messages in thread From: Kasper Sandberg @ 2008-06-09 13:38 UTC (permalink / raw) To: Andrew Morton; +Cc: LKML Mailinglist, linux-ext4 On Mon, 2008-06-09 at 02:59 -0700, Andrew Morton wrote: > On Sun, 08 Jun 2008 21:02:41 +0200 Kasper Sandberg <lkml@metanurb.dk> wrote: > > > Hello. > > > > Some of the ext3 warnings in super.c are not really as useful as they > > can be, for instance the "EXT3-fs warning: maximal mount count reached, > > running e2fsck is recommended" message, does not tell which device it > > actually is. > > Seems sensible. > > > Below is patch(both inlined, and in attached form, since i dont trust my > > mailer), > > That really tricks me. Because the resulting file applies nicely with > `patch --dry-run' but doesn't apply with plain old `patch'. Inlined is > preferred, attached is grumpily accepted, but please avoid duplicating > the patch. I do not know what happened, i did with diff -Naur. I attached because of the fact that i _KNOW_ evolution is not to be trusted with inlined, but i also know inlined is easier for review, even if it is not properly accepted as input for patch. > > > to a patch which fixes that particular message, and a few more. > > I could look at the rest if anyones interrested? > > We like to keep ext3 and ext4 in sync as much as poss, please. Yes, i will do this soon, i have a few things to do first though. > > > Oh, and i dont really know if this is nessecary, but: > > Signed-off-by: Kasper Sandberg <lkml@metanurb.dk> > > It is very much preferred, thanks. > > (patch retained for linux-ext4 review) > > > > > --- super.c.orig 2008-06-08 20:49:26.153047364 +0200 > > +++ super.c 2008-06-08 20:45:20.812047463 +0200 > > @@ -1188,31 +1188,31 @@ > > int res = 0; > > > > if (le32_to_cpu(es->s_rev_level) > EXT3_MAX_SUPP_REV) { > > - printk (KERN_ERR "EXT3-fs warning: revision level too high, " > > - "forcing read-only mode\n"); > > + printk (KERN_ERR "EXT3-fs on %s warning: revision level too high, " > > + "forcing read-only mode\n", sb->s_id); > > res = MS_RDONLY; > > } > > if (read_only) > > return res; > > if (!(sbi->s_mount_state & EXT3_VALID_FS)) > > - printk (KERN_WARNING "EXT3-fs warning: mounting unchecked fs, " > > - "running e2fsck is recommended\n"); > > + printk (KERN_WARNING "EXT3-fs on %s warning: mounting unchecked fs, " > > + "running e2fsck is recommended\n", sb->s_id); > > else if ((sbi->s_mount_state & EXT3_ERROR_FS)) > > printk (KERN_WARNING > > - "EXT3-fs warning: mounting fs with errors, " > > - "running e2fsck is recommended\n"); > > + "EXT3-fs on %s warning: mounting fs with errors, " > > + "running e2fsck is recommended\n", sb->s_id); > > 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)) > > printk (KERN_WARNING > > - "EXT3-fs warning: maximal mount count reached, " > > - "running e2fsck is recommended\n"); > > + "EXT3-fs on %s warning: maximal mount count reached, " > > + "running e2fsck is recommended\n", sb->s_id); > > else if (le32_to_cpu(es->s_checkinterval) && > > (le32_to_cpu(es->s_lastcheck) + > > le32_to_cpu(es->s_checkinterval) <= get_seconds())) > > printk (KERN_WARNING > > - "EXT3-fs warning: checktime reached, " > > - "running e2fsck is recommended\n"); > > + "EXT3-fs on %s warning: checktime reached, " > > + "running e2fsck is recommended\n", sb->s_id); > > #if 0 > > /* @@@ We _will_ want to clear the valid bit if we find > > inconsistencies, to force a fsck at reboot. But for > > @@ -1339,8 +1339,8 @@ > > } > > > > if (bdev_read_only(sb->s_bdev)) { > > - printk(KERN_ERR "EXT3-fs: write access " > > - "unavailable, skipping orphan cleanup.\n"); > > + printk(KERN_ERR "EXT3-fs on %s: write access " > > + "unavailable, skipping orphan cleanup.\n", sb->s_id); > > return; > > } > > > > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Make some ext3 kernel messages useful by showing device 2008-06-09 13:38 ` Kasper Sandberg @ 2008-06-09 20:48 ` Andrew Morton 0 siblings, 0 replies; 6+ messages in thread From: Andrew Morton @ 2008-06-09 20:48 UTC (permalink / raw) To: Kasper Sandberg; +Cc: linux-kernel, linux-ext4 On Mon, 09 Jun 2008 15:38:10 +0200 Kasper Sandberg <lkml@metanurb.dk> wrote: > On Mon, 2008-06-09 at 02:59 -0700, Andrew Morton wrote: > > On Sun, 08 Jun 2008 21:02:41 +0200 Kasper Sandberg <lkml@metanurb.dk> wrote: > > > > > Hello. > > > > > > Some of the ext3 warnings in super.c are not really as useful as they > > > can be, for instance the "EXT3-fs warning: maximal mount count reached, > > > running e2fsck is recommended" message, does not tell which device it > > > actually is. > > > > Seems sensible. > > > > > Below is patch(both inlined, and in attached form, since i dont trust my > > > mailer), > > > > That really tricks me. Because the resulting file applies nicely with > > `patch --dry-run' but doesn't apply with plain old `patch'. Inlined is > > preferred, attached is grumpily accepted, but please avoid duplicating > > the patch. > > I do not know what happened, i did with diff -Naur. I attached because > of the fact that i _KNOW_ evolution is not to be trusted with inlined, > but i also know inlined is easier for review, even if it is not properly > accepted as input for patch. What happened was that you inlined the patch and also attached it. So when my email client does save-to-file, I get two copies of the same patch in the one local file. This causes me problems. I suggest that you try to inline the patches. Email yourself a patch, check that it still applies. If this ends up being too hard then sure, use attachments, but please don't also inline the patch. > > > > > to a patch which fixes that particular message, and a few more. > > > I could look at the rest if anyones interrested? > > > > We like to keep ext3 and ext4 in sync as much as poss, please. > Yes, i will do this soon, i have a few things to do first though. No probs, thanks. > > > > > Oh, and i dont really know if this is nessecary, but: > > > Signed-off-by: Kasper Sandberg <lkml@metanurb.dk> > > > > It is very much preferred, thanks. > > > > (patch retained for linux-ext4 review) > > > > > > > > --- super.c.orig 2008-06-08 20:49:26.153047364 +0200 > > > +++ super.c 2008-06-08 20:45:20.812047463 +0200 btw, please prepare patches in `patch -p1' form. The headers here should have been --- a/fs/ext3/super.c +++ a/fs/ext3/super.c So the patch will apply with `patch -p1' when run from the root of the kernel tree. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Make some ext3 kernel messages useful by showing device 2008-06-09 9:59 ` [PATCH] Make some ext3 kernel messages useful by showing device Andrew Morton 2008-06-09 13:38 ` Kasper Sandberg @ 2008-06-09 13:43 ` Eric Sandeen 2008-06-09 20:49 ` Andrew Morton 1 sibling, 1 reply; 6+ messages in thread From: Eric Sandeen @ 2008-06-09 13:43 UTC (permalink / raw) To: Andrew Morton; +Cc: Kasper Sandberg, LKML Mailinglist, linux-ext4 Andrew Morton wrote: > On Sun, 08 Jun 2008 21:02:41 +0200 Kasper Sandberg <lkml@metanurb.dk> wrote: > >> Hello. >> >> Some of the ext3 warnings in super.c are not really as useful as they >> can be, for instance the "EXT3-fs warning: maximal mount count reached, >> running e2fsck is recommended" message, does not tell which device it >> actually is. > > Seems sensible. > >> Below is patch(both inlined, and in attached form, since i dont trust my >> mailer), > > That really tricks me. Because the resulting file applies nicely with > `patch --dry-run' but doesn't apply with plain old `patch'. Inlined is > preferred, attached is grumpily accepted, but please avoid duplicating > the patch. > >> to a patch which fixes that particular message, and a few more. >> I could look at the rest if anyones interrested? > > We like to keep ext3 and ext4 in sync as much as poss, please. > >> Oh, and i dont really know if this is nessecary, but: >> Signed-off-by: Kasper Sandberg <lkml@metanurb.dk> > > It is very much preferred, thanks. > > (patch retained for linux-ext4 review) The patch itself looks fine to me, and is a good idea. I recently saw a confused user on another fs where successful mount messages were followed by failure mount messages with no indication of which fs had the trouble... There probably are more spots in extX which could use this treatment... should we maybe just be using ext3_warning in more places, which automatically prints the sb, function name, etc? -Eric >> --- super.c.orig 2008-06-08 20:49:26.153047364 +0200 >> +++ super.c 2008-06-08 20:45:20.812047463 +0200 >> @@ -1188,31 +1188,31 @@ >> int res = 0; >> >> if (le32_to_cpu(es->s_rev_level) > EXT3_MAX_SUPP_REV) { >> - printk (KERN_ERR "EXT3-fs warning: revision level too high, " >> - "forcing read-only mode\n"); >> + printk (KERN_ERR "EXT3-fs on %s warning: revision level too high, " >> + "forcing read-only mode\n", sb->s_id); >> res = MS_RDONLY; >> } >> if (read_only) >> return res; >> if (!(sbi->s_mount_state & EXT3_VALID_FS)) >> - printk (KERN_WARNING "EXT3-fs warning: mounting unchecked fs, " >> - "running e2fsck is recommended\n"); >> + printk (KERN_WARNING "EXT3-fs on %s warning: mounting unchecked fs, " >> + "running e2fsck is recommended\n", sb->s_id); >> else if ((sbi->s_mount_state & EXT3_ERROR_FS)) >> printk (KERN_WARNING >> - "EXT3-fs warning: mounting fs with errors, " >> - "running e2fsck is recommended\n"); >> + "EXT3-fs on %s warning: mounting fs with errors, " >> + "running e2fsck is recommended\n", sb->s_id); >> 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)) >> printk (KERN_WARNING >> - "EXT3-fs warning: maximal mount count reached, " >> - "running e2fsck is recommended\n"); >> + "EXT3-fs on %s warning: maximal mount count reached, " >> + "running e2fsck is recommended\n", sb->s_id); >> else if (le32_to_cpu(es->s_checkinterval) && >> (le32_to_cpu(es->s_lastcheck) + >> le32_to_cpu(es->s_checkinterval) <= get_seconds())) >> printk (KERN_WARNING >> - "EXT3-fs warning: checktime reached, " >> - "running e2fsck is recommended\n"); >> + "EXT3-fs on %s warning: checktime reached, " >> + "running e2fsck is recommended\n", sb->s_id); >> #if 0 >> /* @@@ We _will_ want to clear the valid bit if we find >> inconsistencies, to force a fsck at reboot. But for >> @@ -1339,8 +1339,8 @@ >> } >> >> if (bdev_read_only(sb->s_bdev)) { >> - printk(KERN_ERR "EXT3-fs: write access " >> - "unavailable, skipping orphan cleanup.\n"); >> + printk(KERN_ERR "EXT3-fs on %s: write access " >> + "unavailable, skipping orphan cleanup.\n", sb->s_id); >> return; >> } >> >> >> > -- > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Make some ext3 kernel messages useful by showing device 2008-06-09 13:43 ` Eric Sandeen @ 2008-06-09 20:49 ` Andrew Morton 2008-06-09 22:08 ` Andreas Dilger 0 siblings, 1 reply; 6+ messages in thread From: Andrew Morton @ 2008-06-09 20:49 UTC (permalink / raw) To: Eric Sandeen; +Cc: lkml, linux-kernel, linux-ext4 On Mon, 09 Jun 2008 08:43:20 -0500 Eric Sandeen <sandeen@redhat.com> wrote: > > > > It is very much preferred, thanks. > > > > (patch retained for linux-ext4 review) > > The patch itself looks fine to me, and is a good idea. I recently saw a > confused user on another fs where successful mount messages were > followed by failure mount messages with no indication of which fs had > the trouble... > > There probably are more spots in extX which could use this treatment... > should we maybe just be using ext3_warning in more places, which > automatically prints the sb, function name, etc? Good point, thanks. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Make some ext3 kernel messages useful by showing device 2008-06-09 20:49 ` Andrew Morton @ 2008-06-09 22:08 ` Andreas Dilger 0 siblings, 0 replies; 6+ messages in thread From: Andreas Dilger @ 2008-06-09 22:08 UTC (permalink / raw) To: Eric Sandeen; +Cc: Andrew Morton, lkml, linux-kernel, linux-ext4 Eric Sandeen <sandeen@redhat.com> wrote: > The patch itself looks fine to me, and is a good idea. I recently saw a > confused user on another fs where successful mount messages were > followed by failure mount messages with no indication of which fs had > the trouble... > > There probably are more spots in extX which could use this treatment... > should we maybe just be using ext3_warning in more places, which > automatically prints the sb, function name, etc? Most of the messages in ext[234]_fill_super() should really be ext3_error(), but because the superblock isn't properly set up calling ext3_error() is a way to oops the kernel. Probably the cleanest solution is to split ext3_error() into a helper function ext3_error_msg() that can be called from ext3_fill_super(), and the rest of the code that now lives in ext3_handle_error(). Alternately, a flag could be set in the superblock ("EXT3_SETUP_DONE") once everything is initialized, and then ext3_error() calls ext3_handle_error(), and otherwise just prints the message and the mount will fail by itself. This has the benefit that we can use ext3_error() everywhere there is a fatal problem, but the drawback that it may become confusing due to the added "errors=*" semantics that will not behave as they do in other functions. Cheers, Andreas -- Andreas Dilger Sr. Staff Engineer, Lustre Group Sun Microsystems of Canada, Inc. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-06-09 22:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1212951761.32756.4.camel@localhost>
2008-06-09 9:59 ` [PATCH] Make some ext3 kernel messages useful by showing device Andrew Morton
2008-06-09 13:38 ` Kasper Sandberg
2008-06-09 20:48 ` Andrew Morton
2008-06-09 13:43 ` Eric Sandeen
2008-06-09 20:49 ` Andrew Morton
2008-06-09 22:08 ` Andreas Dilger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox