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

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

[cem]

From: Carlos Maiolino <cem@kernel.org>

Hi,

this is the V2 of this series addressing the comments from the previous
version. Details are in each individual patch, but the main change:

I removed the errname() call from patch 1. Turns out it would make
things more complicated as we'd need to not only check if
xfs_error_report() receives an error, but also if errname() returned a
non-NULL value. Having the error name was a nice to have thing, but
printing only the error number make things simpler.

Also hch already cleaned the extern modifiers from the prototypes so I
removed it from here.

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         | 16 +++++++++++-----
 fs/xfs/xfs_error.h         |  9 +++++----
 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, 37 insertions(+), 19 deletions(-)

-- 
2.55.0

[PATCH v2 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_ERRNAME without requiring it
to be enabled.
Report 'unkown error' If CONFIG_SYMBOLIC_ERRNAME is not enabled or the
user passes 0 as an error.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

---
V2:
	- Fix commit description s/_NAME/_ERRNAME
	- xfs_corruption_error should call xfs_error_report with
	  -EFSCORRUPTED
	- Modify xfs_error_report to only print out the error number if
	  it received an error from the caller.
	- Don't use errname() anymore.

 fs/xfs/xfs_error.c         | 16 +++++++++++-----
 fs/xfs/xfs_error.h         |  9 +++++----
 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, 27 insertions(+), 15 deletions(-)

diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index 9ece70fd94ca..c943717f1dd7 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -241,16 +241,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)
 {
 	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);
-
+		if (error)
+			xfs_alert_tag(mp, XFS_PTAG_ERROR_REPORT,
+"Internal error %s (%d) at line %d of file %s.  Caller %pS",
+				tag, error, linenum, filename, failaddr);
+		else
+			xfs_alert_tag(mp, XFS_PTAG_ERROR_REPORT,
+"Internal error %s at line %d of file %s.  Caller %pS",
+				tag, linenum, filename, failaddr);
 		xfs_stack_trace();
 	}
 }
@@ -268,7 +273,8 @@ 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, -EFSCORRUPTED, 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 05fc1d1cf521..3c76d0dad51c 100644
--- a/fs/xfs/xfs_error.h
+++ b/fs/xfs/xfs_error.h
@@ -8,8 +8,9 @@
 
 struct xfs_mount;
 
-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);
 void xfs_corruption_error(const char *tag, int level, struct xfs_mount *mp,
 		const void *buf, size_t bufsize, const char *filename,
 		int linenum, xfs_failaddr_t failaddr);
@@ -20,8 +21,8 @@ void xfs_verifier_error(struct xfs_buf *bp, int error, xfs_failaddr_t failaddr);
 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 dd5d92ca1010..9f499a80038c 100644
--- a/fs/xfs/xfs_exchmaps_item.c
+++ b/fs/xfs/xfs_exchmaps_item.c
@@ -573,13 +573,15 @@ xlog_recover_xmi_commit_pass2(
 
 	len = sizeof(struct xfs_xmi_log_format);
 	if (item->ri_buf[0].iov_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].iov_base;
 	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;
 	}
 
