Page MenuHomeFreeBSD

vfs: store precomputed namecache hash in the vnode
ClosedPublic

Authored by mjg on Aug 2 2020, 6:00 PM.

Details

Summary

This will be committed in two parts. Flag field resizing and renumbering prevents the vnode from growing with later addition of v_nchash and will be committed first.

This significantly speeds up path lookup, Cascade Lake doing access(2) on ufs on /usr/obj/usr/src/amd64.amd64/sys/GENERIC/vnode_if.c, ops/s:
before: 2535298
after: 2797621

That is +10%.

The reversed order of computation here does not seem to matter. I dumped the namecache populated with > 7 mln and recomputed chain placement with both methods, almost no differences.

I tried to lump this in with v_hash, but unfortunately that one has many testicles elsewhere and in particular is not constant for the lifetime of the vnode.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mjg requested review of this revision.Aug 2 2020, 6:00 PM
mjg created this revision.
mjg edited the summary of this revision. (Show Details)
mjg edited the summary of this revision. (Show Details)

I tried to lump this in with v_hash, but unfortunately that one has many testicles elsewhere and in particular is not constant for the lifetime of the vnode.

I am not sure I can fully parse this statement. Did you tried using v_hash as the init value for fnv ?
For UFS v_hash == ino, which cannot change for the whole lifetime of the vnode/inode, even rename does not change it.

sys/kern/vfs_cache.c
509 ↗(On Diff #75277)

You do not need the var.

I meant there are vfs hash consumers which change the value over the lifetime. Such a change would require rehashing namecache entries.

For example nfs:

hash = fnv_32_buf(nfhp->nfh_fh, nfhp->nfh_len,
     FNV1_32_INIT);
 onfhp = np->n_fhp;
 /*
  * Rehash node for new file handle.
  */
 vfs_hash_rehash(vp, hash);

I think vfs hash in the current will need to be revisited and it may be the new v_nchash field will disappear as a result of that, but in the meantime I don't think it's worth the hassle.

sys/kern/vfs_cache.c
509 ↗(On Diff #75277)

I'll remove it on commit

Is this the only example of such situation, where inode of the vnode changes without the vnode going through its lifecycle ?

In the nfs case, that you found, I believe what happens is that server returned different file handle (inode number) for some name, comparing to what we looked up some time ago. In this case, IMO the vnode should be recycled and new vnode allocated.

I fixed somewhat similar reuse of the vnode in UFS (but of course for i_mode == 0) some time ago.

This revision is now accepted and ready to land.Aug 2 2020, 7:45 PM

There is also msdosfs.

To make a more general statement, vfs hash is quite old and never got revisited in terms of its internals, it was just kept working over time. In particular it uses a global lock for all chains. Even ignoring the value of v_hash chaning, hashing consists of a taking whatever the fs considers a hash and adding a random number generated at mount time. Especially in presence of several mount points this may be hashing very poorly with many collisions. Moreover, the algorithm used by the namecache (FNV) needs to be revisited and it is unclear if a joint algorithm for uses is feasible.

As such I think revamping this would require several days and is not warranted right now.

This revision was automatically updated to reflect the committed changes.