Date: Tue, 7 Oct 2025 00:31:37 +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: <CAGudoHEf%2BVzj_ZTv8otuz_FCh3CfO_tok=%2BbGS=__u5qnBkuJA@mail.gmail.com> In-Reply-To: <CAGudoHEWXDwcsXcz=NtHBWFiO48DvRBNCEQFLHXJs52TpbnJWA@mail.gmail.com> References: <202510061823.596IN7oZ084715@gitrepo.freebsd.org> <aORBvZz9pEKf9bns@kib.kiev.ua> <CAGudoHEWXDwcsXcz=NtHBWFiO48DvRBNCEQFLHXJs52TpbnJWA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Oct 7, 2025 at 12:30=E2=80=AFAM Mateusz Guzik <mjguzik@gmail.com> w= rote: > > On Tue, Oct 7, 2025 at 12:25=E2=80=AFAM Konstantin Belousov <kostikbel@gm= ail.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=3D84f981ba57e77bd3c3d0fb= f1469ce51bfd132a6b > > > > > > 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) &= null_hash_mask]) > > > > > > -static CK_LIST_HEAD(null_node_hashhead, null_node) *null_node_hashtb= l; > > > +static CK_SLIST_HEAD(null_node_hashhead, null_node) *null_node_hasht= bl; > > > static struct rwlock null_hash_lock; > > > static u_long null_hash_mask; > > > > > > @@ -116,7 +116,7 @@ null_hashget_locked(struct mount *mp, struct vnod= e *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 *lowe= rvp) > > > > > > 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 *low= ervp, 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 I can concede it would make sense to include this in the commit message.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGudoHEf%2BVzj_ZTv8otuz_FCh3CfO_tok=%2BbGS=__u5qnBkuJA>