On Mon, 2014-06-30 at 04:01 +0900, Ryusuke Konishi wrote:
> On Sun, 29 Jun 2014 18:03:41 +0400, Vyacheslav Dubeyko wrote:
> > From: Vyacheslav Dubeyko <Vyacheslav.Dubeyko@xxxxxxxx>
> > Subject: [PATCH] nilfs2: suggest to use dump_stack() in nilfs_error() and nilfs_warning()
> >
> > This patch suggests to use dump_stack() call in nilfs_error() and
> > nilfs_warning() for more clear understanding of different type of
> > issues. As a result, end-users can report more informative
> > descriptions of issues.
> >
> > Signed-off-by: Vyacheslav Dubeyko <Vyacheslav.Dubeyko@xxxxxxxx>
> > CC: Vyacheslav Dubeyko <slava@xxxxxxxxxxx>
> > CC: Ryusuke Konishi <konishi.ryusuke@xxxxxxxxxxxxx>
>
> NAK.
>
> This patch makes error/warning routine of nilfs so verbose.
>
> dump_stack() should be used for some critical situations or internal
> inconsisent situtations, or for cases in which the identifying call
> path of trouble is not easy.
>
> I think it's not used for regular errors or warnings because they are
> mostly identifiable.
>
> In addition, "errors=panic" mount option is available for nilfs_error
> to force stack dump.
>
I worry about end-user side. Yes, if developer investigates the issue
then he has many opportunities for the investigation. But we have very
frequently end-users' reports with not very informative details. And it
is not so easy to identify the reason of an issue very frequently. We've
spent about a year on trying to identify the reason of the issue with
race condition of competition between segments for dirty blocks. And the
reason of such situation was impossibility to reproduce the issue easily
and to understand situation on end-users' side. So, from my point of
view, it makes sense to show dump_stack() for the case of remount in RO
mode or any warnings.
I think that NILFS2 driver contains many places without necessary output
about errors. For example:
[160281.690959] nilfs_btree_propagate: key = 10, level == 0
[160281.690967] NILFS warning (device dm-0): nilfs_clean_segments:
segment construction failed. (err=-2)
Yes, it is possible to understand that we have some error situation on
segctor side. And somewhere inside of segctor thread took place
situation with -ENOENT error. But it is really hard to understand the
place and environment of such situation. I suppose that if we will have
more detailed error messages output then it will be more easily to
understand an issue's environment and the reason of it.
How do you feel about enhancement of output for the case of errors? I
mean to add messages for such places:
err = some_func();
if (err) {
/* add message output here with details */
return err;
}
Thanks,
Vyacheslav Dubeyko.
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
