public inbox for [email protected]
 help / color / mirror / Atom feed
From: Pavel Begunkov <[email protected]>
To: Jens Axboe <[email protected]>,
	[email protected], [email protected],
	[email protected]
Cc: Alexander Viro <[email protected]>
Subject: Re: [POC RFC 0/3] splice(2) support for io_uring
Date: Wed, 22 Jan 2020 06:11:27 +0300	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>


[-- Attachment #1.1: Type: text/plain, Size: 2148 bytes --]

On 22/01/2020 04:55, Jens Axboe wrote:
> On 1/21/20 5:05 PM, Pavel Begunkov wrote:
>> It works well for basic cases, but there is still work to be done. E.g.
>> it misses @hash_reg_file checks for the second (output) file. Anyway,
>> there are some questions I want to discuss:
>>
>> - why sqe->len is __u32? Splice uses size_t, and I think it's better
>> to have something wider (e.g. u64) for fututre use. That's the story
>> behind added sqe->splice_len.
> 
> IO operations in Linux generally are INT_MAX, so the u32 is plenty big.
> That's why I chose it. For this specifically, if you look at splice:
> 
> 	if (unlikely(len > MAX_RW_COUNT))
> 		len = MAX_RW_COUNT;
> 
> so anything larger is truncated anyway.

Yeah, I saw this one, but that was rather an argument for the future. It's
pretty easy to transfer more than 4GB with sg list, but that would be the case
for splice.

> 
>> - it requires 2 fds, and it's painful. Currently file managing is done
>> by common path (e.g. io_req_set_file(), __io_req_aux_free()). I'm
>> thinking to make each opcode function handle file grabbing/putting
>> themself with some helpers, as it's done in the patch for splice's
>> out-file.
>>     1. Opcode handler knows, whether it have/needs a file, and thus
>>        doesn't need extra checks done in common path.
>>     2. It will be more consistent with splice.
>> Objections? Ideas?
> 
> Sounds reasonable to me, but always easier to judge in patch form :-)
> 
>> - do we need offset pointers with fallback to file->f_pos? Or is it
>> enough to have offset value. Jens, I remember you added the first
>> option somewhere, could you tell the reasoning?
> 
> I recently added support for -1/cur position, which splice also uses. So
> you should be fine with that.
> 

I always have been thinking about it as a legacy from old days, and one of the
problems of posix. It's not hard to count it in the userspace especially in C++
or high-level languages, and is just another obstacle for having a performant
API. So, I'd rather get rid of it here. But is there any reasons against?

-- 
Pavel Begunkov


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2020-01-22  3:12 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-22  0:05 [POC RFC 0/3] splice(2) support for io_uring Pavel Begunkov
2020-01-22  0:05 ` [PATCH 1/3] splice: make do_splice public Pavel Begunkov
2020-01-22  0:05 ` [PATCH 2/3] io_uring: add interface for getting files Pavel Begunkov
2020-01-22  1:54   ` Jens Axboe
2020-01-22  2:24     ` Pavel Begunkov
2020-01-22  0:05 ` [PATCH 3/3] io_uring: add splice(2) support Pavel Begunkov
2020-01-22  2:03   ` Jens Axboe
2020-01-22  2:40     ` Pavel Begunkov
2020-01-22  2:47       ` Jens Axboe
2020-01-22  3:16         ` Pavel Begunkov
2020-01-22  3:22           ` Jens Axboe
2020-01-24 12:31   ` kbuild test robot
2020-01-25 18:28   ` kbuild test robot
2020-01-22  1:55 ` [POC RFC 0/3] splice(2) support for io_uring Jens Axboe
2020-01-22  3:11   ` Pavel Begunkov [this message]
2020-01-22  3:30     ` Jens Axboe
  -- strict thread matches above, loose matches on Subject: below --
2020-04-06 19:09 Askar Safin
2020-04-06 20:01 ` Pavel Begunkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox