From eb386617be4bdfe02eb0972874f726e2bfc7a6e7 Mon Sep 17 00:00:00 2001 From: Kent Overstreet Date: Wed, 21 Feb 2024 22:10:09 -0500 Subject: [PATCH] bcachefs: Errcode tracepoint, documentation Add a tracepoint for downcasting private errors to standard errors, so they can be recovered even when not logged; also, add some documentation. Signed-off-by: Kent Overstreet --- .../filesystems/bcachefs/errorcodes.rst | 30 +++++++++++++++++++ fs/bcachefs/errcode.c | 15 ++++++---- fs/bcachefs/errcode.h | 1 + fs/bcachefs/trace.h | 19 ++++++++++++ 4 files changed, 59 insertions(+), 6 deletions(-) create mode 100644 Documentation/filesystems/bcachefs/errorcodes.rst diff --git a/Documentation/filesystems/bcachefs/errorcodes.rst b/Documentation/filesystems/bcachefs/errorcodes.rst new file mode 100644 index 000000000000..2cccaa0ba7cd --- /dev/null +++ b/Documentation/filesystems/bcachefs/errorcodes.rst @@ -0,0 +1,30 @@ +.. SPDX-License-Identifier: GPL-2.0 + +bcachefs private error codes +---------------------------- + +In bcachefs, as a hard rule we do not throw or directly use standard error +codes (-EINVAL, -EBUSY, etc.). Instead, we define private error codes as needed +in fs/bcachefs/errcode.h. + +This gives us much better error messages and makes debugging much easier. Any +direct uses of standard error codes you see in the source code are simply old +code that has yet to be converted - feel free to clean it up! + +Private error codes may subtype another error code, this allows for grouping of +related errors that should be handled similarly (e.g. transaction restart +errors), as well as specifying which standard error code should be returned at +the bcachefs module boundary. + +At the module boundary, we use bch2_err_class() to convert to a standard error +code; this also emits a trace event so that the original error code be +recovered even if it wasn't logged. + +Do not reuse error codes! Generally speaking, a private error code should only +be thrown in one place. That means that when we see it in a log message we can +see, unambiguously, exactly which file and line number it was returned from. + +Try to give error codes names that are as reasonably descriptive of the error +as possible. Frequently, the error will be logged at a place far removed from +where the error was generated; good names for error codes mean much more +descriptive and useful error messages. diff --git a/fs/bcachefs/errcode.c b/fs/bcachefs/errcode.c index d260ff9bbfeb..43557bebd0f8 100644 --- a/fs/bcachefs/errcode.c +++ b/fs/bcachefs/errcode.c @@ -2,6 +2,7 @@ #include "bcachefs.h" #include "errcode.h" +#include "trace.h" #include @@ -49,15 +50,17 @@ bool __bch2_err_matches(int err, int class) return err == class; } -int __bch2_err_class(int err) +int __bch2_err_class(int bch_err) { - err = -err; - BUG_ON((unsigned) err >= BCH_ERR_MAX); + int std_err = -bch_err; + BUG_ON((unsigned) std_err >= BCH_ERR_MAX); - while (err >= BCH_ERR_START && bch2_errcode_parents[err - BCH_ERR_START]) - err = bch2_errcode_parents[err - BCH_ERR_START]; + while (std_err >= BCH_ERR_START && bch2_errcode_parents[std_err - BCH_ERR_START]) + std_err = bch2_errcode_parents[std_err - BCH_ERR_START]; - return -err; + trace_error_downcast(bch_err, std_err, _RET_IP_); + + return -std_err; } const char *bch2_blk_status_to_str(blk_status_t status) diff --git a/fs/bcachefs/errcode.h b/fs/bcachefs/errcode.h index fe3fc14d3c9a..e960a6eae66a 100644 --- a/fs/bcachefs/errcode.h +++ b/fs/bcachefs/errcode.h @@ -78,6 +78,7 @@ x(ENOMEM, ENOMEM_fs_name_alloc) \ x(ENOMEM, ENOMEM_fs_other_alloc) \ x(ENOMEM, ENOMEM_dev_alloc) \ + x(ENOMEM, ENOMEM_disk_accounting) \ x(ENOSPC, ENOSPC_disk_reservation) \ x(ENOSPC, ENOSPC_bucket_alloc) \ x(ENOSPC, ENOSPC_disk_label_add) \ diff --git a/fs/bcachefs/trace.h b/fs/bcachefs/trace.h index 293b90d704fb..6aa81d1e6d36 100644 --- a/fs/bcachefs/trace.h +++ b/fs/bcachefs/trace.h @@ -1431,6 +1431,25 @@ DEFINE_EVENT(fs_str, data_update, TP_ARGS(c, str) ); +TRACE_EVENT(error_downcast, + TP_PROTO(int bch_err, int std_err, unsigned long ip), + TP_ARGS(bch_err, std_err, ip), + + TP_STRUCT__entry( + __array(char, bch_err, 32 ) + __array(char, std_err, 32 ) + __array(char, ip, 32 ) + ), + + TP_fast_assign( + strscpy(__entry->bch_err, bch2_err_str(bch_err), sizeof(__entry->bch_err)); + strscpy(__entry->std_err, bch2_err_str(std_err), sizeof(__entry->std_err)); + snprintf(__entry->ip, sizeof(__entry->ip), "%ps", (void *) ip); + ), + + TP_printk("%s -> %s %s", __entry->bch_err, __entry->std_err, __entry->ip) +); + #endif /* _TRACE_BCACHEFS_H */ /* This part must be outside protection */