@@ -614,7 +616,8 @@ xlog_recover_xmd_commit_pass2(
 
 	xmd_formatp = item->ri_buf[0].iov_base;
 	if (item->ri_buf[0].iov_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 99d6ecccdaa7..a29c90f0650f 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -1229,7 +1229,8 @@ xfs_inode_item_format_convert(
 	struct xfs_inode_log_format_32	*in_f32 = buf->iov_base;
 
 	if (buf->iov_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 e7e49529658b..e92afc455d0f 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -2945,7 +2945,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 1b36cf12d4e3..5c522790e5be 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -971,7 +971,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 v2 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>

---
V2:
	- Parenthesize macro parameters

 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 5c522790e5be..395c896673fe 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -943,8 +943,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;
@@ -971,7 +972,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 eb83c5dac032..855a46433154 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 v2 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 7c6238fed61e..4906a4828d11 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -358,7 +358,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 v2 2/3] xfs: enable xfs_trans_cancel() to report and error code

[Darrick J. Wong]

On Mon, Sep 14, 2026 at 01:57:40PM +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>
> 
> ---
> V2:
> 	- Parenthesize macro parameters
> 
>  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 5c522790e5be..395c896673fe 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -943,8 +943,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;
> @@ -971,7 +972,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 eb83c5dac032..855a46433154 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);

Nitpicking: struct xfs_trans, not xfs_trans_t (kill the typedef usage
since you're modifying the line anyway).

> +#define xfs_trans_cancel(tp) __xfs_trans_cancel((tp), (0))

Moar nitpicking: No need for parentheses around the 0.

With those two changes,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D


> +#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
> 

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

[Darrick J. Wong]

On Mon, Sep 14, 2026 at 01:57:39PM +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_ERRNAME without requiring it

Eh?  No it doesn't...

> to be enabled.
> Report 'unkown error' If CONFIG_SYMBOLIC_ERRNAME is not enabled or the
> user passes 0 as an error.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> 
> ---
> V2:
> 	- Fix commit description s/_NAME/_ERRNAME
> 	- xfs_corruption_error should call xfs_error_report with
> 	  -EFSCORRUPTED
> 	- Modify xfs_error_report to only print out the error number if
> 	  it received an error from the caller.
> 	- Don't use errname() anymore.

...right?

>  fs/xfs/xfs_error.c         | 16 +++++++++++-----
>  fs/xfs/xfs_error.h         |  9 +++++----
>  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, 27 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index 9ece70fd94ca..c943717f1dd7 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -241,16 +241,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)
>  {
>  	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);
> -
> +		if (error)
> +			xfs_alert_tag(mp, XFS_PTAG_ERROR_REPORT,
> +"Internal error %s (%d) at line %d of file %s.  Caller %pS",
> +				tag, error, linenum, filename, failaddr);
> +		else
> +			xfs_alert_tag(mp, XFS_PTAG_ERROR_REPORT,
> +"Internal error %s at line %d of file %s.  Caller %pS",
> +				tag, linenum, filename, failaddr);

/me wonders if we should add a tracepoint to these error reporting
functions so that one isn't totally reliant on scraping dmesg, but maybe
adding fserror_report_metadata is more appropriate for that?  Will have
to ruminate on that for a while.

In the meantime, this patch looks fine to me,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

>  		xfs_stack_trace();
>  	}
>  }
> @@ -268,7 +273,8 @@ 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, -EFSCORRUPTED, 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 05fc1d1cf521..3c76d0dad51c 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -8,8 +8,9 @@
>  
>  struct xfs_mount;
>  
> -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);
>  void xfs_corruption_error(const char *tag, int level, struct xfs_mount *mp,
>  		const void *buf, size_t bufsize, const char *filename,
>  		int linenum, xfs_failaddr_t failaddr);
> @@ -20,8 +21,8 @@ void xfs_verifier_error(struct xfs_buf *bp, int error, xfs_failaddr_t failaddr);
>  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 dd5d92ca1010..9f499a80038c 100644
> --- a/fs/xfs/xfs_exchmaps_item.c
> +++ b/fs/xfs/xfs_exchmaps_item.c
> @@ -573,13 +573,15 @@ xlog_recover_xmi_commit_pass2(
>  
>  	len = sizeof(struct xfs_xmi_log_format);
>  	if (item->ri_buf[0].iov_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].iov_base;
>  	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;
>  	}
>  
> @@ -614,7 +616,8 @@ xlog_recover_xmd_commit_pass2(
>  
>  	xmd_formatp = item->ri_buf[0].iov_base;
>  	if (item->ri_buf[0].iov_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 99d6ecccdaa7..a29c90f0650f 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -1229,7 +1229,8 @@ xfs_inode_item_format_convert(
>  	struct xfs_inode_log_format_32	*in_f32 = buf->iov_base;
>  
>  	if (buf->iov_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 e7e49529658b..e92afc455d0f 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -2945,7 +2945,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 1b36cf12d4e3..5c522790e5be 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -971,7 +971,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 v2 1/3] xfs: add xfs_error_report the ability to display an error code

[Carlos Maiolino]

On Mon, Sep 14, 2026 at 07:42:24AM -0700, Darrick J. Wong wrote:
> On Mon, Sep 14, 2026 at 01:57:39PM +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_ERRNAME without requiring it
> 
> Eh?  No it doesn't...
> 
> > to be enabled.
> > Report 'unkown error' If CONFIG_SYMBOLIC_ERRNAME is not enabled or the
> > user passes 0 as an error.
> > 
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > 
> > ---
> > V2:
> > 	- Fix commit description s/_NAME/_ERRNAME
> > 	- xfs_corruption_error should call xfs_error_report with
> > 	  -EFSCORRUPTED
> > 	- Modify xfs_error_report to only print out the error number if
> > 	  it received an error from the caller.
> > 	- Don't use errname() anymore.
> 
> ...right?

Of course I'd forget something... I'll fix that for next version...


> 
> >  fs/xfs/xfs_error.c         | 16 +++++++++++-----
> >  fs/xfs/xfs_error.h         |  9 +++++----
> >  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, 27 insertions(+), 15 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> > index 9ece70fd94ca..c943717f1dd7 100644
> > --- a/fs/xfs/xfs_error.c
> > +++ b/fs/xfs/xfs_error.c
> > @@ -241,16 +241,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)
> >  {
> >  	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);
> > -
> > +		if (error)
> > +			xfs_alert_tag(mp, XFS_PTAG_ERROR_REPORT,
> > +"Internal error %s (%d) at line %d of file %s.  Caller %pS",
> > +				tag, error, linenum, filename, failaddr);
> > +		else
> > +			xfs_alert_tag(mp, XFS_PTAG_ERROR_REPORT,
> > +"Internal error %s at line %d of file %s.  Caller %pS",
> > +				tag, linenum, filename, failaddr);
> 
> /me wonders if we should add a tracepoint to these error reporting
> functions so that one isn't totally reliant on scraping dmesg, but maybe
> adding fserror_report_metadata is more appropriate for that?  Will have
> to ruminate on that for a while.

Hmmm, what you think about controlling with the tracepoint? Spitting out
the error? *If* that's the case, the problem I see is when users hit the
error we can't enable a tracepoint on a past error :P so I'd rather have
it logged in dmesg without needing to rely on having tracepoints enabled
or not.

Or perhaps I missed your point here?

> 
> In the meantime, this patch looks fine to me,
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

Thanks!


> 
> --D
> 
> >  		xfs_stack_trace();
> >  	}
> >  }
> > @@ -268,7 +273,8 @@ 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, -EFSCORRUPTED, 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 05fc1d1cf521..3c76d0dad51c 100644
> > --- a/fs/xfs/xfs_error.h
> > +++ b/fs/xfs/xfs_error.h
> > @@ -8,8 +8,9 @@
> >  
> >  struct xfs_mount;
> >  
> > -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);
> >  void xfs_corruption_error(const char *tag, int level, struct xfs_mount *mp,
> >  		const void *buf, size_t bufsize, const char *filename,
> >  		int linenum, xfs_failaddr_t failaddr);
> > @@ -20,8 +21,8 @@ void xfs_verifier_error(struct xfs_buf *bp, int error, xfs_failaddr_t failaddr);
> >  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 dd5d92ca1010..9f499a80038c 100644
> > --- a/fs/xfs/xfs_exchmaps_item.c
> > +++ b/fs/xfs/xfs_exchmaps_item.c
> > @@ -573,13 +573,15 @@ xlog_recover_xmi_commit_pass2(
> >  
> >  	len = sizeof(struct xfs_xmi_log_format);
> >  	if (item->ri_buf[0].iov_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].iov_base;
> >  	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;
> >  	}
> >  
> > @@ -614,7 +616,8 @@ xlog_recover_xmd_commit_pass2(
> >  
> >  	xmd_formatp = item->ri_buf[0].iov_base;
> >  	if (item->ri_buf[0].iov_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 99d6ecccdaa7..a29c90f0650f 100644
> > --- a/fs/xfs/xfs_inode_item.c
> > +++ b/fs/xfs/xfs_inode_item.c
> > @@ -1229,7 +1229,8 @@ xfs_inode_item_format_convert(
> >  	struct xfs_inode_log_format_32	*in_f32 = buf->iov_base;
> >  
> >  	if (buf->iov_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 e7e49529658b..e92afc455d0f 100644
> > --- a/fs/xfs/xfs_log_recover.c
> > +++ b/fs/xfs/xfs_log_recover.c
> > @@ -2945,7 +2945,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 1b36cf12d4e3..5c522790e5be 100644
> > --- a/fs/xfs/xfs_trans.c
> > +++ b/fs/xfs/xfs_trans.c
> > @@ -971,7 +971,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 v2 1/3] xfs: add xfs_error_report the ability to display an error code

[Darrick J. Wong]

On Mon, Sep 14, 2026 at 05:00:51PM +0200, Carlos Maiolino wrote:
> On Mon, Sep 14, 2026 at 07:42:24AM -0700, Darrick J. Wong wrote:
> > On Mon, Sep 14, 2026 at 01:57:39PM +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_ERRNAME without requiring it
> > 
> > Eh?  No it doesn't...
> > 
> > > to be enabled.
> > > Report 'unkown error' If CONFIG_SYMBOLIC_ERRNAME is not enabled or the
> > > user passes 0 as an error.
> > > 
> > > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > > 
> > > ---
> > > V2:
> > > 	- Fix commit description s/_NAME/_ERRNAME
> > > 	- xfs_corruption_error should call xfs_error_report with
> > > 	  -EFSCORRUPTED
> > > 	- Modify xfs_error_report to only print out the error number if
> > > 	  it received an error from the caller.
> > > 	- Don't use errname() anymore.
> > 
> > ...right?
> 
> Of course I'd forget something... I'll fix that for next version...
> 
> 
> > 
> > >  fs/xfs/xfs_error.c         | 16 +++++++++++-----
> > >  fs/xfs/xfs_error.h         |  9 +++++----
> > >  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, 27 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> > > index 9ece70fd94ca..c943717f1dd7 100644
> > > --- a/fs/xfs/xfs_error.c
> > > +++ b/fs/xfs/xfs_error.c
> > > @@ -241,16 +241,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)
> > >  {
> > >  	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);
> > > -
> > > +		if (error)
> > > +			xfs_alert_tag(mp, XFS_PTAG_ERROR_REPORT,
> > > +"Internal error %s (%d) at line %d of file %s.  Caller %pS",
> > > +				tag, error, linenum, filename, failaddr);
> > > +		else
> > > +			xfs_alert_tag(mp, XFS_PTAG_ERROR_REPORT,
> > > +"Internal error %s at line %d of file %s.  Caller %pS",
> > > +				tag, linenum, filename, failaddr);
> > 
> > /me wonders if we should add a tracepoint to these error reporting
> > functions so that one isn't totally reliant on scraping dmesg, but maybe
> > adding fserror_report_metadata is more appropriate for that?  Will have
> > to ruminate on that for a while.
> 
> Hmmm, what you think about controlling with the tracepoint? Spitting out
> the error? *If* that's the case, the problem I see is when users hit the
> error we can't enable a tracepoint on a past error :P so I'd rather have
> it logged in dmesg without needing to rely on having tracepoints enabled
> or not.
> 
> Or perhaps I missed your point here?

I wasn't going to delete anything out of these functions, just add a new
fserror_report_metadata() call.  dmesg scraping would continue working
as it always has, but now a monitoring program could watch for events
via fsnotify.

--D

> > 
> > In the meantime, this patch looks fine to me,
> > Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
> 
> Thanks!
> 
> 
> > 
> > --D
> > 
> > >  		xfs_stack_trace();
> > >  	}
> > >  }
> > > @@ -268,7 +273,8 @@ 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, -EFSCORRUPTED, 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 05fc1d1cf521..3c76d0dad51c 100644
> > > --- a/fs/xfs/xfs_error.h
> > > +++ b/fs/xfs/xfs_error.h
> > > @@ -8,8 +8,9 @@
> > >  
> > >  struct xfs_mount;
> > >  
> > > -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);
> > >  void xfs_corruption_error(const char *tag, int level, struct xfs_mount *mp,
> > >  		const void *buf, size_t bufsize, const char *filename,
> > >  		int linenum, xfs_failaddr_t failaddr);
> > > @@ -20,8 +21,8 @@ void xfs_verifier_error(struct xfs_buf *bp, int error, xfs_failaddr_t failaddr);
> > >  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 dd5d92ca1010..9f499a80038c 100644
> > > --- a/fs/xfs/xfs_exchmaps_item.c
> > > +++ b/fs/xfs/xfs_exchmaps_item.c
> > > @@ -573,13 +573,15 @@ xlog_recover_xmi_commit_pass2(
> > >  
> > >  	len = sizeof(struct xfs_xmi_log_format);
> > >  	if (item->ri_buf[0].iov_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].iov_base;
> > >  	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;
> > >  	}
> > >  
> > > @@ -614,7 +616,8 @@ xlog_recover_xmd_commit_pass2(
> > >  
> > >  	xmd_formatp = item->ri_buf[0].iov_base;
> > >  	if (item->ri_buf[0].iov_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 99d6ecccdaa7..a29c90f0650f 100644
> > > --- a/fs/xfs/xfs_inode_item.c
> > > +++ b/fs/xfs/xfs_inode_item.c
> > > @@ -1229,7 +1229,8 @@ xfs_inode_item_format_convert(
> > >  	struct xfs_inode_log_format_32	*in_f32 = buf->iov_base;
> > >  
> > >  	if (buf->iov_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 e7e49529658b..e92afc455d0f 100644
> > > --- a/fs/xfs/xfs_log_recover.c
> > > +++ b/fs/xfs/xfs_log_recover.c
> > > @@ -2945,7 +2945,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 1b36cf12d4e3..5c522790e5be 100644
> > > --- a/fs/xfs/xfs_trans.c
> > > +++ b/fs/xfs/xfs_trans.c
> > > @@ -971,7 +971,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 v2 1/3] xfs: add xfs_error_report the ability to display an error code

[Carlos Maiolino]

On Mon, Sep 14, 2026 at 08:08:27AM -0700, Darrick J. Wong wrote:
> On Mon, Sep 14, 2026 at 05:00:51PM +0200, Carlos Maiolino wrote:
> > On Mon, Sep 14, 2026 at 07:42:24AM -0700, Darrick J. Wong wrote:
> > > On Mon, Sep 14, 2026 at 01:57:39PM +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_ERRNAME without requiring it
> > > 
> > > Eh?  No it doesn't...
> > > 
> > > > to be enabled.
> > > > Report 'unkown error' If CONFIG_SYMBOLIC_ERRNAME is not enabled or the
> > > > user passes 0 as an error.
> > > > 
> > > > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > > > 
> > > > ---
> > > > V2:
> > > > 	- Fix commit description s/_NAME/_ERRNAME
> > > > 	- xfs_corruption_error should call xfs_error_report with
> > > > 	  -EFSCORRUPTED
> > > > 	- Modify xfs_error_report to only print out the error number if
> > > > 	  it received an error from the caller.
> > > > 	- Don't use errname() anymore.
> > > 
> > > ...right?
> > 
> > Of course I'd forget something... I'll fix that for next version...
> > 
> > 
> > > 
> > > >  fs/xfs/xfs_error.c         | 16 +++++++++++-----
> > > >  fs/xfs/xfs_error.h         |  9 +++++----
> > > >  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, 27 insertions(+), 15 deletions(-)
> > > > 
> > > > diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> > > > index 9ece70fd94ca..c943717f1dd7 100644
> > > > --- a/fs/xfs/xfs_error.c
> > > > +++ b/fs/xfs/xfs_error.c
> > > > @@ -241,16 +241,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)
> > > >  {
> > > >  	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);
> > > > -
> > > > +		if (error)
> > > > +			xfs_alert_tag(mp, XFS_PTAG_ERROR_REPORT,
> > > > +"Internal error %s (%d) at line %d of file %s.  Caller %pS",
> > > > +				tag, error, linenum, filename, failaddr);
> > > > +		else
> > > > +			xfs_alert_tag(mp, XFS_PTAG_ERROR_REPORT,
> > > > +"Internal error %s at line %d of file %s.  Caller %pS",
> > > > +				tag, linenum, filename, failaddr);
> > > 
> > > /me wonders if we should add a tracepoint to these error reporting
> > > functions so that one isn't totally reliant on scraping dmesg, but maybe
> > > adding fserror_report_metadata is more appropriate for that?  Will have
> > > to ruminate on that for a while.
> > 
> > Hmmm, what you think about controlling with the tracepoint? Spitting out
> > the error? *If* that's the case, the problem I see is when users hit the
> > error we can't enable a tracepoint on a past error :P so I'd rather have
> > it logged in dmesg without needing to rely on having tracepoints enabled
> > or not.
> > 
> > Or perhaps I missed your point here?
> 
> I wasn't going to delete anything out of these functions, just add a new
> fserror_report_metadata() call.  dmesg scraping would continue working
> as it always has, but now a monitoring program could watch for events
> via fsnotify.

Ok, sounds reasonable, I'm happy to do it on another patch though, just
to not go through yet another review round on this one.

> 
> --D
> 
> > > 
> > > In the meantime, this patch looks fine to me,
> > > Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
> > 
> > Thanks!
> > 
> > 
> > > 
> > > --D
> > > 
> > > >  		xfs_stack_trace();
> > > >  	}
> > > >  }
> > > > @@ -268,7 +273,8 @@ 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, -EFSCORRUPTED, 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 05fc1d1cf521..3c76d0dad51c 100644
> > > > --- a/fs/xfs/xfs_error.h
> > > > +++ b/fs/xfs/xfs_error.h
> > > > @@ -8,8 +8,9 @@
> > > >  
> > > >  struct xfs_mount;
> > > >  
> > > > -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);
> > > >  void xfs_corruption_error(const char *tag, int level, struct xfs_mount *mp,
> > > >  		const void *buf, size_t bufsize, const char *filename,
> > > >  		int linenum, xfs_failaddr_t failaddr);
> > > > @@ -20,8 +21,8 @@ void xfs_verifier_error(struct xfs_buf *bp, int error, xfs_failaddr_t failaddr);
> > > >  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 dd5d92ca1010..9f499a80038c 100644
> > > > --- a/fs/xfs/xfs_exchmaps_item.c
> > > > +++ b/fs/xfs/xfs_exchmaps_item.c
> > > > @@ -573,13 +573,15 @@ xlog_recover_xmi_commit_pass2(
> > > >  
> > > >  	len = sizeof(struct xfs_xmi_log_format);
> > > >  	if (item->ri_buf[0].iov_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].iov_base;
> > > >  	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;
> > > >  	}
> > > >  
> > > > @@ -614,7 +616,8 @@ xlog_recover_xmd_commit_pass2(
> > > >  
> > > >  	xmd_formatp = item->ri_buf[0].iov_base;
> > > >  	if (item->ri_buf[0].iov_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 99d6ecccdaa7..a29c90f0650f 100644
> > > > --- a/fs/xfs/xfs_inode_item.c
> > > > +++ b/fs/xfs/xfs_inode_item.c
> > > > @@ -1229,7 +1229,8 @@ xfs_inode_item_format_convert(
> > > >  	struct xfs_inode_log_format_32	*in_f32 = buf->iov_base;
> > > >  
> > > >  	if (buf->iov_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 e7e49529658b..e92afc455d0f 100644
> > > > --- a/fs/xfs/xfs_log_recover.c
> > > > +++ b/fs/xfs/xfs_log_recover.c
> > > > @@ -2945,7 +2945,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 1b36cf12d4e3..5c522790e5be 100644
> > > > --- a/fs/xfs/xfs_trans.c
> > > > +++ b/fs/xfs/xfs_trans.c
> > > > @@ -971,7 +971,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
> > > > 
> > > 
> > 
>