public inbox for [email protected]
 help / color / mirror / Atom feed
From: Jens Axboe <[email protected]>
To: Pavel Begunkov <[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: Tue, 21 Jan 2020 18:55:59 -0700	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

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.

> - 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.

-- 
Jens Axboe


  parent reply	other threads:[~2020-01-22  1:56 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 ` Jens Axboe [this message]
2020-01-22  3:11   ` [POC RFC 0/3] splice(2) support for io_uring Pavel Begunkov
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