> On 8 Jul 2020, at 17.06, Jens Axboe <axboe@xxxxxxxxx> wrote:
>
> On 7/8/20 9:02 AM, Matthew Wilcox wrote:
>>> On Wed, Jul 08, 2020 at 08:59:50AM -0600, Jens Axboe wrote:
>>> On 7/8/20 8:58 AM, Matthew Wilcox wrote:
>>>> On Wed, Jul 08, 2020 at 08:54:07AM -0600, Jens Axboe wrote:
>>>>> On 7/8/20 6:58 AM, Kanchan Joshi wrote:
>>>>>>>> +#define IOCB_NO_CMPL (15 << 28)
>>>>>>>>
>>>>>>>> struct kiocb {
>>>>>>>> [...]
>>>>>>>> - void (*ki_complete)(struct kiocb *iocb, long ret, long ret2);
>>>>>>>> + loff_t __user *ki_uposp;
>>>>>>>> - int ki_flags;
>>>>>>>> + unsigned int ki_flags;
>>>>>>>>
>>>>>>>> +typedef void ki_cmpl(struct kiocb *, long ret, long ret2);
>>>>>>>> +static ki_cmpl * const ki_cmpls[15];
>>>>>>>>
>>>>>>>> +void ki_complete(struct kiocb *iocb, long ret, long ret2)
>>>>>>>> +{
>>>>>>>> + unsigned int id = iocb->ki_flags >> 28;
>>>>>>>> +
>>>>>>>> + if (id < 15)
>>>>>>>> + ki_cmpls[id](iocb, ret, ret2);
>>>>>>>> +}
>>>>>>>>
>>>>>>>> +int kiocb_cmpl_register(void (*cb)(struct kiocb *, long, long))
>>>>>>>> +{
>>>>>>>> + for (i = 0; i < 15; i++) {
>>>>>>>> + if (ki_cmpls[id])
>>>>>>>> + continue;
>>>>>>>> + ki_cmpls[id] = cb;
>>>>>>>> + return id;
>>>>>>>> + }
>>>>>>>> + WARN();
>>>>>>>> + return -1;
>>>>>>>> +}
>>>>>>>
>>>>>>> That could work, we don't really have a lot of different completion
>>>>>>> types in the kernel.
>>>>>>
>>>>>> Thanks, this looks sorted.
>>>>>
>>>>> Not really, someone still needs to do that work. I took a quick look, and
>>>>> most of it looks straight forward. The only potential complication is
>>>>> ocfs2, which does a swap of the completion for the kiocb. That would just
>>>>> turn into an upper flag swap. And potential sync kiocb with NULL
>>>>> ki_complete. The latter should be fine, I think we just need to reserve
>>>>> completion nr 0 for being that.
>>>>
>>>> I was reserving completion 15 for that ;-)
>>>>
>>>> +#define IOCB_NO_CMPL (15 << 28)
>>>> ...
>>>> + if (id < 15)
>>>> + ki_cmpls[id](iocb, ret, ret2);
>>>>
>>>> Saves us one pointer in the array ...
>>>
>>> That works. Are you going to turn this into an actual series of patches,
>>> adding the functionality and converting users?
>>
>> I was under the impression Kanchan was going to do that, but I can run it
>> off quickly ...
>
> I just wanted to get clarification there, because to me it sounded like
> you expected Kanchan to do it, and Kanchan assuming it "was sorted". I'd
> consider that a prerequisite for the append series as far as io_uring is
> concerned, hence _someone_ needs to actually do it ;-)
>
I believe Kanchan meant that now the trade-off we were asking to clear out is sorted.
We will send a new version shortly for the current functionality - we can see what we are missing on when the uring interface is clear.
We really want this to be stable as a lot of other things are depending on this (e.g., fio patches)
Javier
