On 1/30/19 10:11 PM, Andy Lutomirski wrote:
>>>> On Jan 28, 2019, at 5:20 PM, Jens Axboe <axboe@xxxxxxxxx> wrote:
>>>>
>>>> On 1/28/19 5:47 PM, Andy Lutomirski wrote:
>>>> On Mon, Jan 28, 2019 at 6:57 AM Christoph Hellwig <hch@xxxxxx> wrote:
>>>>
>>>> [please make sure linux-api and linux-man are CCed on new syscalls
>>>> so that we get API experts to review them]
>>>
>>>>> +static int io_import_iovec(struct io_ring_ctx *ctx, int rw,
>>>>> + const struct io_uring_sqe *sqe,
>>>>> + struct iovec **iovec, struct iov_iter *iter)
>>>>> +{
>>>>> + void __user *buf = u64_to_user_ptr(sqe->addr);
>>>>> +
>>>>> +#ifdef CONFIG_COMPAT
>>>>> + if (ctx->compat)
>>>>> + return compat_import_iovec(rw, buf, sqe->len, UIO_FASTIOV,
>>>>> + iovec, iter);
>>>>> +#endif
>>>>
>>>> I think we can just check in_compat_syscall() here, which means we
>>>> can kill the ->compat member, and the separate compat version of the
>>>> setup syscall.
>>>
>>> Since this whole API is new, I don't suppose you could introduce a
>>> struct iovec64 or similar and just make the ABI be identical for
>>> 64-bit and 32-bit code?
>>
>> Sure, that would be straight forward. Is there a strong reason to do
>> so outside of "that would be nice"? It's not like it's a huge amount
>> of code.
>
> Here are some minor-ish benefits:
>
> - It avoids having a code path that is only used with 32 bit code on
> 64 bit kernels and is therefore rarely tested. (In this particular
> case, the code path doesn't diverge much, but for most compat
> syscalls, it's almost an entirely separate implementation of the main
> syscall code.)
>
> - It makes life easier for tools like strace.
>
> - It minimizes the chance of making a giant mess on x32, which isn't
> really native or compat.
Not really anything major here, at least not to the extent that
suffering the pain of having a different iovec for this is warranted.
--
Jens Axboe
