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?
--
Jens Axboe
