Re: [PATCH V3 02/16] io_uring: add IORING_OP_FUSED_CMD

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

On Sat, Mar 18, 2023 at 08:31:44AM -0600, Jens Axboe wrote:
> On 3/14/23 6:57?AM, Ming Lei wrote:
> > 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 has to be set as 1,
> > and its ->issue() needs to retrieve buffer from master request's
> > fused_cmd_kbuf.
> 
> Since we'd be introducing this as a new concept, probably makes sense to
> name it something other than master/slave. What about primary and
> secondary? Producer/consumer?

Either of the two looks fine for me, and I will take secondary in next
version if no one objects.

> 
> > +static inline bool io_fused_slave_write_to_buf(u8 op)
> > +{
> > +	switch (op) {
> > +	case IORING_OP_READ:
> > +	case IORING_OP_READV:
> > +	case IORING_OP_READ_FIXED:
> > +	case IORING_OP_RECVMSG:
> > +	case IORING_OP_RECV:
> > +		return 1;
> > +	default:
> > +		return 0;
> > +	}
> > +}
> 
> Maybe add a data direction bit to the hot opdef part? Any command that
> has fused support should ensure that it is set correctly.

Good idea!

> 
> > +int io_import_kbuf_for_slave(unsigned long buf_off, unsigned int len, int dir,
> > +		struct iov_iter *iter, struct io_kiocb *slave)
> > +{
> 
> The kbuf naming should probably also change, as it kind of overlaps with
> the kbufs we already have and which are not really related.

How about _bvec_buf_ or simply _buf_?

Thanks,
Ming