Jan Kara <jack@xxxxxxx> writes:
> Hi!
>
> On Tue 22-10-24 12:33:27, Mohammed Anees wrote:
>> > Benchmarks, please. Look at what operations are done on this list.
>> > It's not at all obvious to me that what you've done here will improve
>> > performance of any operation.
>>
>> This patch aims to improve this operation in io_cancel() syscall,
>> currently this iterates through all the requests in the Linked list,
>> checking for a match, which could take a significant time if the
>> requests are high and once it finds one it deletes it. Using a hash
>> table will significant reduce the search time, which is what the comment
>> suggests as well.
>>
>> /* TODO: use a hash or array, this sucks. */
>> list_for_each_entry(kiocb, &ctx->active_reqs, ki_list) {
>> if (kiocb->ki_res.obj == obj) {
>> ret = kiocb->ki_cancel(&kiocb->rw);
>> list_del_init(&kiocb->ki_list);
>> break;
>> }
>> }
>>
>> I have tested this patch and believe it doesn’t affect the
>> other functions. As for the io_cancel() syscall, please let
>> me know exactly how you’d like me to test it so I can benchmark
>> it accordingly.
>
> Well, I'd say that calling io_cancel() isn't really frequent operation. Or
> are you aware of any workload that would be regularly doing that? Hence
> optimizing performance for such operation isn't going to bring much benefit
> to real users. On the other hand the additional complexity of handling
> hashtable for requests in flight (although it isn't big on its own) is
> going to impact everybody using AIO. Hence I agree with Matthew that
> changes like you propose are not a clear win when looking at the bigger
> picture and need good justification.
... and cancelation is only supported by usb gadgetfs. I'd say submit a
patch that gets rid of that todo so nobody else wastes time on it.
Cheers,
Jeff
