Re: [PATCH 1/3] xfs: add xfs_error_report the ability to display an error code

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Sep 08, 2026 at 11:53:57AM -0500, Eric Sandeen wrote:
> On 9/8/26 9:54 AM, cem@kernel.org wrote:
> > From: Carlos Maiolino <cem@kernel.org>
> > 
> > Users could opt to request an error code to be printed now.
> > This relies on errname() and CONFIG_SYMBOLIC_NAME without requiring it
> > to be enabled.
> > Report 'unkown error' If CONFIG_SYMBOLIC_NAME is not enabled or the
> > user passes 0 as an error.
> 
> s/SYMBOLIC_NAME/SYMBOLIC_ERRNAME/g
> 
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > ---
> >  fs/xfs/xfs_error.c         | 13 ++++++++++---
> >  fs/xfs/xfs_error.h         | 10 +++++-----
> >  fs/xfs/xfs_exchmaps_item.c |  9 ++++++---
> >  fs/xfs/xfs_inode_item.c    |  3 ++-
> >  fs/xfs/xfs_log_recover.c   |  3 ++-
> >  fs/xfs/xfs_trans.c         |  2 +-
> >  6 files changed, 26 insertions(+), 14 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> > index dbd87e137694..3177ec58bb2e 100644
> > --- a/fs/xfs/xfs_error.c
> > +++ b/fs/xfs/xfs_error.c
> > @@ -14,6 +14,7 @@
> >  #include "xfs_error.h"
> >  #include "xfs_sysfs.h"
> >  #include "xfs_inode.h"
> > +#include <linux/errname.h>
> >  
> >  #ifdef DEBUG
> >  
> > @@ -377,15 +378,21 @@ void
> >  xfs_error_report(
> >  	const char		*tag,
> >  	int			level,
> > +	int			error,
> >  	struct xfs_mount	*mp,
> >  	const char		*filename,
> >  	int			linenum,
> >  	xfs_failaddr_t		failaddr)
> >  {
> > +	const char *err_str = errname(error);
> > +
> > +	if (!err_str)
> > +		err_str = "unknown error";
> > +
> >  	if (level <= xfs_error_level) {
> >  		xfs_alert_tag(mp, XFS_PTAG_ERROR_REPORT,
> > -		"Internal error %s at line %d of file %s.  Caller %pS",
> > -			    tag, linenum, filename, failaddr);
> > +		"Internal error %s (%s) at line %d of file %s.  Caller %pS",
> > +			    tag, err_str, linenum, filename, failaddr);
> I might suggest making the printk conditional, rather than making err_str conditional.
> 
> It might make the code messier, but the entire purpose of this change is to give
> the user more information and a clearer view of what happened. Changing
> "error 93 at line" to "error 93 (unknown error) at line" doesn't really do that,
> and IMHO adds confusion and adds no value.

Oh, something like:

	if (level <= xfs_error_level) {
		const char	*err_str = errname(error);

		if (err_str)
			xfs_alert_tag(mp, XFS_PTAG_ERROR_REPORT,
 "Internal error %s (%s) at line...", tag, err_str, ...);
		else
			xfs_alert_tag(mp, XFS_PTAG_ERROR_REPORT,
 "Internal error %s (%d) at line...", tag, error, ...);
	}

Hm?

I sorta like having the #define'd error name in the log message (e.g.
"Internal error frogs (ENOENT) at line..." but tbh these days I just run
errno(1) to translate an errno to a sentence fragment.

--D

> It sounds like the config is typically enabled, but I'd still rather not add
> extra words to debugging printks that add no useful info if possible. It may
> raise more questions than it answers.
> 
> overall I like the idea though :)
> 
> -Eric
>