Re: [PATCH RFC] io_uring: extend io_uring_sqe flags bits

[Jens Axboe <axboe@kernel.dk>]

On 11/1/24 10:55 AM, Pavel Begunkov wrote:
> On 11/1/24 14:42, Jens Axboe wrote:
>> On 11/1/24 8:34 AM, Ming Lei wrote:
>>> On Fri, Nov 01, 2024 at 07:59:38AM -0600, Jens Axboe wrote:
>>>> On 10/31/24 8:42 PM, Ming Lei wrote:
>>>>> On Fri, Nov 01, 2024 at 10:12:25AM +0800, Ming Lei wrote:
>>>>>> On Thu, Oct 31, 2024 at 03:22:18PM -0600, Jens Axboe wrote:
>>>>>>> In hindsight everything is clearer, but it probably should've been known
>>>>>>> that 8 bits of ->flags would run out sooner than later. Rather than
>>>>>>> gobble up the last bit for a random use case, add a bit that controls
>>>>>>> whether or not ->personality is used as a flags2 argument. If that is
>>>>>>> the case, then there's a new IOSQE2_PERSONALITY flag that tells io_uring
>>>>>>> which personality field to read.
>>>>>>>
>>>>>>> While this isn't the prettiest, it does allow extending with 15 extra
>>>>>>> flags, and retains being able to use personality with any kind of
>>>>>>> command. The exception is uring cmd, where personality2 will overlap
>>>>>>> with the space set aside for SQE128. If they really need that, then that
>>>>>>
>>>>>> The space is the 1st `short` for uring_cmd, instead of SQE128 only.
>>>>>>
>>>>>> Also it is overlapped with ->optval and ->addr3, so just wondering why not
>>>>>> use ->__pad2?
>>>>>>
>>>>>> Another ways is to use __pad2 for sqe2_flags for non-uring_cmd, and for
>>>>>> uring_cmd, use its top 16 as sqe2_flags, this way does work, but it is
>>>>>> just a bit ugly to use.
>>>>>
>>>>> Also IOSQE2_PERSONALITY doesn't have to be per-SQE, and it can be one
>>>>> feature of IORING_FEAT_IOSQE2_PERSONALITY, that is why I thought it is
>>>>> fine to take the 7th bit as SQE_GROUP now.
>>>>
>>>> Not sure I follow your thinking there, can you expand?
>>>
>>> It could be one io_uring setup flag, such as
>>> IORING_SETUP_IOSQE2_PERSONALITY.
>>>
>>> If this flag is set, take __pad2 as sqe2_flags, otherwise use current
>>> way, so it doesn't have to take bit7 of sqe_flags for this purpose.
>>
>> Would probably have to be a IORING_SETUP_IOSQE2_FLAGS or something in
>> general. And while that could work, not a huge fan of that. I think we
>> should retain that for when a v2 of the sqe is done, to coordinate which
>> version to use.
> 
> A setup flag over an sqe flag for marking IMHO would be a _much_
> better approach. It doesn't take an SQE bit for nothing, you can
> parse and process it in the slow setup path, enable static keys
> for the hot path and so on.

Alright, if both of you like that, then let me rework the flags patch to
do that instead. If we go that route, then we can just defer doing the
actual setup flag until we need a new bit beyond filling the last
sqe->flags bit with GROUP.

-- 
Jens Axboe