svn commit: r367744 - in head/sys: compat/freebsd32 kern sys

[Brooks Davis]

On Tue, Nov 17, 2020 at 11:59:50AM -0600, Kyle Evans wrote:
> On Tue, Nov 17, 2020 at 11:11 AM Brooks Davis <brooks at freebsd.org> wrote:
> >
> > On Tue, Nov 17, 2020 at 03:36:58AM +0000, Kyle Evans wrote:
> > > Modified: head/sys/compat/freebsd32/freebsd32.h
> > > ==============================================================================
> > > --- head/sys/compat/freebsd32/freebsd32.h     Tue Nov 17 03:34:01 2020        (r367743)
> > > +++ head/sys/compat/freebsd32/freebsd32.h     Tue Nov 17 03:36:58 2020        (r367744)
> > > @@ -94,6 +94,27 @@ struct itimerval32 {
> > >       struct timeval32 it_value;
> > >  };
> > >
> > > +struct umtx_time32 {
> > > +     struct  timespec32      _timeout;
> > > +     uint32_t                _flags;
> > > +     uint32_t                _clockid;
> > > +};
> > > +
> > > +struct umtx_robust_lists_params_compat32 {
> > > +     uint32_t        robust_list_offset;
> > > +     uint32_t        robust_priv_list_offset;
> > > +     uint32_t        robust_inact_offset;
> > > +};
> > > +
> > > +struct umutex32 {
> > > +     volatile __lwpid_t      m_owner;        /* Owner of the mutex */
> > > +     __uint32_t              m_flags;        /* Flags of the mutex */
> > > +     __uint32_t              m_ceilings[2];  /* Priority protect ceiling */
> > > +     __uint32_t              m_rb_lnk;       /* Robust linkage */
> > > +     __uint32_t              m_pad;
> > > +     __uint32_t              m_spare[2];
> > > +};
> > > +
> > >  #define FREEBSD4_MFSNAMELEN  16
> > >  #define FREEBSD4_MNAMELEN    (88 - 2 * sizeof(int32_t))
> > >
> > >
> > > Modified: head/sys/compat/freebsd32/freebsd32_misc.c
> > > ==============================================================================
> > > --- head/sys/compat/freebsd32/freebsd32_misc.c        Tue Nov 17 03:34:01 2020        (r367743)
> > > +++ head/sys/compat/freebsd32/freebsd32_misc.c        Tue Nov 17 03:36:58 2020        (r367744)
> > > @@ -84,6 +84,7 @@ __FBSDID("$FreeBSD$");
> > >  #include <sys/thr.h>
> > >  #include <sys/unistd.h>
> > >  #include <sys/ucontext.h>
> > > +#include <sys/umtx.h>
> > >  #include <sys/vnode.h>
> > >  #include <sys/wait.h>
> > >  #include <sys/ipc.h>
> > > @@ -3764,4 +3765,12 @@ freebsd32_sched_rr_get_interval(struct thread *td,
> > >               error = copyout(&ts32, uap->interval, sizeof(ts32));
> > >       }
> > >       return (error);
> > > +}
> > > +
> > > +int
> > > +freebsd32__umtx_op(struct thread *td, struct freebsd32__umtx_op_args *uap)
> > > +{
> > > +
> > > +     return (kern__umtx_op(td, uap->obj, uap->op, uap->val, uap->uaddr,
> > > +         uap->uaddr2, &umtx_native_ops32));
> > >  }
> > >
> >
> > Putting any of this under compat/freebsd32 seems like a somewhat
> > odd choice since all the work is done in kern_umtx.h.  In CheriBSD,
> > everything just lives there so nothing has to be exposed in headers.
> >
> 
> I have no strong opinion here -- my initial impression of the
> suggestion to move the struct definitions into freebsd32 was that:
> 
> 1.) One can then quickly reference the definition of, e.g., timespec32
> when I'm looking at a umtx_time32, and
> 2.) It'd be 'cleaner', requiring less #ifdef soup in kern_umtx.c
> 
> The follow-up patch muddies the waters a lot, as we end up using the
> compat32 definitions on all 64-bit platforms anyways even without
> compat32. I don't object to moving any/all of this back, if you think
> that's better.

(1) makes sense to me.  I'm less convinced of (2) especially given the
followup.  As a rule, I've been removing compat bits from headers when
they only need to be defined in a single .c file.  If nothing else, I
don't like that it presents a somewhat-false implication that the
interfaces are public (and there have been quite a few cases where they
weren't correctly guarded with _KERNEL).

-- Brooks
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-all/attachments/20201117/b40f07cc/attachment.sig>