On Tue, Apr 04, 2023 at 03:09:35PM +0100, Tudor Cretu wrote:
On 04-04-2023 14:28, Beata Michalska wrote:
On Tue, Apr 04, 2023 at 01:46:37PM +0200, Kevin Brodsky wrote:
- Beata
On 04/04/2023 12:22, Tudor Cretu wrote:
AFAICT __pad2 also needs to be expanded. In fact I'm surprised that these changes work in PCuABI as they stand, because with __pad2 unchanged I would expect the struct to be 16 bytes smaller than the uapi definition. Maybe I'm missing something?
Good catch!
This struct is not actually used. The uAPI header is used if it exists, here (lines 21-25):
https://git.morello-project.org/tudcre01/morello-linux-ltp/-/blob/81111e0f44...
Right I understand now! But then maybe we shouldn't bother updating these definitions? Do other tests actually build fine in purecap without uapi headers? If we modify these fallbacks we may want to test them too, but I'm not sure it's worth it.
I considered not updating them.
I'm starting to wonder if it's not the same situation for struct clone_args, which we modified but might not be actually using?
clone_args seems slightly different as its definition is not being pulled from kernel headers so this one needed to be updated as Musl does not provide one. I guess we could make it all depend on kernel headers but then we are ending up in a mess when respective libc(s) might be expecting smth else to maintain backward compatibility.
In the case of io_uring, the struct definition depends on IOSQE_FIXED_FILE (not sure why such a choice of def guard) so as long as that one is not defined in the uAPI header, the 'custom' definition of io_uring_sqe will be provided, so I assume that's for older kernels which might be potentially left-as-is, it is slightly behind kernel version either way.
The choice for the def guard is strange indeed. The struct io_uring_sqe was introduced in the io_uring uAPI header in a kernel version before IOSQE_FIXED_FILE was introduced. So there is a range of kernel versions where this LTP header would cause compilation issues of redefining the io_uring_sqe struct.
Though that would require placing somewhere a comment saying this one is not considered to be an issue for our use case. (?)
I'm not sure if I understand correctly. Is your suggestion to leave the 'custom' definition of io_uring_sqe unmodified and just add a comment?
Yes, what I am suggesting is leaving things as they are: this definition should not be pulled in for newer kernels, including Morello kernel, so it should be safe to leave it unmodified, as log as we leave some trace on why we have decided to do so.
--- BR B.
Kind regards, Tudor
BR B.
Kevin