Date: Tue, 7 Oct 2025 00:30:45 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: Konstantin Belousov <kostikbel@gmail.com> Cc: Mateusz Guzik <mjg@freebsd.org>, src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 84f981ba57e7 - main - nullfs: shrink null_node to 32 bytes Message-ID: <CAGudoHEWXDwcsXcz=NtHBWFiO48DvRBNCEQFLHXJs52TpbnJWA@mail.gmail.com> In-Reply-To: <aORBvZz9pEKf9bns@kib.kiev.ua> References: <202510061823.596IN7oZ084715@gitrepo.freebsd.org> <aORBvZz9pEKf9bns@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Oct 7, 2025 at 12:25=E2=80=AFAM Konstantin Belousov <kostikbel@gmai= l.com> wrote: > > On Mon, Oct 06, 2025 at 06:23:07PM +0000, Mateusz Guzik wrote: > > The branch main has been updated by mjg: > > > > URL: https://cgit.FreeBSD.org/src/commit/?id=3D84f981ba57e77bd3c3d0fbf1= 469ce51bfd132a6b > > > > commit 84f981ba57e77bd3c3d0fbf1469ce51bfd132a6b > > Author: Mateusz Guzik <mjg@FreeBSD.org> > > AuthorDate: 2025-10-06 17:59:17 +0000 > > Commit: Mateusz Guzik <mjg@FreeBSD.org> > > CommitDate: 2025-10-06 18:23:01 +0000 > > > > nullfs: shrink null_node to 32 bytes > > --- > > sys/fs/nullfs/null.h | 2 +- > > sys/fs/nullfs/null_subr.c | 12 +++++++----- > > 2 files changed, 8 insertions(+), 6 deletions(-) > > > > diff --git a/sys/fs/nullfs/null.h b/sys/fs/nullfs/null.h > > index aa7a689bec34..ad3f7779e108 100644 > > --- a/sys/fs/nullfs/null.h > > +++ b/sys/fs/nullfs/null.h > > @@ -53,7 +53,7 @@ struct null_mount { > > * A cache of vnode references > > */ > > struct null_node { > > - CK_LIST_ENTRY(null_node) null_hash; /* Hash list */ > > + CK_SLIST_ENTRY(null_node) null_hash; /* Hash list */ > > struct vnode *null_lowervp; /* VREFed once */ > > struct vnode *null_vnode; /* Back pointer */ > > u_int null_flags; > > diff --git a/sys/fs/nullfs/null_subr.c b/sys/fs/nullfs/null_subr.c > > index ad8cd08279cc..bb0ff9966dfd 100644 > > --- a/sys/fs/nullfs/null_subr.c > > +++ b/sys/fs/nullfs/null_subr.c > > @@ -59,7 +59,7 @@ VFS_SMR_DECLARE; > > > > #define NULL_NHASH(vp) (&null_node_hashtbl[vfs_hash_index(vp) & n= ull_hash_mask]) > > > > -static CK_LIST_HEAD(null_node_hashhead, null_node) *null_node_hashtbl; > > +static CK_SLIST_HEAD(null_node_hashhead, null_node) *null_node_hashtbl= ; > > static struct rwlock null_hash_lock; > > static u_long null_hash_mask; > > > > @@ -116,7 +116,7 @@ null_hashget_locked(struct mount *mp, struct vnode = *lowervp) > > * reference count (but NOT the lower vnode's VREF counter). > > */ > > hd =3D NULL_NHASH(lowervp); > > - CK_LIST_FOREACH(a, hd, null_hash) { > > + CK_SLIST_FOREACH(a, hd, null_hash) { > > if (a->null_lowervp !=3D lowervp) > > continue; > > /* > > @@ -148,7 +148,7 @@ null_hashget(struct mount *mp, struct vnode *lowerv= p) > > > > vfs_smr_enter(); > > hd =3D NULL_NHASH(lowervp); > > - CK_LIST_FOREACH(a, hd, null_hash) { > > + CK_SLIST_FOREACH(a, hd, null_hash) { > > if (a->null_lowervp !=3D lowervp) > > continue; > > /* > > @@ -189,7 +189,7 @@ null_hashins(struct mount *mp, struct null_node *xp= ) > > } > > } > > #endif > > - CK_LIST_INSERT_HEAD(hd, xp, null_hash); > > + CK_SLIST_INSERT_HEAD(hd, xp, null_hash); > > } > > > > static void > > @@ -305,9 +305,11 @@ null_nodeget(struct mount *mp, struct vnode *lower= vp, struct vnode **vpp) > > void > > null_hashrem(struct null_node *xp) > > { > > + struct null_node_hashhead *hd; > > > > + hd =3D NULL_NHASH(xp->null_lowervp); > > rw_wlock(&null_hash_lock); > > - CK_LIST_REMOVE(xp, null_hash); > > + CK_SLIST_REMOVE(hd, xp, null_node, null_hash); > This changes O(1) removal into O(N), for N being the size of the hash > chain length for specific hash. I.e. it is on par with the lookup. > > Why it is fine? Why it was not mentioned and explained in the commit > message? > For the hash to be at all useful the chains are supposed to be short. Or to put it differently, a sensibly sized hash with a sensible hashing function makes it automatically fine. This was worthwhile to do because the original size of 40 bytes is a very poor fit for the allocator. fwiw the namecache already works this way for few years. the regular vnode hash should probably get the same treatment, saving 8 bytes off of struct vnode
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGudoHEWXDwcsXcz=NtHBWFiO48DvRBNCEQFLHXJs52TpbnJWA>