Re: svn commit: r366997 - head/sys/kern
In message <20201024172452.gd2...@kib.kiev.ua>, Konstantin Belousov writes: > On Sat, Oct 24, 2020 at 01:30:37PM +, Mateusz Guzik wrote: > > Author: mjg > > Date: Sat Oct 24 13:30:37 2020 > > New Revision: 366997 > > URL: https://svnweb.freebsd.org/changeset/base/366997 > > > > Log: > > vfs: fix a race where reclaim vholds freed vnodes > A description of the race in the commit message would be respectful to > other readers of the code, so that we do not need to reverse-eng the > change to understand what and why was fixed. +1 -- Cheers, Cy Schubert FreeBSD UNIX: Web: https://FreeBSD.org NTP: Web: https://nwtime.org The need of the many outweighs the greed of the few. ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r366997 - head/sys/kern
On Sat, Oct 24, 2020 at 01:30:37PM +, Mateusz Guzik wrote:
> Author: mjg
> Date: Sat Oct 24 13:30:37 2020
> New Revision: 366997
> URL: https://svnweb.freebsd.org/changeset/base/366997
>
> Log:
> vfs: fix a race where reclaim vholds freed vnodes
A description of the race in the commit message would be respectful to
other readers of the code, so that we do not need to reverse-eng the
change to understand what and why was fixed.
>
> Reported by:pho
> Tested by: pho (previous version)
> Fixes: r366974 ("vfs: stop taking the interlock in vnode reclaim")
>
> Modified:
> head/sys/kern/vfs_subr.c
>
> Modified: head/sys/kern/vfs_subr.c
> ==
> --- head/sys/kern/vfs_subr.c Sat Oct 24 13:16:10 2020(r366996)
> +++ head/sys/kern/vfs_subr.c Sat Oct 24 13:30:37 2020(r366997)
> @@ -109,6 +109,7 @@ static void syncer_shutdown(void *arg, int howto);
> static int vtryrecycle(struct vnode *vp);
> static void v_init_counters(struct vnode *);
> static void vgonel(struct vnode *);
> +static bool vhold_recycle(struct vnode *);
> static void vfs_knllock(void *arg);
> static void vfs_knlunlock(void *arg);
> static void vfs_knl_assert_locked(void *arg);
> @@ -1126,7 +1127,8 @@ restart:
> goto next_iter;
> }
>
> - vhold(vp);
> + if (!vhold_recycle(vp))
> + goto next_iter;
> TAILQ_REMOVE(_list, mvp, v_vnodelist);
> TAILQ_INSERT_AFTER(_list, vp, mvp, v_vnodelist);
> mtx_unlock(_list_mtx);
> @@ -1231,7 +1233,8 @@ restart:
> if (__predict_false(vp->v_type == VBAD || vp->v_type == VNON)) {
> continue;
> }
> - vhold(vp);
> + if (!vhold_recycle(vp))
> + continue;
> count--;
> mtx_unlock(_list_mtx);
> vtryrecycle(vp);
> @@ -3248,13 +3251,11 @@ vholdnz(struct vnode *vp)
> * However, while this is more performant, it hinders debugging by
> eliminating
> * the previously mentioned invariant.
> */
> -bool
> -vhold_smr(struct vnode *vp)
> +static bool __always_inline
> +_vhold_cond(struct vnode *vp)
> {
> int count;
>
> - VFS_SMR_ASSERT_ENTERED();
> -
> count = atomic_load_int(>v_holdcnt);
> for (;;) {
> if (count & VHOLD_NO_SMR) {
> @@ -3270,6 +3271,28 @@ vhold_smr(struct vnode *vp)
> return (true);
> }
> }
> +}
> +
> +bool
> +vhold_smr(struct vnode *vp)
> +{
> +
> + VFS_SMR_ASSERT_ENTERED();
> + return (_vhold_cond(vp));
> +}
> +
> +/*
> + * Special case for vnode recycling.
> + *
> + * Vnodes are present on the global list until UMA takes them out.
> + * Attempts to recycle only need the relevant lock and have no use for SMR.
> + */
> +static bool
> +vhold_recycle(struct vnode *vp)
> +{
> +
> + mtx_assert(_list_mtx, MA_OWNED);
> + return (_vhold_cond(vp));
> }
>
> static void __noinline
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
svn commit: r366997 - head/sys/kern
Author: mjg
Date: Sat Oct 24 13:30:37 2020
New Revision: 366997
URL: https://svnweb.freebsd.org/changeset/base/366997
Log:
vfs: fix a race where reclaim vholds freed vnodes
Reported by: pho
Tested by:pho (previous version)
Fixes:r366974 ("vfs: stop taking the interlock in vnode reclaim")
Modified:
head/sys/kern/vfs_subr.c
Modified: head/sys/kern/vfs_subr.c
==
--- head/sys/kern/vfs_subr.cSat Oct 24 13:16:10 2020(r366996)
+++ head/sys/kern/vfs_subr.cSat Oct 24 13:30:37 2020(r366997)
@@ -109,6 +109,7 @@ static void syncer_shutdown(void *arg, int howto);
static int vtryrecycle(struct vnode *vp);
static voidv_init_counters(struct vnode *);
static voidvgonel(struct vnode *);
+static boolvhold_recycle(struct vnode *);
static voidvfs_knllock(void *arg);
static voidvfs_knlunlock(void *arg);
static voidvfs_knl_assert_locked(void *arg);
@@ -1126,7 +1127,8 @@ restart:
goto next_iter;
}
- vhold(vp);
+ if (!vhold_recycle(vp))
+ goto next_iter;
TAILQ_REMOVE(_list, mvp, v_vnodelist);
TAILQ_INSERT_AFTER(_list, vp, mvp, v_vnodelist);
mtx_unlock(_list_mtx);
@@ -1231,7 +1233,8 @@ restart:
if (__predict_false(vp->v_type == VBAD || vp->v_type == VNON)) {
continue;
}
- vhold(vp);
+ if (!vhold_recycle(vp))
+ continue;
count--;
mtx_unlock(_list_mtx);
vtryrecycle(vp);
@@ -3248,13 +3251,11 @@ vholdnz(struct vnode *vp)
* However, while this is more performant, it hinders debugging by eliminating
* the previously mentioned invariant.
*/
-bool
-vhold_smr(struct vnode *vp)
+static bool __always_inline
+_vhold_cond(struct vnode *vp)
{
int count;
- VFS_SMR_ASSERT_ENTERED();
-
count = atomic_load_int(>v_holdcnt);
for (;;) {
if (count & VHOLD_NO_SMR) {
@@ -3270,6 +3271,28 @@ vhold_smr(struct vnode *vp)
return (true);
}
}
+}
+
+bool
+vhold_smr(struct vnode *vp)
+{
+
+ VFS_SMR_ASSERT_ENTERED();
+ return (_vhold_cond(vp));
+}
+
+/*
+ * Special case for vnode recycling.
+ *
+ * Vnodes are present on the global list until UMA takes them out.
+ * Attempts to recycle only need the relevant lock and have no use for SMR.
+ */
+static bool
+vhold_recycle(struct vnode *vp)
+{
+
+ mtx_assert(_list_mtx, MA_OWNED);
+ return (_vhold_cond(vp));
}
static void __noinline
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
