linux-kernel - Re: [PATCH] fs: aio: Transition from Linked List to Hash Table for Active Request Management in AIO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <x491pzwtogw.fsf@segfault.usersys.redhat.com>
Date: Thu, 31 Oct 2024 09:02:23 -0400
From: Jeff Moyer <jmoyer@...hat.com>
To: Jan Kara <jack@...e.cz>
Cc: Mohammed Anees <pvmohammedanees2003@...il.com>,  willy@...radead.org,
  bcrl@...ck.org,  brauner@...nel.org,  linux-aio@...ck.org,
  linux-fsdevel@...r.kernel.org,  linux-kernel@...r.kernel.org,
  viro@...iv.linux.org.uk
Subject: Re: [PATCH] fs: aio: Transition from Linked List to Hash Table for
 Active Request Management in AIO

Jan Kara <jack@...e.cz> 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