Re: [PATCH 2/9] io_uring: support user sqe ext flags

[Pavel Begunkov <asml.silence@gmail.com>]

On 4/23/24 14:57, Ming Lei wrote:
> On Mon, Apr 22, 2024 at 12:16:12PM -0600, Jens Axboe wrote:
>> On 4/7/24 7:03 PM, Ming Lei wrote:
>>> sqe->flags is u8, and now we have used 7 bits, so take the last one for
>>> extending purpose.
>>>
>>> If bit7(IOSQE_HAS_EXT_FLAGS_BIT) is 1, it means this sqe carries ext flags
>>> from the last byte(.ext_flags), or bit23~bit16 of sqe->uring_cmd_flags for
>>> IORING_OP_URING_CMD.
>>>
>>> io_slot_flags() return value is converted to `ULL` because the affected bits
>>> are beyond 32bit now.
>>
>> If we're extending flags, which is something we arguably need to do at
>> some point, I think we should have them be generic and not spread out.
> 
> Sorry, maybe I don't get your idea, and the ext_flag itself is always
> initialized in io_init_req(), like normal sqe->flags, same with its
> usage.
> 
>> If uring_cmd needs specific flags and don't have them, then we should
>> add it just for that.
> 
> The only difference is that bit23~bit16 of sqe->uring_cmd_flags is
> borrowed for uring_cmd's ext flags, because sqe byte0~47 have been taken,
> and can't be reused for generic flag. If we want to use byte48~63, it has
> to be overlapped with uring_cmd's payload, and it is one generic sqe
> flag, which is applied on uring_cmd too.

Which is exactly the mess nobody would want to see. And I'd also
argue 8 extra bits is not enough anyway, otherwise the history will
repeat itself pretty soon

> That is the only way I thought of, or any other suggestion for extending sqe
> flags generically?

idea 1: just use the last bit. When we need another one it'd be time
to think about a long overdue SQE layout v2, this way we can try
to make flags u32 and clean up other problems.

idea 2: the group assembling flag can move into cmds. Very roughly:

io_cmd_init() {
	ublk_cmd_init();
}

ublk_cmd_init() {
	io_uring_start_grouping(ctx, cmd);
}

io_uring_start_grouping(ctx, cmd) {
	ctx->grouping = true;
	ctx->group_head = cmd->req;
}

submit_sqe() {
	if (ctx->grouping) {
		link_to_group(req, ctx->group_head);
		if (!(req->flags & REQ_F_LINK))
			ctx->grouping = false;
	}
}

-- 
Pavel Begunkov