Re: [PATCH 1/2] io_uring: Pass whole sqe to commands

[Breno Leitao <leitao@debian.org>]

On Thu, Apr 20, 2023 at 02:31:39PM +0200, Christoph Hellwig wrote:
> On Thu, Apr 20, 2023 at 05:29:18AM -0700, Breno Leitao wrote:
> > > > -	cmd_size = uring_cmd_pdu_size(req->ctx->flags & IORING_SETUP_SQE128);
> > > > +	if (req->ctx->flags & IORING_SETUP_SQE128)
> > > > +		size <<= 1;
> > > 
> > > 
> > > Why does this stop using uring_cmd_pdu_size()?
> > 
> > Before, only the cmd payload (sqe->cmd) was being copied to the async
> > structure. We are copying over the whole sqe now, since we can use SQE
> > fields inside the ioctl callbacks (instead of only cmd fields). So, the
> > copy now is 64 bytes for single SQE or 128 for double SQEs.
> 
> That's the point of this series and I get it.  But why do we remove
> the nice and self-documenting helper that returns once or twice
> the sizeof of the SQE structure and instead add a magic open coded
> left shift?

uring_cmd_pdu_size() returns the size of the payload, not the size of
the SQE structure. Basically it returns 16 bytes or single SQE or 80
for double SQE.

Since we are not coping the payload anymore, this is not necessary. Now
we are copying 64 bytes for the single SQE or 128 bytes for double SQE.

Do you prefer I create a helper that returns the SQE size, instead of
doing the left shift?

Thank you!