On Tue, Aug 10, 2021 at 10:38:32AM +0200, Hanna Reitz wrote: > On 09.08.21 21:08, Vivek Goyal wrote: > > On Fri, Jul 30, 2021 at 05:01:34PM +0200, Max Reitz wrote: > > > lo_find() right now takes two lookup keys for two maps, namely the file > > > handle for inodes_by_handle and the statx information for inodes_by_ids. > > > However, we only need the statx information if looking up the inode by > > > the file handle failed. > > > > > > There are two callers of lo_find(): The first one, lo_do_lookup(), has > > > both keys anyway, so passing them does not incur any additional cost. > > > The second one, lookup_name(), though, needs to explicitly invoke > > > name_to_handle_at() (through get_file_handle()) and statx() (through > > > do_statx()). We need to try to get a file handle as the primary key, so > > > we cannot get rid of get_file_handle(), but we only need the statx > > > information if looking up an inode by handle failed; so we can defer > > > that until the lookup has indeed failed. > > So IIUC, this patch seems to be all about avoiding do_statx() > > call in lookup_name() if file handle could be successfully > > generated. > > > > So can't we just not modify lookup_name() to not call statx() > > if file handle could be generated. And also modfiy lo_find() > > to use st/mnt_id only if fhandle==NULL. > > > > That probably is much simpler change as compared to passing function > > pointers around. > > Definitely, but I don’t know whether it’s correct.
What problem do you see from correctness point of view. > > Or, we can just drop this patch and say that we don’t need to over-optimize > C virtiofsd. Rust version is used by very few people, while C version is in production. So I will definitely optimize C version. Once rust version is widely available and available in product, then we can start paying less attention to C version, IMHO. Vivek
