[PATCH 0/3] Enable xfs error reporting to print out error codes

[PATCH 0/3] Enable xfs error reporting to print out error codes

[cem]

From: Carlos Maiolino <cem@kernel.org>

Hello.

This is the first 'formal' version of this series after a round of
RFC...

The goal remains the same, enabling us to easily find what error
caused a transaction to be cancelled. But the infra-structure is usable
for remaining error cases.

The main updates regarding the RFC version is xfs_error_report now
uses errname() to print out the error.

Also gets rid of the unneeded extern modifier from it.

Instead of a variadic function uses a couple different macros to select
between with/without error. I named it xfs_trans_cancel_error() because
it sounded clear to me than the _with() suffix. I'd not join an argument
to defent one or another though if you guys are unhappy with _error,
I'll change back to _with().

Last patch is simple enough and I kept Darrick's review on it as there
is not much change in there other than setting up the right macro.

Cheers.

Carlos Maiolino (3):
  xfs: add xfs_error_report the ability to display an error code
  xfs: enable xfs_trans_cancel() to report and error code
  xfs: make xfs_iomap_write_direct() report an error to xfs_trans_cancel

 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_iomap.c         |  2 +-
 fs/xfs/xfs_log_recover.c   |  3 ++-
 fs/xfs/xfs_trans.c         |  8 +++++---
 fs/xfs/xfs_trans.h         |  6 +++++-
 8 files changed, 36 insertions(+), 18 deletions(-)

-- 
2.55.0

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

[cem]

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.

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);
 
 		xfs_stack_trace();
 	}
@@ -404,7 +411,7 @@ xfs_corruption_error(
 {
 	if (buf && level <= xfs_error_level)
 		xfs_hex_dump(buf, bufsize);
-	xfs_error_report(tag, level, mp, filename, linenum, failaddr);
+	xfs_error_report(tag, level, 0, mp, filename, linenum, failaddr);
 	xfs_alert(mp, "Corruption detected. Unmount and run xfs_repair");
 }
 
diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
index 0b9c5ba8a598..f3b5564c2f4f 100644
--- a/fs/xfs/xfs_error.h
+++ b/fs/xfs/xfs_error.h
@@ -8,9 +8,9 @@
 
 struct xfs_mount;
 
-extern void xfs_error_report(const char *tag, int level, struct xfs_mount *mp,
-			const char *filename, int linenum,
-			xfs_failaddr_t failaddr);
+void xfs_error_report(const char *tag, int level, int error,
+		struct xfs_mount *mp, const char *filename,
+		int linenum, xfs_failaddr_t failaddr);
 extern void xfs_corruption_error(const char *tag, int level,
 			struct xfs_mount *mp, const void *buf, size_t bufsize,
 			const char *filename, int linenum,
@@ -25,8 +25,8 @@ extern void xfs_inode_verifier_error(struct xfs_inode *ip, int error,
 			const char *name, const void *buf, size_t bufsz,
 			xfs_failaddr_t failaddr);
 
-#define	XFS_ERROR_REPORT(e, lvl, mp)	\
-	xfs_error_report(e, lvl, mp, __FILE__, __LINE__, __return_address)
+#define	XFS_ERROR_REPORT(e, lvl, error, mp)	\
+	xfs_error_report(e, lvl, error, mp, __FILE__, __LINE__, __return_address)
 #define	XFS_CORRUPTION_ERROR(e, lvl, mp, buf, bufsize)	\
 	xfs_corruption_error(e, lvl, mp, buf, bufsize, \
 			     __FILE__, __LINE__, __return_address)
diff --git a/fs/xfs/xfs_exchmaps_item.c b/fs/xfs/xfs_exchmaps_item.c
index 264a121c5e16..2cbc1b9aee43 100644
--- a/fs/xfs/xfs_exchmaps_item.c
+++ b/fs/xfs/xfs_exchmaps_item.c
@@ -559,13 +559,15 @@ xlog_recover_xmi_commit_pass2(
 
 	len = sizeof(struct xfs_xmi_log_format);
 	if (item->ri_buf[0].i_len != len) {
-		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, log->l_mp);
+		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
+				 -EFSCORRUPTED, log->l_mp);
 		return -EFSCORRUPTED;
 	}
 
 	xmi_formatp = item->ri_buf[0].i_addr;
 	if (xmi_formatp->__pad != 0) {
-		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, log->l_mp);
+		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
+				 -EFSCORRUPTED, log->l_mp);
 		return -EFSCORRUPTED;
 	}
 
