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: <20241031120423.5rq6uykywklkptkv@quack3>
Date: Thu, 31 Oct 2024 13:04:23 +0100
From: Jan Kara <jack@...e.cz>
To: Mohammed Anees <pvmohammedanees2003@...il.com>
Cc: willy@...radead.org, bcrl@...ck.org, brauner@...nel.org, jack@...e.cz,
	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

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.

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR