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

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

[cem]

From: Carlos Maiolino <cem@kernel.org>

Hello,

this is the V3 of this series addressing last round of reviews.

Darrich, I believe I addressed all your comments, could you please
double-check it? I kept your RwB.

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 v3 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.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
V3:
	- Remove stale description aboutt ERRNAME
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 cf0d610265fe..457617979668 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -2948,7 +2948,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 v3 1/3] xfs: add xfs_error_report the ability to display an error code

[Darrick J. Wong]

On Thu, Sep 17, 2026 at 12:35:46PM +0200, cem@kernel.org wrote:
> From: Carlos Maiolino <cem@kernel.org>
> 
> Users could opt to request an error code to be printed now.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

Looks good,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
> V3:
> 	- Remove stale description aboutt ERRNAME
> 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 cf0d610265fe..457617979668 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -2948,7 +2948,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 v3 1/3] xfs: add xfs_error_report the ability to display an error code

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH v3 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>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
V3:
	- Kill typedef from __xfs_trans_cancel
	- Remove uneeded wrap of parameter 0
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..36a49622b870 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(struct xfs_trans *, 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

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

[Christoph Hellwig]

On Thu, Sep 17, 2026 at 12:35:47PM +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>
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
> V3:
> 	- Kill typedef from __xfs_trans_cancel
> 	- Remove uneeded wrap of parameter 0
> 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..36a49622b870 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(struct xfs_trans *, int);
> +
> +#define xfs_trans_cancel(tp) __xfs_trans_cancel((tp), 0)
> +#define xfs_trans_cancel_error(tp, error) __xfs_trans_cancel((tp), (error))

Can we turn these into inlines instead of macros?

And do away with the pointless __xfs_trans_cancel, just implement
xfs_trans_cancel_error in xfs_trans.c, and wrap xfs_trans_cancel around
it.

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

[Carlos Maiolino]

On Mon, Sep 21, 2026 at 10:33:56AM +0200, Christoph Hellwig wrote:
> On Thu, Sep 17, 2026 at 12:35:47PM +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>
> > Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
> > ---
> > V3:
> > 	- Kill typedef from __xfs_trans_cancel
> > 	- Remove uneeded wrap of parameter 0
> > 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..36a49622b870 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(struct xfs_trans *, int);
> > +
> > +#define xfs_trans_cancel(tp) __xfs_trans_cancel((tp), 0)
> > +#define xfs_trans_cancel_error(tp, error) __xfs_trans_cancel((tp), (error))
> 
> Can we turn these into inlines instead of macros?

This sounds good.

> 
> And do away with the pointless __xfs_trans_cancel, just implement
> xfs_trans_cancel_error in xfs_trans.c, and wrap xfs_trans_cancel around
> it.

The main reason I created __xfs_trans_cancel was to avoid a fs/xfs/*
change of calls to xfs_trans_cancel().

If you think that's a must, I can do that in this patch, otherwise I'd
prefer to add it on a separate patch just removing the call, without any
logic change.
What do you think?

Cheers.

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

[Christoph Hellwig]

On Wed, Sep 23, 2026 at 12:49:01PM +0200, Carlos Maiolino wrote:
> > And do away with the pointless __xfs_trans_cancel, just implement
> > xfs_trans_cancel_error in xfs_trans.c, and wrap xfs_trans_cancel around
> > it.
> 
> The main reason I created __xfs_trans_cancel was to avoid a fs/xfs/*
> change of calls to xfs_trans_cancel().

And that's a good idea.  But we don't need the extra
__xfs_trans_cancel vs xfs_trans_cancel_error indirection.  Just rename
what is __xfs_trans_cancel in this series to xfs_trans_cancel_error,
and make xfs_trans_cancel an inline wrapper around that.

[PATCH v3 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 v3 3/3] xfs: make xfs_iomap_write_direct() report an error to xfs_trans_cancel

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>