On 1/16/19 8:16 AM, Arnd Bergmann wrote:
> On Wed, Jan 16, 2019 at 4:12 PM Jens Axboe <axboe@xxxxxxxxx> wrote:
>> On 1/16/19 3:41 AM, Arnd Bergmann wrote:
>>> On Tue, Jan 15, 2019 at 3:55 AM Jens Axboe <axboe@xxxxxxxxx> wrote:
>>>> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
>>>> index 3cf7b533b3d1..194e79c0032e 100644
>>>> --- a/arch/x86/entry/syscalls/syscall_32.tbl
>>>> +++ b/arch/x86/entry/syscalls/syscall_32.tbl
>>>> +SYSCALL_DEFINE2(io_uring_setup, u32, entries,
>>>> + struct io_uring_params __user *, params)
>>>> +{
>>>> + return io_uring_setup(entries, params, false);
>>>> +}
>>>> +
>>>> +#ifdef CONFIG_COMPAT
>>>> +COMPAT_SYSCALL_DEFINE2(io_uring_setup, u32, entries,
>>>> + struct io_uring_params __user *, params)
>>>> +{
>>>> + return io_uring_setup(entries, params, true);
>>>> +}
>>>> +#endif
>>>
>>> The compat syscall has the same calling conventions as the
>>> native one here, so I think you can just use that directly.
>>
>> Not sure I understand what you mean here. I need to know if it's the
>> compat one, hence 'true' vs 'false', so I know what the size of the user
>> pointers/structs are.
>
> My mistake, I missed the true/false difference between the two
> functions.
>
>>>> +/*
>>>> + * IO submission data structure (Submission Queue Entry)
>>>> + */
>>>> +struct io_uring_sqe {
>>>> + __u8 opcode; /* type of operation for this sqe */
>>>> + __u8 flags; /* as of now unused */
>>>> + __u16 ioprio; /* ioprio for the request */
>>>> + __s32 fd; /* file descriptor to do IO on */
>>>> + __u64 off; /* offset into file */
>>>> + union {
>>>> + void *addr; /* buffer or iovecs */
>>>> + __u64 __pad;
>>>> + };
>>>
>>> It seems a bit unfortunate to keep the pointer field only
>>> almost compatible between 32-bit and 64-bit big-endian
>>> architectures, as that requires an in_compat_syscall()
>>> check whenever we access the pointer from the kernel.
>>>
>>> Could you use a __u64 field to store the pointer itself
>>> instead?
>>
>> I feel like I'm missing something here, we'll still need the compat code
>> on the kernel side for 32-bit app on 64-bit kernel, so what would we
>> solve by making this an __u64?
>
> It means you don't have to define a compat_io_uring_sqe
> structure with a compat_uptr_t member in it.
Yeah, I finally got it, I'm making the change. Thanks!
--
Jens Axboe
