Source-Changes-D archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: CVS commit: src



> Module Name:    src
> Committed By:   christos
> Date:           Sun Nov  3 22:24:23 UTC 2024
> 
> Modified Files:
>         src/distrib/sets/lists/comp: ad.arm ad.m68k ad.mips ad.powerpc ad.riscv
>             ad.sh3 md.alpha md.amd64 md.hppa md.i386 md.ia64 md.or1k md.sparc
>             md.vax
>         src/include: lwp.h
> ...
> Log Message:
> Split __lwp_getprivate_fast and __lwp_*tcb from mcontext.h into a separate
> lwp.h file.

Please revert this immediately.

We discussed this extensively in private the other day.

Everything about this commit is exactly the opposite of the agreement
we came to about how this should be done, as documented in PR
misc/58796 (https://gnats.NetBSD.org/58796), which you didn't cite for
cross-reference as I requested but which I quote here:

> 1. Move the definitions of _lwp_getprivate/gettcb/settcb_fast (plus
> TLS_TP_OFFSET and TLS_DTV_OFFSET, and whatever other parts are
> directly relevant) to a new header file <machine/lwp_private.h>.
> 
> 2. Use #include <lwp.h> in the Arm and PowerPC
> <machine/lwp_private.h> implementations (and whichever other ones
> need _lwp_getprivate/setprivate or similar).
> 
> 3. Include <machine/lwp_private.h> explicitly in all users of
> _lwp_getprivate/gettcb/settcb_fast instead of using the kooky
> _RTLD_SOURCE/_LIBC_SOURCE/__LIBPTHREAD_SOURCE__ conditionalization.

Instead of (1), you have created a new dumping ground machine/lwp.h
which will invite definitions unrelated to the lwp private/tcb
pointer; this is exactly what I objected to.  (I suggested an
alternative machine/lwp_tls.h if the `_private.h' suffix is
unsettling, just as long as it is not `machine/lwp.h', or if you
really insist on machine/lwp.h, then at least put #ifdef _KERNEL
#error with a message clearly stating its narrow scope.)

Instead of (2) where each header file includes what it needs first,
you have created a bogus ordering dependency for <machine/lwp.h> so it
cannot be independently included.

Instead of (3), you have polluted <lwp.h> with private definitions
that were formerly exposed only to rtld/libpthread/libc internals.

And on top of this, you broke almost all the builds (52 out of 73):

https://releng.netbsd.org/builds/HEAD/202411040510Z/

You should expect changes which are exactly the opposite of how the
extensive discussion concluded to be presumptively controversial and
inappropriate for committing without public review.  So please revert
in the next 12h or I will.


Home | Main Index | Thread Index | Old Index