Re: [RFC PATCH 28/30] Improved symbolic error names

2022-09-01 Thread Kent Overstreet
On Thu, Sep 01, 2022 at 04:19:35PM -0700, Joe Perches wrote:
> On Tue, 2022-08-30 at 14:49 -0700, Suren Baghdasaryan wrote:
> > From: Kent Overstreet 
> > 
> > This patch adds per-error-site error codes, with error strings that
> > include their file and line number.
> > 
> > To use, change code that returns an error, e.g.
> > return -ENOMEM;
> > to
> > return -ERR(ENOMEM);
> > 
> > Then, errname() will return a string that includes the file and line
> > number of the ERR() call, for example
> > printk("Got error %s!\n", errname(err));
> > will result in
> > Got error ENOMEM at foo.c:1234
> 
> Why? Something wrong with just using %pe ?
> 
>   printk("Got error %pe at %s:%d!\n", ERR_PTR(err), __FILE__, __LINE__);
> 
> Likely __FILE__ and __LINE__ aren't particularly useful.

That doesn't do what this patchset does. If it only did that, it wouldn't make
much sense, would it? :)

With this patchset,
 printk("Got error %pe!\n", ptr);

prints out a file and line number, but it's _not_ the file/line number of the
printk statement - it's the file/line number where the error originated!

:)



Re: [RFC PATCH 28/30] Improved symbolic error names

2022-09-01 Thread Joe Perches
On Tue, 2022-08-30 at 14:49 -0700, Suren Baghdasaryan wrote:
> From: Kent Overstreet 
> 
> This patch adds per-error-site error codes, with error strings that
> include their file and line number.
> 
> To use, change code that returns an error, e.g.
> return -ENOMEM;
> to
> return -ERR(ENOMEM);
> 
> Then, errname() will return a string that includes the file and line
> number of the ERR() call, for example
> printk("Got error %s!\n", errname(err));
> will result in
> Got error ENOMEM at foo.c:1234

Why? Something wrong with just using %pe ?

printk("Got error %pe at %s:%d!\n", ERR_PTR(err), __FILE__, __LINE__);

Likely __FILE__ and __LINE__ aren't particularly useful.

And using ERR would add rather a lot of bloat as each codetag_error_code
struct would be unique.

+#define ERR(_err)  \
+({ \
+   static struct codetag_error_code\
+   __used  \
+   __section("error_code_tags")\
+   __aligned(8) e = {  \
+   .str= #_err " at " __FILE__ ":" __stringify(__LINE__),\
+   .err= _err, \
+   };  \
+   \
+   e.err;  \
+})




[RFC PATCH 28/30] Improved symbolic error names

2022-08-30 Thread Suren Baghdasaryan
From: Kent Overstreet 

This patch adds per-error-site error codes, with error strings that
include their file and line number.

To use, change code that returns an error, e.g.
return -ENOMEM;
to
return -ERR(ENOMEM);

Then, errname() will return a string that includes the file and line
number of the ERR() call, for example
printk("Got error %s!\n", errname(err));
will result in
Got error ENOMEM at foo.c:1234

To convert back to the original error code (before returning it to
outside code that does not understand dynamic error codes), use
return error_class(err);

To test if an error is of some type, replace
if (err == -ENOMEM)
with
if (error_matches(err, ENOMEM))

Implementation notes:

Error codes are allocated dynamically on module load and deallocated on
module unload. On memory allocation failure (i.e. the data structures
for indexing error strings and error parents), ERR() will fall back to
returning the error code that it was passed.

MAX_ERRNO has been raised from 4096 to 1 million, which should be
sufficient given the number of lines of code and the fraction that throw
errors in the kernel codebase.

This has implications for ERR_PTR(), since the range of the address
space reserved for errors is unavailable for other purposes. Since
ERR_PTR() ptrs are at the top of the address space there should not be
any major difficulties.

Signed-off-by: Kent Overstreet 
---
 include/asm-generic/codetag.lds.h |   3 +-
 include/linux/err.h   |   2 +-
 include/linux/errname.h   |  50 +++
 lib/errname.c | 103 ++
 4 files changed, 156 insertions(+), 2 deletions(-)

diff --git a/include/asm-generic/codetag.lds.h 
b/include/asm-generic/codetag.lds.h
index d799f4aced82..b087cf1874a9 100644
--- a/include/asm-generic/codetag.lds.h
+++ b/include/asm-generic/codetag.lds.h
@@ -11,6 +11,7 @@
 #define CODETAG_SECTIONS() \
SECTION_WITH_BOUNDARIES(alloc_tags) \
SECTION_WITH_BOUNDARIES(dynamic_fault_tags) \
-   SECTION_WITH_BOUNDARIES(time_stats_tags)
+   SECTION_WITH_BOUNDARIES(time_stats_tags)\
+   SECTION_WITH_BOUNDARIES(error_code_tags)
 
 #endif /* __ASM_GENERIC_CODETAG_LDS_H */
diff --git a/include/linux/err.h b/include/linux/err.h
index a139c64aef2a..1d8d6c46ab9c 100644
--- a/include/linux/err.h
+++ b/include/linux/err.h
@@ -15,7 +15,7 @@
  * This should be a per-architecture thing, to allow different
  * error and pointer decisions.
  */
-#define MAX_ERRNO  4095
+#define MAX_ERRNO  ((1 << 20) - 1)
 
 #ifndef __ASSEMBLY__
 
diff --git a/include/linux/errname.h b/include/linux/errname.h
index e8576ad90cb7..dd39fe7120bb 100644
--- a/include/linux/errname.h
+++ b/include/linux/errname.h
@@ -5,12 +5,62 @@
 #include 
 
 #ifdef CONFIG_SYMBOLIC_ERRNAME
+
 const char *errname(int err);
+
+#include 
+
+struct codetag_error_code {
+   const char  *str;
+   int err;
+};
+
+/**
+ * ERR - return an error code that records the error site
+ *
+ * E.g., instead of
+ *   return -ENOMEM;
+ * Use
+ *   return -ERR(ENOMEM);
+ *
+ * Then, when a caller prints out the error with errname(), the error string
+ * will include the file and line number.
+ */
+#define ERR(_err)  \
+({ \
+   static struct codetag_error_code\
+   __used  \
+   __section("error_code_tags")\
+   __aligned(8) e = {  \
+   .str= #_err " at " __FILE__ ":" __stringify(__LINE__),\
+   .err= _err, \
+   };  \
+   \
+   e.err;  \
+})
+
+int error_class(int err);
+bool error_matches(int err, int class);
+
 #else
+
+static inline int error_class(int err)
+{
+   return err;
+}
+
+static inline bool error_matches(int err, int class)
+{
+   return err == class;
+}
+
+#define ERR(_err)  _err
+
 static inline const char *errname(int err)
 {
return NULL;
 }
+
 #endif
 
 #endif /* _LINUX_ERRNAME_H */
diff --git a/lib/errname.c b/lib/errname.c
index 05cbf731545f..2db8f5301ba0 100644
--- a/lib/errname.c
+++ b/lib/errname.c
@@ -1,9 +1,20 @@
 // SPDX-License-Identifier: GPL-2.0
 #include 
+#include 
 #include 
 #include 
+#include 
 #include 
 #include 
+#include 
+#include 
+
+#define DYNAMIC_ERRCODE_START  4096
+
+static DEFINE_IDR(dynamic_error_strings);
+static DEFINE_XARRAY(error_classes);
+
+static struct codetag_type *cttype;
 
 /*
  * Ensure these tables do not accidentally become giganti