Date: Wed, 28 Aug 2019 22:35:55 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r351584 - in head/sys: fs/nullfs fs/unionfs kern sys Message-ID: <CAGudoHEdw9%2BgF-=vpszw1uvZLdoCot=qLpjwD9X3PkrUOz_FOg@mail.gmail.com> In-Reply-To: <201908282034.x7SKYPZQ076957@repo.freebsd.org> References: <201908282034.x7SKYPZQ076957@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 8/28/19, Mateusz Guzik <mjg@freebsd.org> wrote: > Author: mjg > Date: Wed Aug 28 20:34:24 2019 > New Revision: 351584 > URL: https://svnweb.freebsd.org/changeset/base/351584 > > Log: > vfs: add VOP_NEED_INACTIVE > > vnode usecount drops to 0 all the time (e.g. for directories during path > lookup). > When that happens the kernel would always lock the exclusive lock for the > vnode > in order to call vinactive(). This blocks other threads who want to use > the vnode > for looukp. > > vinactive is very rarely needed and can be tested for without the vnode > lock held. > > This patch gives filesytems an opportunity to do it, sample total wait > time for > tmpfs over 500 minutes of poudriere -j 104: > > before: 557563641706 (lockmgr:tmpfs) > after: 46309603301 (lockmgr:tmpfs) > > Sponsored by: The FreeBSD Foundation > Differential Revision: https://reviews.freebsd.org/D21371 > Reviewed by: kib Tested by: pho > Modified: > head/sys/fs/nullfs/null_vnops.c > head/sys/fs/unionfs/union_vnops.c > head/sys/kern/vfs_default.c > head/sys/kern/vfs_subr.c > head/sys/kern/vnode_if.src > head/sys/sys/vnode.h > > Modified: head/sys/fs/nullfs/null_vnops.c > ============================================================================== > --- head/sys/fs/nullfs/null_vnops.c Wed Aug 28 20:23:49 2019 (r351583) > +++ head/sys/fs/nullfs/null_vnops.c Wed Aug 28 20:34:24 2019 (r351584) > @@ -907,6 +907,7 @@ struct vop_vector null_vnodeops = { > .vop_getattr = null_getattr, > .vop_getwritemount = null_getwritemount, > .vop_inactive = null_inactive, > + .vop_need_inactive = vop_stdneed_inactive, > .vop_islocked = vop_stdislocked, > .vop_lock1 = null_lock, > .vop_lookup = null_lookup, > > Modified: head/sys/fs/unionfs/union_vnops.c > ============================================================================== > --- head/sys/fs/unionfs/union_vnops.c Wed Aug 28 20:23:49 2019 (r351583) > +++ head/sys/fs/unionfs/union_vnops.c Wed Aug 28 20:34:24 2019 (r351584) > @@ -2523,6 +2523,7 @@ struct vop_vector unionfs_vnodeops = { > .vop_getextattr = unionfs_getextattr, > .vop_getwritemount = unionfs_getwritemount, > .vop_inactive = unionfs_inactive, > + .vop_need_inactive = vop_stdneed_inactive, > .vop_islocked = unionfs_islocked, > .vop_ioctl = unionfs_ioctl, > .vop_link = unionfs_link, > > Modified: head/sys/kern/vfs_default.c > ============================================================================== > --- head/sys/kern/vfs_default.c Wed Aug 28 20:23:49 2019 (r351583) > +++ head/sys/kern/vfs_default.c Wed Aug 28 20:34:24 2019 (r351584) > @@ -120,6 +120,7 @@ struct vop_vector default_vnodeops = { > .vop_getpages_async = vop_stdgetpages_async, > .vop_getwritemount = vop_stdgetwritemount, > .vop_inactive = VOP_NULL, > + .vop_need_inactive = vop_stdneed_inactive, > .vop_ioctl = vop_stdioctl, > .vop_kqfilter = vop_stdkqfilter, > .vop_islocked = vop_stdislocked, > @@ -1155,6 +1156,13 @@ vop_stdadd_writecount(struct vop_add_writecount_args > * > } > VI_UNLOCK(vp); > return (error); > +} > + > +int > +vop_stdneed_inactive(struct vop_need_inactive_args *ap) > +{ > + > + return (1); > } > > static int > > Modified: head/sys/kern/vfs_subr.c > ============================================================================== > --- head/sys/kern/vfs_subr.c Wed Aug 28 20:23:49 2019 (r351583) > +++ head/sys/kern/vfs_subr.c Wed Aug 28 20:34:24 2019 (r351584) > @@ -2891,6 +2891,21 @@ vputx(struct vnode *vp, int func) > CTR2(KTR_VFS, "%s: return vnode %p to the freelist", __func__, vp); > > /* > + * Check if the fs wants to perform inactive processing. Note we > + * may be only holding the interlock, in which case it is possible > + * someone else called vgone on the vnode and ->v_data is now NULL. > + * Since vgone performs inactive on its own there is nothing to do > + * here but to drop our hold count. > + */ > + if (__predict_false(vp->v_iflag & VI_DOOMED) || > + VOP_NEED_INACTIVE(vp) == 0) { > + if (func == VPUTX_VPUT) > + VOP_UNLOCK(vp, 0); > + vdropl(vp); > + return; > + } > + > + /* > * We must call VOP_INACTIVE with the node locked. Mark > * as VI_DOINGINACT to avoid recursion. > */ > @@ -4353,6 +4368,7 @@ static struct vop_vector sync_vnodeops = { > .vop_close = sync_close, /* close */ > .vop_fsync = sync_fsync, /* fsync */ > .vop_inactive = sync_inactive, /* inactive */ > + .vop_need_inactive = vop_stdneed_inactive, /* need_inactive */ > .vop_reclaim = sync_reclaim, /* reclaim */ > .vop_lock1 = vop_stdlock, /* lock */ > .vop_unlock = vop_stdunlock, /* unlock */ > @@ -4514,6 +4530,20 @@ sync_reclaim(struct vop_reclaim_args *ap) > return (0); > } > > +int > +vn_need_pageq_flush(struct vnode *vp) > +{ > + struct vm_object *obj; > + int need; > + > + MPASS(mtx_owned(VI_MTX(vp))); > + need = 0; > + if ((obj = vp->v_object) != NULL && (vp->v_vflag & VV_NOSYNC) == 0 && > + (obj->flags & OBJ_MIGHTBEDIRTY) != 0) > + need = 1; > + return (need); > +} > + > /* > * Check if vnode represents a disk device > */ > @@ -4893,6 +4923,22 @@ vop_unlock_post(void *ap, int rc) > > if (a->a_flags & LK_INTERLOCK) > ASSERT_VI_UNLOCKED(a->a_vp, "VOP_UNLOCK"); > +} > + > +void > +vop_need_inactive_pre(void *ap) > +{ > + struct vop_need_inactive_args *a = ap; > + > + ASSERT_VI_LOCKED(a->a_vp, "VOP_NEED_INACTIVE"); > +} > + > +void > +vop_need_inactive_post(void *ap, int rc) > +{ > + struct vop_need_inactive_args *a = ap; > + > + ASSERT_VI_LOCKED(a->a_vp, "VOP_NEED_INACTIVE"); > } > #endif > > > Modified: head/sys/kern/vnode_if.src > ============================================================================== > --- head/sys/kern/vnode_if.src Wed Aug 28 20:23:49 2019 (r351583) > +++ head/sys/kern/vnode_if.src Wed Aug 28 20:34:24 2019 (r351584) > @@ -358,6 +358,12 @@ vop_inactive { > IN struct thread *td; > }; > > +%! need_inactive pre vop_need_inactive_pre > +%! need_inactive post vop_need_inactive_post > + > +vop_need_inactive { > + IN struct vnode *vp; > +}; > > %% reclaim vp E E E > %! reclaim post vop_reclaim_post > > Modified: head/sys/sys/vnode.h > ============================================================================== > --- head/sys/sys/vnode.h Wed Aug 28 20:23:49 2019 (r351583) > +++ head/sys/sys/vnode.h Wed Aug 28 20:34:24 2019 (r351584) > @@ -682,6 +682,7 @@ int vn_generic_copy_file_range(struct vnode *invp, off > struct vnode *outvp, off_t *outoffp, size_t *lenp, > unsigned int flags, struct ucred *incred, struct ucred *outcred, > struct thread *fsize_td); > +int vn_need_pageq_flush(struct vnode *vp); > int vn_isdisk(struct vnode *vp, int *errp); > int _vn_lock(struct vnode *vp, int flags, char *file, int line); > #define vn_lock(vp, flags) _vn_lock(vp, flags, __FILE__, __LINE__) > @@ -753,6 +754,7 @@ int vop_stdfsync(struct vop_fsync_args *); > int vop_stdgetwritemount(struct vop_getwritemount_args *); > int vop_stdgetpages(struct vop_getpages_args *); > int vop_stdinactive(struct vop_inactive_args *); > +int vop_stdneed_inactive(struct vop_need_inactive_args *); > int vop_stdislocked(struct vop_islocked_args *); > int vop_stdkqfilter(struct vop_kqfilter_args *); > int vop_stdlock(struct vop_lock1_args *); > @@ -813,12 +815,16 @@ void vop_lock_pre(void *a); > void vop_lock_post(void *a, int rc); > void vop_unlock_pre(void *a); > void vop_unlock_post(void *a, int rc); > +void vop_need_inactive_pre(void *a); > +void vop_need_inactive_post(void *a, int rc); > #else > #define vop_strategy_pre(x) do { } while (0) > #define vop_lock_pre(x) do { } while (0) > #define vop_lock_post(x, y) do { } while (0) > #define vop_unlock_pre(x) do { } while (0) > #define vop_unlock_post(x, y) do { } while (0) > +#define vop_need_inactive_pre(x) do { } while (0) > +#define vop_need_inactive_post(x, y) do { } while (0) > #endif > > void vop_rename_fail(struct vop_rename_args *ap); > > -- Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGudoHEdw9%2BgF-=vpszw1uvZLdoCot=qLpjwD9X3PkrUOz_FOg>