Re: [PATCH V2 00/17] io_uring/ublk: add IORING_OP_FUSED_CMD

[Ming Lei <ming.lei@redhat.com>]

On Wed, Mar 15, 2023 at 03:08:21PM +0800, Ziyang Zhang wrote:
> On 2023/3/7 22:15, Ming Lei wrote:
> > Hello,
> > 
> > Add IORING_OP_FUSED_CMD, it is one special URING_CMD, which has to
> > be SQE128. The 1st SQE(master) is one 64byte URING_CMD, and the 2nd
> > 64byte SQE(slave) is another normal 64byte OP. For any OP which needs
> > to support slave OP, io_issue_defs[op].fused_slave needs to be set as 1,
> > and its ->issue() can retrieve/import buffer from master request's
> > fused_cmd_kbuf. The slave OP is actually submitted from kernel, part of
> > this idea is from Xiaoguang's ublk ebpf patchset, but this patchset
> > submits slave OP just like normal OP issued from userspace, that said,
> > SQE order is kept, and batching handling is done too.
> > 
> > Please see detailed design in commit log of the 3th patch, and one big
> > point is how to handle buffer ownership.
> > 
> > With this way, it is easy to support zero copy for ublk/fuse device.
> > 
> > Basically userspace can specify any sub-buffer of the ublk block request
> > buffer from the fused command just by setting 'offset/len'
> > in the slave SQE for running slave OP. This way is flexible to implement
> > io mapping: mirror, stripped, ...
> > 
> > The 4th & 5th patches enable fused slave support for the following OPs:
> > 
> > 	OP_READ/OP_WRITE
> > 	OP_SEND/OP_RECV/OP_SEND_ZC
> > 
> > The other ublk patches cleans ublk driver and implement fused command
> > for supporting zero copy.
> > 
> > Follows userspace code:
> > 
> > https://github.com/ming1/ubdsrv/tree/fused-cmd-zc-v2
> > 
> > All three(loop, nbd and qcow2) ublk targets have supported zero copy by passing:
> > 
> > 	ublk add -t [loop|nbd|qcow2] -z .... 
> > 
> > Basic fs mount/kernel building and builtin test are done.
> > 
> > Also add liburing test case for covering fused command based on miniublk
> > of blktest:
> > 
> > https://github.com/ming1/liburing/commits/fused_cmd_miniublk
> > 
> > Performance improvement is obvious on memory bandwidth
> > related workloads, such as, 1~2X improvement on 64K/512K BS
> > IO test on loop with ramfs backing file.
> > 
> > Any comments are welcome!
> > 
> 
> 
> Hi Ming,
> 
> Maybe we can split patch 06-12 to a separate cleanup pacthset. I think
> these patches can be merged first because they are not related to zero copy.

Hi Ziyang,

I think the fused command approach is doable, and the patchset is basically
ready to go now:

- the fused command approach won't affect normal uring OP
- most of the change focuses on io_uring/fused_cmd.*
- this approach works reliably and efficiently, pass xfstests, and
  verified on all three ublk targets(loop, nbd, qcow2), improvement is
  pretty obvious

so I'd rather to put them in one series and aim at v6.14, then we can avoid conflict.

However if things doesn't work toward the expected direction, we can move on with
another way at that time, so it is too early to separate the cleanup patchset now.

thanks,
Ming