@@ -600,7 +602,8 @@ xlog_recover_xmd_commit_pass2(
 
 	xmd_formatp = item->ri_buf[0].i_addr;
 	if (item->ri_buf[0].i_len != sizeof(struct xfs_xmd_log_format)) {
-		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, log->l_mp);
+		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
+				 -EFSCORRUPTED, log->l_mp);
 		return -EFSCORRUPTED;
 	}
 
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index c6cb0b6b9e46..c361e02110a0 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -1185,7 +1185,8 @@ xfs_inode_item_format_convert(
 	struct xfs_inode_log_format_32	*in_f32 = buf->i_addr;
 
 	if (buf->i_len != sizeof(*in_f32)) {
-		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, NULL);
+		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
+				 -EFSCORRUPTED, NULL);
 		return -EFSCORRUPTED;
 	}
 
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 2f76531842f8..b1979d00d2e3 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -2932,7 +2932,8 @@ xlog_recover_process(
 		 * fatal log corruption failure.
 		 */
 		if (xfs_has_crc(log->l_mp)) {
-			XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, log->l_mp);
+			XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
+					 -EFSCORRUPTED, log->l_mp);
 			return -EFSCORRUPTED;
 		}
 	}
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index c6657072361a..792a67a7ec28 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -999,7 +999,7 @@ xfs_trans_cancel(
 	 * here.
 	 */
 	if (dirty && !xfs_is_shutdown(mp)) {
-		XFS_ERROR_REPORT("xfs_trans_cancel", XFS_ERRLEVEL_LOW, mp);
+		XFS_ERROR_REPORT("xfs_trans_cancel", XFS_ERRLEVEL_LOW, 0, mp);
 		xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
 	}
 #ifdef DEBUG
-- 
2.55.0

[PATCH 2/3] xfs: enable xfs_trans_cancel() to report and error code

[cem]

From: Carlos Maiolino <cem@kernel.org>

Once in a while it's useful to know exactly what kind of error caused a
transaction to be cancelled.
Enable xfs_trans_cancel() to receive an error code to be reported.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_trans.c | 8 +++++---
 fs/xfs/xfs_trans.h | 6 +++++-
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 792a67a7ec28..5bbbb9226864 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -971,8 +971,9 @@ xfs_trans_commit(
  * xfs_trans_commit().
  */
 void
-xfs_trans_cancel(
-	struct xfs_trans	*tp)
+__xfs_trans_cancel(
+	struct xfs_trans	*tp,
+	int			error)
 {
 	struct xfs_mount	*mp = tp->t_mountp;
 	struct xlog		*log = mp->m_log;
@@ -999,7 +1000,8 @@ xfs_trans_cancel(
 	 * here.
 	 */
 	if (dirty && !xfs_is_shutdown(mp)) {
-		XFS_ERROR_REPORT("xfs_trans_cancel", XFS_ERRLEVEL_LOW, 0, mp);
+		XFS_ERROR_REPORT("xfs_trans_cancel", XFS_ERRLEVEL_LOW,
+				 error, mp);
 		xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
 	}
 #ifdef DEBUG
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index 2b366851e9a4..30135152d043 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -213,6 +213,11 @@ xfs_trans_read_buf(
 				      flags, bpp, ops);
 }
 
+void __xfs_trans_cancel(xfs_trans_t *, int);
+
+#define xfs_trans_cancel(tp) __xfs_trans_cancel(tp, 0)
+#define xfs_trans_cancel_error(tp, error) __xfs_trans_cancel(tp, error)
+
 struct xfs_buf	*xfs_trans_getsb(struct xfs_trans *);
 struct xfs_buf	*xfs_trans_getrtsb(struct xfs_trans *tp);
 
@@ -237,7 +242,6 @@ void		xfs_trans_log_inode(xfs_trans_t *, struct xfs_inode *, uint);
 int		xfs_trans_commit(struct xfs_trans *);
 int		xfs_trans_roll(struct xfs_trans **);
 int		xfs_trans_roll_inode(struct xfs_trans **, struct xfs_inode *);
-void		xfs_trans_cancel(xfs_trans_t *);
 int		xfs_trans_ail_init(struct xfs_mount *);
 void		xfs_trans_ail_destroy(struct xfs_mount *);
 
-- 
2.55.0

[PATCH 3/3] xfs: make xfs_iomap_write_direct() report an error to xfs_trans_cancel

[cem]

From: Carlos Maiolino <cem@kernel.org>

I've been in the situation a few times where the transaction got
cancelled by some reason. Most of time because bmapi_write failed and I
start a guessing game trying to understand why.

So if xfs_iomap_write_direct fails and it causes the transaction to be
cancelled, send the error to xfs_trans_cancel so the error gets logged
into the kernel log buffer.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/xfs_iomap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index ff05e6b1b0bb..9fed19a34bd0 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -344,7 +344,7 @@ xfs_iomap_write_direct(
 	return error;
 
 out_trans_cancel:
-	xfs_trans_cancel(tp);
+	xfs_trans_cancel_error(tp, error);
 	goto out_unlock;
 }
 
-- 
2.55.0

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

[Darrick J. Wong]

On Tue, Sep 08, 2026 at 04:54:14PM +0200, 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.
> 
> 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";

Should it just snprintf the error number to a buffer?
> +
>  	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);
>  
>  		xfs_stack_trace();
>  	}
> @@ -404,7 +411,7 @@ xfs_corruption_error(
>  {
>  	if (buf && level <= xfs_error_level)
>  		xfs_hex_dump(buf, bufsize);
> -	xfs_error_report(tag, level, mp, filename, linenum, failaddr);
> +	xfs_error_report(tag, level, 0, mp, filename, linenum, failaddr);

Shouldn't this be -EFSCORRUPTED too?

--D

>  	xfs_alert(mp, "Corruption detected. Unmount and run xfs_repair");
>  }
>  
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index 0b9c5ba8a598..f3b5564c2f4f 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -8,9 +8,9 @@
>  
>  struct xfs_mount;
>  
> -extern void xfs_error_report(const char *tag, int level, struct xfs_mount *mp,
> -			const char *filename, int linenum,
> -			xfs_failaddr_t failaddr);
> +void xfs_error_report(const char *tag, int level, int error,
> +		struct xfs_mount *mp, const char *filename,
> +		int linenum, xfs_failaddr_t failaddr);
>  extern void xfs_corruption_error(const char *tag, int level,
>  			struct xfs_mount *mp, const void *buf, size_t bufsize,
>  			const char *filename, int linenum,
> @@ -25,8 +25,8 @@ extern void xfs_inode_verifier_error(struct xfs_inode *ip, int error,
>  			const char *name, const void *buf, size_t bufsz,
>  			xfs_failaddr_t failaddr);
>  
> -#define	XFS_ERROR_REPORT(e, lvl, mp)	\
> -	xfs_error_report(e, lvl, mp, __FILE__, __LINE__, __return_address)
> +#define	XFS_ERROR_REPORT(e, lvl, error, mp)	\
> +	xfs_error_report(e, lvl, error, mp, __FILE__, __LINE__, __return_address)
>  #define	XFS_CORRUPTION_ERROR(e, lvl, mp, buf, bufsize)	\
>  	xfs_corruption_error(e, lvl, mp, buf, bufsize, \
>  			     __FILE__, __LINE__, __return_address)
> diff --git a/fs/xfs/xfs_exchmaps_item.c b/fs/xfs/xfs_exchmaps_item.c
> index 264a121c5e16..2cbc1b9aee43 100644
> --- a/fs/xfs/xfs_exchmaps_item.c
> +++ b/fs/xfs/xfs_exchmaps_item.c
> @@ -559,13 +559,15 @@ xlog_recover_xmi_commit_pass2(
>  
>  	len = sizeof(struct xfs_xmi_log_format);
>  	if (item->ri_buf[0].i_len != len) {
> -		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, log->l_mp);
> +		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
> +				 -EFSCORRUPTED, log->l_mp);
>  		return -EFSCORRUPTED;
>  	}
>  
>  	xmi_formatp = item->ri_buf[0].i_addr;
>  	if (xmi_formatp->__pad != 0) {
> -		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, log->l_mp);
> +		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
> +				 -EFSCORRUPTED, log->l_mp);
>  		return -EFSCORRUPTED;
>  	}
>  
> @@ -600,7 +602,8 @@ xlog_recover_xmd_commit_pass2(
>  
>  	xmd_formatp = item->ri_buf[0].i_addr;
>  	if (item->ri_buf[0].i_len != sizeof(struct xfs_xmd_log_format)) {
> -		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, log->l_mp);
> +		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
> +				 -EFSCORRUPTED, log->l_mp);
>  		return -EFSCORRUPTED;
>  	}
>  
> diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> index c6cb0b6b9e46..c361e02110a0 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -1185,7 +1185,8 @@ xfs_inode_item_format_convert(
>  	struct xfs_inode_log_format_32	*in_f32 = buf->i_addr;
>  
>  	if (buf->i_len != sizeof(*in_f32)) {
> -		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, NULL);
> +		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
> +				 -EFSCORRUPTED, NULL);
>  		return -EFSCORRUPTED;
>  	}
>  
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 2f76531842f8..b1979d00d2e3 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -2932,7 +2932,8 @@ xlog_recover_process(
>  		 * fatal log corruption failure.
>  		 */
>  		if (xfs_has_crc(log->l_mp)) {
> -			XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, log->l_mp);
> +			XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
> +					 -EFSCORRUPTED, log->l_mp);
>  			return -EFSCORRUPTED;
>  		}
>  	}
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index c6657072361a..792a67a7ec28 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -999,7 +999,7 @@ xfs_trans_cancel(
>  	 * here.
>  	 */
>  	if (dirty && !xfs_is_shutdown(mp)) {
> -		XFS_ERROR_REPORT("xfs_trans_cancel", XFS_ERRLEVEL_LOW, mp);
> +		XFS_ERROR_REPORT("xfs_trans_cancel", XFS_ERRLEVEL_LOW, 0, mp);
>  		xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
>  	}
>  #ifdef DEBUG
> -- 
> 2.55.0
> 
> 

Re: [PATCH 2/3] xfs: enable xfs_trans_cancel() to report and error code

[Darrick J. Wong]

On Tue, Sep 08, 2026 at 04:54:15PM +0200, cem@kernel.org wrote:
> From: Carlos Maiolino <cem@kernel.org>
> 
> Once in a while it's useful to know exactly what kind of error caused a
> transaction to be cancelled.
> Enable xfs_trans_cancel() to receive an error code to be reported.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
>  fs/xfs/xfs_trans.c | 8 +++++---
>  fs/xfs/xfs_trans.h | 6 +++++-
>  2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index 792a67a7ec28..5bbbb9226864 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -971,8 +971,9 @@ xfs_trans_commit(
>   * xfs_trans_commit().
>   */
>  void
> -xfs_trans_cancel(
> -	struct xfs_trans	*tp)
> +__xfs_trans_cancel(
> +	struct xfs_trans	*tp,
> +	int			error)
>  {
>  	struct xfs_mount	*mp = tp->t_mountp;
>  	struct xlog		*log = mp->m_log;
> @@ -999,7 +1000,8 @@ xfs_trans_cancel(
>  	 * here.
>  	 */
>  	if (dirty && !xfs_is_shutdown(mp)) {
> -		XFS_ERROR_REPORT("xfs_trans_cancel", XFS_ERRLEVEL_LOW, 0, mp);
> +		XFS_ERROR_REPORT("xfs_trans_cancel", XFS_ERRLEVEL_LOW,
> +				 error, mp);
>  		xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
>  	}
>  #ifdef DEBUG
> diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
> index 2b366851e9a4..30135152d043 100644
> --- a/fs/xfs/xfs_trans.h
> +++ b/fs/xfs/xfs_trans.h
> @@ -213,6 +213,11 @@ xfs_trans_read_buf(
>  				      flags, bpp, ops);
>  }
>  
> +void __xfs_trans_cancel(xfs_trans_t *, int);
> +
> +#define xfs_trans_cancel(tp) __xfs_trans_cancel(tp, 0)
> +#define xfs_trans_cancel_error(tp, error) __xfs_trans_cancel(tp, error)

Nitpicking: the arguments should be wrapped in parentheses so you can't
do evil things to the preprocessor.  SOrry about being sloppy about
that.

#define xfs_trans_cancel_error(tp, error) \
	__xfs_trans_cancel((tp), (error))

--D

> +
>  struct xfs_buf	*xfs_trans_getsb(struct xfs_trans *);
>  struct xfs_buf	*xfs_trans_getrtsb(struct xfs_trans *tp);
>  
> @@ -237,7 +242,6 @@ void		xfs_trans_log_inode(xfs_trans_t *, struct xfs_inode *, uint);
>  int		xfs_trans_commit(struct xfs_trans *);
>  int		xfs_trans_roll(struct xfs_trans **);
>  int		xfs_trans_roll_inode(struct xfs_trans **, struct xfs_inode *);
> -void		xfs_trans_cancel(xfs_trans_t *);
>  int		xfs_trans_ail_init(struct xfs_mount *);
>  void		xfs_trans_ail_destroy(struct xfs_mount *);
>  
> -- 
> 2.55.0
> 
> 

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

[Eric Sandeen]

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.

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

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

[Darrick J. Wong]

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
> 

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

[Eric Sandeen]

On 9/8/26 12:02 PM, Darrick J. Wong wrote:
> 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?

Yeah I think so. Messy code but better dmesg IMHO.

> 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.
TBH I've lost track of what a tag vs an errno vs a string is, but I stand
by my observation that emitting "unknown error" when errname() returns null
seems like a net negative change. :)

-Eric

Re: [PATCH 2/3] xfs: enable xfs_trans_cancel() to report and error code

[Carlos Maiolino]

On Tue, Sep 08, 2026 at 08:27:54AM -0700, Darrick J. Wong wrote:
> On Tue, Sep 08, 2026 at 04:54:15PM +0200, cem@kernel.org wrote:
> > From: Carlos Maiolino <cem@kernel.org>
> > 
> > Once in a while it's useful to know exactly what kind of error caused a
> > transaction to be cancelled.
> > Enable xfs_trans_cancel() to receive an error code to be reported.
> > 
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > ---
> >  fs/xfs/xfs_trans.c | 8 +++++---
> >  fs/xfs/xfs_trans.h | 6 +++++-
> >  2 files changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> > index 792a67a7ec28..5bbbb9226864 100644
> > --- a/fs/xfs/xfs_trans.c
> > +++ b/fs/xfs/xfs_trans.c
> > @@ -971,8 +971,9 @@ xfs_trans_commit(
> >   * xfs_trans_commit().
> >   */
> >  void
> > -xfs_trans_cancel(
> > -	struct xfs_trans	*tp)
> > +__xfs_trans_cancel(
> > +	struct xfs_trans	*tp,
> > +	int			error)
> >  {
> >  	struct xfs_mount	*mp = tp->t_mountp;
> >  	struct xlog		*log = mp->m_log;
> > @@ -999,7 +1000,8 @@ xfs_trans_cancel(
> >  	 * here.
> >  	 */
> >  	if (dirty && !xfs_is_shutdown(mp)) {
> > -		XFS_ERROR_REPORT("xfs_trans_cancel", XFS_ERRLEVEL_LOW, 0, mp);
> > +		XFS_ERROR_REPORT("xfs_trans_cancel", XFS_ERRLEVEL_LOW,
> > +				 error, mp);
> >  		xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
> >  	}
> >  #ifdef DEBUG
> > diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
> > index 2b366851e9a4..30135152d043 100644
> > --- a/fs/xfs/xfs_trans.h
> > +++ b/fs/xfs/xfs_trans.h
> > @@ -213,6 +213,11 @@ xfs_trans_read_buf(
> >  				      flags, bpp, ops);
> >  }
> >  
> > +void __xfs_trans_cancel(xfs_trans_t *, int);
> > +
> > +#define xfs_trans_cancel(tp) __xfs_trans_cancel(tp, 0)
> > +#define xfs_trans_cancel_error(tp, error) __xfs_trans_cancel(tp, error)
> 
> Nitpicking: the arguments should be wrapped in parentheses so you can't
> do evil things to the preprocessor.  SOrry about being sloppy about
> that.

You were not sloppy.... That's preprocessor 101 and I shouldn't have
overlooked that. Thanks for catching it.

> 
> #define xfs_trans_cancel_error(tp, error) \
> 	__xfs_trans_cancel((tp), (error))
> 
> --D
> 
> > +
> >  struct xfs_buf	*xfs_trans_getsb(struct xfs_trans *);
> >  struct xfs_buf	*xfs_trans_getrtsb(struct xfs_trans *tp);
> >  
> > @@ -237,7 +242,6 @@ void		xfs_trans_log_inode(xfs_trans_t *, struct xfs_inode *, uint);
> >  int		xfs_trans_commit(struct xfs_trans *);
> >  int		xfs_trans_roll(struct xfs_trans **);
> >  int		xfs_trans_roll_inode(struct xfs_trans **, struct xfs_inode *);
> > -void		xfs_trans_cancel(xfs_trans_t *);
> >  int		xfs_trans_ail_init(struct xfs_mount *);
> >  void		xfs_trans_ail_destroy(struct xfs_mount *);
> >  
> > -- 
> > 2.55.0
> > 
> > 
> 

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

[Carlos Maiolino]

On Tue, Sep 08, 2026 at 08:16:35AM -0700, Darrick J. Wong wrote:
> On Tue, Sep 08, 2026 at 04:54:14PM +0200, 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.
> > 
> > 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";
> 
> Should it just snprintf the error number to a buffer?

I thought about that and also about using num_to_str() to print out the
error number *if* the ERRNAME config is disabled. I just thought it
wasn't worth handling an extra buffer here giving ERRNAME is enabled on
most major distros AFAICT.

I quick chat with Eric too and he suggested "unknown error" might be
confusing and perhaps replace the above with a blank string like:

err_str = "";


My whole point though in doing this way by not using a snprintf() and
num_to_str() is I believe it would be an overkill for this and opted to
KISS :)

> > +
> >  	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);
> >  
> >  		xfs_stack_trace();
> >  	}
> > @@ -404,7 +411,7 @@ xfs_corruption_error(
> >  {
> >  	if (buf && level <= xfs_error_level)
> >  		xfs_hex_dump(buf, bufsize);
> > -	xfs_error_report(tag, level, mp, filename, linenum, failaddr);
> > +	xfs_error_report(tag, level, 0, mp, filename, linenum, failaddr);
> 
> Shouldn't this be -EFSCORRUPTED too?

Indeed, I didn't want to change any 'implicit' error handling in this
patch specifically. I was hoping to do that in a future patch adding
errors where pertinent. But I have zero objection in changing this.

> 
> --D
> 
> >  	xfs_alert(mp, "Corruption detected. Unmount and run xfs_repair");
> >  }
> >  
> > diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> > index 0b9c5ba8a598..f3b5564c2f4f 100644
> > --- a/fs/xfs/xfs_error.h
> > +++ b/fs/xfs/xfs_error.h
> > @@ -8,9 +8,9 @@
> >  
> >  struct xfs_mount;
> >  
> > -extern void xfs_error_report(const char *tag, int level, struct xfs_mount *mp,
> > -			const char *filename, int linenum,
> > -			xfs_failaddr_t failaddr);
> > +void xfs_error_report(const char *tag, int level, int error,
> > +		struct xfs_mount *mp, const char *filename,
> > +		int linenum, xfs_failaddr_t failaddr);
> >  extern void xfs_corruption_error(const char *tag, int level,
> >  			struct xfs_mount *mp, const void *buf, size_t bufsize,
> >  			const char *filename, int linenum,
> > @@ -25,8 +25,8 @@ extern void xfs_inode_verifier_error(struct xfs_inode *ip, int error,
> >  			const char *name, const void *buf, size_t bufsz,
> >  			xfs_failaddr_t failaddr);
> >  
> > -#define	XFS_ERROR_REPORT(e, lvl, mp)	\
> > -	xfs_error_report(e, lvl, mp, __FILE__, __LINE__, __return_address)
> > +#define	XFS_ERROR_REPORT(e, lvl, error, mp)	\
> > +	xfs_error_report(e, lvl, error, mp, __FILE__, __LINE__, __return_address)
> >  #define	XFS_CORRUPTION_ERROR(e, lvl, mp, buf, bufsize)	\
> >  	xfs_corruption_error(e, lvl, mp, buf, bufsize, \
> >  			     __FILE__, __LINE__, __return_address)
> > diff --git a/fs/xfs/xfs_exchmaps_item.c b/fs/xfs/xfs_exchmaps_item.c
> > index 264a121c5e16..2cbc1b9aee43 100644
> > --- a/fs/xfs/xfs_exchmaps_item.c
> > +++ b/fs/xfs/xfs_exchmaps_item.c
> > @@ -559,13 +559,15 @@ xlog_recover_xmi_commit_pass2(
> >  
> >  	len = sizeof(struct xfs_xmi_log_format);
> >  	if (item->ri_buf[0].i_len != len) {
> > -		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, log->l_mp);
> > +		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
> > +				 -EFSCORRUPTED, log->l_mp);
> >  		return -EFSCORRUPTED;
> >  	}
> >  
> >  	xmi_formatp = item->ri_buf[0].i_addr;
> >  	if (xmi_formatp->__pad != 0) {
> > -		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, log->l_mp);
> > +		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
> > +				 -EFSCORRUPTED, log->l_mp);
> >  		return -EFSCORRUPTED;
> >  	}
> >  
> > @@ -600,7 +602,8 @@ xlog_recover_xmd_commit_pass2(
> >  
> >  	xmd_formatp = item->ri_buf[0].i_addr;
> >  	if (item->ri_buf[0].i_len != sizeof(struct xfs_xmd_log_format)) {
> > -		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, log->l_mp);
> > +		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
> > +				 -EFSCORRUPTED, log->l_mp);
> >  		return -EFSCORRUPTED;
> >  	}
> >  
> > diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> > index c6cb0b6b9e46..c361e02110a0 100644
> > --- a/fs/xfs/xfs_inode_item.c
> > +++ b/fs/xfs/xfs_inode_item.c
> > @@ -1185,7 +1185,8 @@ xfs_inode_item_format_convert(
> >  	struct xfs_inode_log_format_32	*in_f32 = buf->i_addr;
> >  
> >  	if (buf->i_len != sizeof(*in_f32)) {
> > -		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, NULL);
> > +		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
> > +				 -EFSCORRUPTED, NULL);
> >  		return -EFSCORRUPTED;
> >  	}
> >  
> > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> > index 2f76531842f8..b1979d00d2e3 100644
> > --- a/fs/xfs/xfs_log_recover.c
> > +++ b/fs/xfs/xfs_log_recover.c
> > @@ -2932,7 +2932,8 @@ xlog_recover_process(
> >  		 * fatal log corruption failure.
> >  		 */
> >  		if (xfs_has_crc(log->l_mp)) {
> > -			XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, log->l_mp);
> > +			XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
> > +					 -EFSCORRUPTED, log->l_mp);
> >  			return -EFSCORRUPTED;
> >  		}
> >  	}
> > diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> > index c6657072361a..792a67a7ec28 100644
> > --- a/fs/xfs/xfs_trans.c
> > +++ b/fs/xfs/xfs_trans.c
> > @@ -999,7 +999,7 @@ xfs_trans_cancel(
> >  	 * here.
> >  	 */
> >  	if (dirty && !xfs_is_shutdown(mp)) {
> > -		XFS_ERROR_REPORT("xfs_trans_cancel", XFS_ERRLEVEL_LOW, mp);
> > +		XFS_ERROR_REPORT("xfs_trans_cancel", XFS_ERRLEVEL_LOW, 0, mp);
> >  		xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
> >  	}
> >  #ifdef DEBUG
> > -- 
> > 2.55.0
> > 
> > 
> 

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

[Carlos Maiolino]

/me takes a mental not to never look at patch replies in an out-of-order
fashion....

On Tue, Sep 08, 2026 at 03:40:57PM -0500, Eric Sandeen wrote:
> On 9/8/26 12:02 PM, Darrick J. Wong wrote:
> > 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?
> 
> Yeah I think so. Messy code but better dmesg IMHO.
> 
> > 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.
> TBH I've lost track of what a tag vs an errno vs a string is, but I stand
> by my observation that emitting "unknown error" when errname() returns null
> seems like a net negative change. :)
> 

Thanks for the reviews. My plan was to not add more strings and too much
extra logic, but those are all points against this :)
I'll craft something along these lines.