On Thu, Jan 4, 2018 at 9:00 AM, Christoph Hellwig hch@lst.de wrote:
+}
+SYSCALL_DEFINE6(io_pgetevents,
aio_context_t, ctx_id,long, min_nr,long, nr,struct io_event __user *, events,struct timespec __user *, timeout,const sigset_t __user *, sigmask)+{
+COMPAT_SYSCALL_DEFINE6(io_pgetevents,
compat_aio_context_t, ctx_id,compat_long_t, min_nr,compat_long_t, nr,struct io_event __user *, events,struct compat_timespec __user *, timeout,const compat_sigset_t __user *, sigmask)+{
Hmm, these two new syscall entry points turn into four when we add in support for 64-bit time_t, as we'd have to support all combinations of 32/64 bit aio_context_t and time_t.
Would it be better to start this interface out by defining it using a 64-bit timeout structure? The downside would be that the user space syscall wrappers have to start out with a conversion, if we don't do it, then the opposite conversion would have to get added later.
Arnd
On Tue, Jan 09, 2018 at 11:16:16PM +0100, Arnd Bergmann wrote:
Hmm, these two new syscall entry points turn into four when we add in support for 64-bit time_t, as we'd have to support all combinations of 32/64 bit aio_context_t and time_t.
At least they'll also replace plain old io_getevents :)
Would it be better to start this interface out by defining it using a 64-bit timeout structure? The downside would be that the user space syscall wrappers have to start out with a conversion, if we don't do it, then the opposite conversion would have to get added later.
Which structure do you want? In the end applications using libaio or even the syscalls directly (like seastar) are a special bread, so they could probably just deal with whatever structure we want to pass.
On Wed, Jan 10, 2018 at 9:11 AM, Christoph Hellwig hch@lst.de wrote:
On Tue, Jan 09, 2018 at 11:16:16PM +0100, Arnd Bergmann wrote:
Hmm, these two new syscall entry points turn into four when we add in support for 64-bit time_t, as we'd have to support all combinations of 32/64 bit aio_context_t and time_t.
At least they'll also replace plain old io_getevents :)
Ah, right, so we'd save two other calls in the process: the native 32-bit io_getevents with compat_timespec, and the compat io_getevents with 64-bit timespec.
Would it be better to start this interface out by defining it using a 64-bit timeout structure? The downside would be that the user space syscall wrappers have to start out with a conversion, if we don't do it, then the opposite conversion would have to get added later.
Which structure do you want? In the end applications using libaio or even the syscalls directly (like seastar) are a special bread, so they could probably just deal with whatever structure we want to pass.
I'd suggest passing a variant of timespec with two 64-bit members. Deepa has posted patches for this structure in the past and was planning to do a new version (with minor changes from review) soon, but we can just well use it in your patch if that gets merged first.
If we merge io_pgetevents quickly (before the bulk of the y2038 syscall conversion), I'd say we should use
struct __kernel_timespec64 { __s64 tv_sec; __s64 tv_nsec; };
The tv_nsec type is unfortunately much trickier than it should be: C99 requires it to be 'long', so user space needs to define the 64-bit 'struct timespec' with internal padding in the right places depending on endianess, and the kernel has to be careful about either zeroing the upper half or checking it for being zeroed by user space depending on whether we come from a 32-bit or 64-bit task, but I'm fairly sure we have that part worked out by now.
Arnd
On Wed, Jan 10, 2018 at 12:03:24PM +0100, Arnd Bergmann wrote:
I'd suggest passing a variant of timespec with two 64-bit members. Deepa has posted patches for this structure in the past and was planning to do a new version (with minor changes from review) soon, but we can just well use it in your patch if that gets merged first.
If we merge io_pgetevents quickly (before the bulk of the y2038 syscall conversion), I'd say we should use
struct __kernel_timespec64 { __s64 tv_sec; __s64 tv_nsec; };
The tv_nsec type is unfortunately much trickier than it should be: C99 requires it to be 'long', so user space needs to define the 64-bit 'struct timespec' with internal padding in the right places depending on endianess, and the kernel has to be careful about either zeroing the upper half or checking it for being zeroed by user space depending on whether we come from a 32-bit or 64-bit task, but I'm fairly sure we have that part worked out by now.
Eww. Being the ginea pig is never good, and in this the fact that the aio syscalls aren't in glibc is just going to make things worse in chances of diverging from the future 'standard'.
I'm tempted to say I'd rather deal with the new 64-bit time_t version later, especially as that should only affect 32-bit ports.
On Wed, Jan 10, 2018 at 3:59 PM, Christoph Hellwig hch@lst.de wrote:
On Wed, Jan 10, 2018 at 12:03:24PM +0100, Arnd Bergmann wrote:
I'd suggest passing a variant of timespec with two 64-bit members. Deepa has posted patches for this structure in the past and was planning to do a new version (with minor changes from review) soon, but we can just well use it in your patch if that gets merged first.
If we merge io_pgetevents quickly (before the bulk of the y2038 syscall conversion), I'd say we should use
struct __kernel_timespec64 { __s64 tv_sec; __s64 tv_nsec; };
The tv_nsec type is unfortunately much trickier than it should be: C99 requires it to be 'long', so user space needs to define the 64-bit 'struct timespec' with internal padding in the right places depending on endianess, and the kernel has to be careful about either zeroing the upper half or checking it for being zeroed by user space depending on whether we come from a 32-bit or 64-bit task, but I'm fairly sure we have that part worked out by now.
Eww. Being the ginea pig is never good, and in this the fact that the aio syscalls aren't in glibc is just going to make things worse in chances of diverging from the future 'standard'.
I'm tempted to say I'd rather deal with the new 64-bit time_t version later, especially as that should only affect 32-bit ports.
Ok, fair enough. Given that the old version gets obsoleted by this one, it shouldn't get much uglier than it already is here when you start out with a regular timespec. We just need to be aware of possible clashes when we get the patches merged at the same time.
Arnd
On Wed, Jan 10, 2018 at 04:48:05PM +0100, Arnd Bergmann wrote:
Ok, fair enough. Given that the old version gets obsoleted by this one, it shouldn't get much uglier than it already is here when you start out with a regular timespec. We just need to be aware of possible clashes when we get the patches merged at the same time.
I still hope to get this into 4.15. If it gets delayed and we have a coherent 64-bit time_t abi around already I can reconsider the decision.
On Wed, Jan 10, 2018 at 04:49:04PM +0100, Christoph Hellwig wrote:
On Wed, Jan 10, 2018 at 04:48:05PM +0100, Arnd Bergmann wrote:
Ok, fair enough. Given that the old version gets obsoleted by this one, it shouldn't get much uglier than it already is here when you start out with a regular timespec. We just need to be aware of possible clashes when we get the patches merged at the same time.
I still hope to get this into 4.15. If it gets delayed and we have a coherent 64-bit time_t abi around already I can reconsider the decision.
s/4.15/4.16/ of course.
-
Arnd Bergmann -
Christoph Hellwig