Re: [PATCH] jbd: Lower severity of aborted journal from EMERG toCRIT

[Lubomir Rintel]

On Mon, 2013-11-18 at 17:45 +0100, Jan Kara wrote:
> On Fri 15-11-13 15:56:52, Lubomir Rintel wrote:
> > According to Documentation/kernel-parameters.txt, KERN_EMERG is reserved for
> > events that render system unusable.
> > 
> > This is hardly the case in case of flash memory stick hardware removal without
> > umounting. Syslog is often configured to forward messages with EMERG severity
> > to all logged-in terminals, often causing unnecessary noise.
>   The logic behind using KERN_EMERG in that place is that the filesystem is
> dead. If it was an important filesystem in your system, the whole system is
> unusable. In kernel, we don't know whether the filesystem was important or
> not. So KERN_EMERG isn't adequate in all the cases but KERN_CRIT is
> neither. What if we made that message print also device name (it would be
> more useful anyway in that case) and you could then filter out messages for
> unimportant devices in syslogd?

In case there's a failure on a filesystem, there are other better means
in place to communicate the issue than a message in message buffer; in
such case the accesses to the filesystem will result in EIO (that is for
important filesystems; for the memory sticks that disappear from system
I presume userspace would just umount them).

Therefore I don't think it makes sense to differentiate between
important and less important file systems in this particular place and I
can't think of a good way do do that either. Even if there were devices
for which EMERG level here would make sense I don't think replacing the
stock syslog configurations with a quirk for this particular message is
feasible; and to my knowledge at least rsyslog is not able to understand
structured logs it the way journald does.

That said I don't mind replacing the printk(KERN_*) with dev_*() (maybe
even other occurrences, checkpatch.pl suggests that anyway and it seems
rather useful) but I'd still like it to be kern_crit().

What do you think about that?

Lubo

> > Signed-off-by: Lubomir Rintel <lkundrak@xxxxx>
> > ---
> >  fs/jbd/journal.c  |    2 +-
> >  fs/jbd2/journal.c |    2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/jbd/journal.c b/fs/jbd/journal.c
> > index 2d04f9a..9dbc1b6 100644
> > --- a/fs/jbd/journal.c
> > +++ b/fs/jbd/journal.c
> > @@ -605,7 +605,7 @@ out_unlock:
> >  	spin_unlock(&journal->j_state_lock);
> >  
> >  	if (unlikely(is_journal_aborted(journal))) {
> > -		printk(KERN_EMERG "journal commit I/O error\n");
> > +		printk(KERN_CRIT "journal commit I/O error\n");
> >  		err = -EIO;
> >  	}
> >  	return err;
> > diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> > index 5203264..2d1f53c 100644
> > --- a/fs/jbd2/journal.c
> > +++ b/fs/jbd2/journal.c
> > @@ -719,7 +719,7 @@ int jbd2_log_wait_commit(journal_t *journal, tid_t tid)
> >  	read_unlock(&journal->j_state_lock);
> >  
> >  	if (unlikely(is_journal_aborted(journal))) {
> > -		printk(KERN_EMERG "journal commit I/O error\n");
> > +		printk(KERN_CRIT "journal commit I/O error\n");
> >  		err = -EIO;
> >  	}
> >  	return err;
> > -- 
> > 1.7.1
> > 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/