On Sat, Jun 14, 2025 at 08:50:54AM -0400, John Baldwin wrote:
> On 6/13/25 15:40, Konstantin Belousov wrote:
> > The branch main has been updated by kib:
> > 
> > URL: 
> > https://cgit.FreeBSD.org/src/commit/?id=4e207e3849d47648ced17da16aad39355b99d9b2
> > 
> > commit 4e207e3849d47648ced17da16aad39355b99d9b2
> > Author:     Konstantin Belousov <k...@freebsd.org>
> > AuthorDate: 2025-06-13 17:32:50 +0000
> > Commit:     Konstantin Belousov <k...@freebsd.org>
> > CommitDate: 2025-06-13 19:39:55 +0000
> > 
> >      exterr: make SET_ERRORX() macros an expression evaluating to the errno
> >      And move the actual td_kexterr fill code into the function, saving some
> >      text.
> >      Suggested and reviewed by:      markj
> >      Sponsored by:   The FreeBSD Foundation
> >      Differential revision:  https://reviews.freebsd.org/D50836
> > ---
> >   sys/kern/sys_generic.c | 20 ++++++++++++++++++++
> >   sys/sys/exterrvar.h    | 22 +++++++---------------
> >   2 files changed, 27 insertions(+), 15 deletions(-)
> > 
> > diff --git a/sys/kern/sys_generic.c b/sys/kern/sys_generic.c
> > index ec61d0bdc541..d31ff3b939cc 100644
> > --- a/sys/kern/sys_generic.c
> > +++ b/sys/kern/sys_generic.c
> > @@ -2281,3 +2281,23 @@ sys_exterrctl(struct thread *td, struct 
> > exterrctl_args *uap)
> >             return (EINVAL);
> >     }
> >   }
> > +
> > +int
> > +exterr_set(int eerror, int category, const char *mmsg, uintptr_t pp1,
> > +    uintptr_t pp2, int line)
> > +{
> > +   struct thread *td;
> > +
> > +   td = curthread;
> > +   if ((td->td_pflags2 & TDP2_UEXTERR) != 0) {
> > +           td->td_pflags2 |= TDP2_EXTERR;
> > +           td->td_kexterr.error = eerror;
> > +           td->td_kexterr.cat = category;
> > +           td->td_kexterr.msg = mmsg;
> > +           td->td_kexterr.p1 = pp1;
> > +           td->td_kexterr.p2 = pp2;
> > +           td->td_kexterr.src_line = line;
> > +           ktrexterr(td);
> > +   }
> > +   return (eerror);
> > +}
> > diff --git a/sys/sys/exterrvar.h b/sys/sys/exterrvar.h
> > index 4b168446e23b..d3c2c7c92d06 100644
> > --- a/sys/sys/exterrvar.h
> > +++ b/sys/sys/exterrvar.h
> > @@ -31,27 +31,19 @@
> >   #endif
> >   #ifdef    BLOAT_KERNEL_WITH_EXTERR
> > -#define    SET_ERROR_MSG(mmsg)     _Td->td_kexterr.msg = mmsg
> > +#define    SET_ERROR_MSG(mmsg)     (mmsg)
> >   #else
> > -#define    SET_ERROR_MSG(mmsg)     _Td->td_kexterr.msg = NULL
> > +#define    SET_ERROR_MSG(mmsg)     NULL
> >   #endif
> > -#define    SET_ERROR2(eerror, mmsg, pp1, pp2) do { \
> > -   struct thread *_Td = curthread;                         \
> > -   if ((_Td->td_pflags2 & TDP2_UEXTERR) != 0) {            \
> > -           _Td->td_pflags2 |= TDP2_EXTERR;                 \
> > -           _Td->td_kexterr.error = eerror;                 \
> > -           _Td->td_kexterr.cat = EXTERR_CATEGORY;          \
> > -           SET_ERROR_MSG(mmsg);                            \
> > -           _Td->td_kexterr.p1 = (uintptr_t)pp1;            \
> > -           _Td->td_kexterr.p2 = (uintptr_t)pp2;            \
> > -           _Td->td_kexterr.src_line = __LINE__;            \
> > -           ktrexterr(_Td);                                 \
> > -   }                                                       \
> > -} while (0)
> > +#define    SET_ERROR2(eerror, mmsg, pp1, pp2)                              
> > \
> > +   exterr_set(eerror, EXTERR_CATEGORY, SET_ERROR_MSG(mmsg),        \
> > +       (uintptr_t)(pp1), (uintptr_t)(pp2), __LINE__)
> >   #define   SET_ERROR0(eerror, mmsg)        SET_ERROR2(eerror, mmsg, 0, 0)
> >   #define   SET_ERROR1(eerror, mmsg, pp1)   SET_ERROR2(eerror, mmsg, pp1, 0)
> > +int exterr_set(int eerror, int category, const char *mmsg, uintptr_t pp1,
> > +    uintptr_t pp2, int line);
> >   int exterr_to_ue(struct thread *td, struct uexterror *ue);
> >   void ktrexterr(struct thread *td);
> 
> BTW, you could have a single SET_ERROR(eerror, mmsg, ...) wrapper around 
> SET_ERRORx
> following what is done for CTR() (vs CTRx()).

Note that this would collide with an OpenZFS macro of the same name
(which exists to make it easy to insert dtrace probes into error paths).

If your suggestion is implemented, then it would be sensible to use a
different name to avoid collisions, maybe SET_EXTERROR() or just
EXTERROR().

Reply via email to