Thread (10 messages) 10 messages, 3 authors, 2022-02-11

Re: [PATCH v2 2/3] block: io_uring: add READV_PI/WRITEV_PI operations

From: Alexander V. Buev <hidden>
Date: 2022-02-11 09:40:03
Also in: io-uring

On 2/10/22 12:03 PM, Alexander V. Buev wrote:
quoted
quoted
On 2/10/22 6:08 AM, Alexander V. Buev wrote:
quoted
Added new READV_PI/WRITEV_PI operations to io_uring.
Added new pi_addr & pi_len fields to SQE struct.
Added new pi_iter field and IOCB_USE_PI flag to kiocb struct.
Make corresponding corrections to io uring trace event.

+struct io_rw_pi_state {
+	struct iov_iter			iter;
+	struct iov_iter_state		iter_state;
+	struct iovec			fast_iov[UIO_FASTIOV_PI];
+};
+
+struct io_rw_pi {
+	struct io_rw			rw;
+	struct iovec			*pi_iov;
+	u32				nr_pi_segs;
+	struct io_rw_pi_state		*s;
+};
One immediate issue I see here is that io_rw_pi is big, and we try very
hard to keep the per-command payload to 64-bytes. This would be 88 bytes
by my count :-/

Do you need everything from io_rw? If not, I'd just make io_rw_pi
contain the bits you need and see if you can squeeze it into the
existing cacheline.
In short - Yes. Current patch code call existing io_read/io_write functions.
This functions use io_rw struct information and process this data.
I wanted to use existing functions but may be this is wrong way in this 
case.
                                                                                
The second problem with request size is that the patch adds pi_iter   
pointer to kiocb struct. This also increase whole request union
length.

So I can see some (may be possible) solution for this: 

 1) do not store whole kiocb struct in request
    and write fully separated io_read/write_pi functions

 2) make special CONFIG_XXX variable and simplify hide this code
    as default
Option 2 really sucks, because then obviously everyone wants their
feature enabled, and then we are back to square one. So never rely on a
config option, if it can be avoided.

I'd like to see what option 1 looks like, that sounds like a far better
solution.
Accepted. I am starting to prepare v3 in this way. 

Thanks to all for feedback!

-- 
Alexander Buev
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help