From: Mathieu Desnoyers
Sent: 25 January 2022 19:01
----- On Jan 25, 2022, at 9:41 AM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:
----- On Jan 25, 2022, at 7:21 AM, Christian Brauner brauner@kernel.org wrote:
[...]
include/uapi/linux/rseq.h | 17 ++++-------------
[...]
union {
A bit unfortunate we seem to have to keep the union around even though it's just one field now.
Well, as far as the user-space projects that I know of which use rseq are concerned (glibc, librseq, tcmalloc), those end up with their own copy of the uapi header anyway to deal with the big/little endian field on 32-bit. So I'm very much open to remove the union if we accept that this uapi header is really just meant to express the ABI and is not expected to be used as an API by user-space.
That would mean we also bring a uapi header copy into the kernel rseq selftests as well to minimize the gap between librseq and the kernel sefltests (the kernel sefltests pretty much include a copy of librseq for convenience. librseq is maintained out of tree).
Thoughts ?
Actually, if we go ahead and remove the union, and replace:
struct rseq { union { __u64 ptr64; } rseq_cs; [...] } v;
by:
struct rseq { __u64 rseq_cs; } v;
expressions such as these are unchanged:
- sizeof(v.rseq_cs),
- &v.rseq_cs,
- __alignof__(v.rseq_cs),
- offsetof(struct rseq, rseq_cs).
So users of the uapi rseq.h (as an API) can still use rseq_abi->rseq_cs before and after the change.
But: v.rseq_cs.ptr_64 = (uintptr_t)&foo; is broken.
Based on this, I am inclined to remove the union, and just make the rseq_cs field a __u64.
It really is a shame that you can't do: void *rseq_cs __attribute__((size(8))); and have the compiler just DTRT on 32bit systems.
David
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)