* [PATCH v2 0/3] Enable xfs error reporting to print out error codes
@ 2026-09-14 11:57 cem
2026-09-14 11:57 ` [PATCH v2 1/3] xfs: add xfs_error_report the ability to display an error code cem
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: cem @ 2026-09-14 11:57 UTC (permalink / raw)
To: cem; +Cc: djwong, hchc, sandeen, linux-xfs
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
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v2 1/3] xfs: add xfs_error_report the ability to display an error code 2026-09-14 11:57 [PATCH v2 0/3] Enable xfs error reporting to print out error codes cem @ 2026-09-14 11:57 ` cem 2026-09-14 14:42 ` Darrick J. Wong 2026-09-14 11:57 ` [PATCH v2 2/3] xfs: enable xfs_trans_cancel() to report and " cem 2026-09-14 11:57 ` [PATCH v2 3/3] xfs: make xfs_iomap_write_direct() report an error to xfs_trans_cancel cem 2 siblings, 1 reply; 9+ messages in thread From: cem @ 2026-09-14 11:57 UTC (permalink / raw) To: cem; +Cc: djwong, hchc, sandeen, linux-xfs 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] xfs: add xfs_error_report the ability to display an error code 2026-09-14 11:57 ` [PATCH v2 1/3] xfs: add xfs_error_report the ability to display an error code cem @ 2026-09-14 14:42 ` Darrick J. Wong 2026-09-14 15:00 ` Carlos Maiolino 0 siblings, 1 reply; 9+ messages in thread From: Darrick J. Wong @ 2026-09-14 14:42 UTC (permalink / raw) To: cem; +Cc: hchc, sandeen, linux-xfs 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 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] xfs: add xfs_error_report the ability to display an error code 2026-09-14 14:42 ` Darrick J. Wong @ 2026-09-14 15:00 ` Carlos Maiolino 2026-09-14 15:08 ` Darrick J. Wong 0 siblings, 1 reply; 9+ messages in thread From: Carlos Maiolino @ 2026-09-14 15:00 UTC (permalink / raw) To: Darrick J. Wong; +Cc: hchc, sandeen, linux-xfs 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 > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] xfs: add xfs_error_report the ability to display an error code 2026-09-14 15:00 ` Carlos Maiolino @ 2026-09-14 15:08 ` Darrick J. Wong 2026-09-17 10:22 ` Carlos Maiolino 0 siblings, 1 reply; 9+ messages in thread From: Darrick J. Wong @ 2026-09-14 15:08 UTC (permalink / raw) To: Carlos Maiolino; +Cc: hchc, sandeen, linux-xfs 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 > > > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] xfs: add xfs_error_report the ability to display an error code 2026-09-14 15:08 ` Darrick J. Wong @ 2026-09-17 10:22 ` Carlos Maiolino 0 siblings, 0 replies; 9+ messages in thread From: Carlos Maiolino @ 2026-09-17 10:22 UTC (permalink / raw) To: Darrick J. Wong; +Cc: hchc, sandeen, linux-xfs 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 > > > > > > > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] xfs: enable xfs_trans_cancel() to report and error code 2026-09-14 11:57 [PATCH v2 0/3] Enable xfs error reporting to print out error codes cem 2026-09-14 11:57 ` [PATCH v2 1/3] xfs: add xfs_error_report the ability to display an error code cem @ 2026-09-14 11:57 ` cem 2026-09-14 14:38 ` Darrick J. Wong 2026-09-14 11:57 ` [PATCH v2 3/3] xfs: make xfs_iomap_write_direct() report an error to xfs_trans_cancel cem 2 siblings, 1 reply; 9+ messages in thread From: cem @ 2026-09-14 11:57 UTC (permalink / raw) To: cem; +Cc: djwong, hchc, sandeen, linux-xfs 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] xfs: enable xfs_trans_cancel() to report and error code 2026-09-14 11:57 ` [PATCH v2 2/3] xfs: enable xfs_trans_cancel() to report and " cem @ 2026-09-14 14:38 ` Darrick J. Wong 0 siblings, 0 replies; 9+ messages in thread From: Darrick J. Wong @ 2026-09-14 14:38 UTC (permalink / raw) To: cem; +Cc: hchc, sandeen, linux-xfs 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 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] xfs: make xfs_iomap_write_direct() report an error to xfs_trans_cancel 2026-09-14 11:57 [PATCH v2 0/3] Enable xfs error reporting to print out error codes cem 2026-09-14 11:57 ` [PATCH v2 1/3] xfs: add xfs_error_report the ability to display an error code cem 2026-09-14 11:57 ` [PATCH v2 2/3] xfs: enable xfs_trans_cancel() to report and " cem @ 2026-09-14 11:57 ` cem 2 siblings, 0 replies; 9+ messages in thread From: cem @ 2026-09-14 11:57 UTC (permalink / raw) To: cem; +Cc: djwong, hchc, sandeen, linux-xfs 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-09-17 10:22 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-14 11:57 [PATCH v2 0/3] Enable xfs error reporting to print out error codes cem 2026-09-14 11:57 ` [PATCH v2 1/3] xfs: add xfs_error_report the ability to display an error code cem 2026-09-14 14:42 ` Darrick J. Wong 2026-09-14 15:00 ` Carlos Maiolino 2026-09-14 15:08 ` Darrick J. Wong 2026-09-17 10:22 ` Carlos Maiolino 2026-09-14 11:57 ` [PATCH v2 2/3] xfs: enable xfs_trans_cancel() to report and " cem 2026-09-14 14:38 ` Darrick J. Wong 2026-09-14 11:57 ` [PATCH v2 3/3] xfs: make xfs_iomap_write_direct() report an error to xfs_trans_cancel cem
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).