On 3/10/22 05:42, Mateusz Guzik wrote:
The branch main has been updated by mjg:
URL:
https://cgit.FreeBSD.org/src/commit/?id=c35ec1efdcb2978bc3b6a0098c2b412be8d33e39
commit c35ec1efdcb2978bc3b6a0098c2b412be8d33e39
Author: Mateusz Guzik <m...@freebsd.org>
AuthorDate: 2022-03-07 10:33:59 +0000
Commit: Mateusz Guzik <m...@freebsd.org>
CommitDate: 2022-03-10 09:41:50 +0000
vfs: [1/2] fix stalls in vnode reclaim by not requeieing from vnlru
Reported by: pho
---
sys/kern/vfs_subr.c | 55 +++++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 45 insertions(+), 10 deletions(-)
diff --git a/sys/kern/vfs_subr.c b/sys/kern/vfs_subr.c
index afafd02d92b9..436323873f7f 100644
--- a/sys/kern/vfs_subr.c
+++ b/sys/kern/vfs_subr.c
@@ -113,6 +113,8 @@ static void vn_seqc_init(struct vnode *);
static void vn_seqc_write_end_free(struct vnode *vp);
static void vgonel(struct vnode *);
static bool vhold_recycle_free(struct vnode *);
+static void vdropl_recycle(struct vnode *vp);
+static void vdrop_recycle(struct vnode *vp);
static void vfs_knllock(void *arg);
static void vfs_knlunlock(void *arg);
static void vfs_knl_assert_lock(void *arg, int what);
@@ -1207,11 +1209,11 @@ restart:
mtx_unlock(&vnode_list_mtx);
if (vn_start_write(vp, &mp, V_NOWAIT) != 0) {
- vdrop(vp);
+ vdrop_recycle(vp);
goto next_iter_unlocked;
}
if (VOP_LOCK(vp, LK_EXCLUSIVE|LK_NOWAIT) != 0) {
- vdrop(vp);
+ vdrop_recycle(vp);
vn_finished_write(mp);
goto next_iter_unlocked;
}
@@ -1222,14 +1224,14 @@ restart:
(vp->v_object != NULL && vp->v_object->handle == vp &&
vp->v_object->resident_page_count > trigger)) {
VOP_UNLOCK(vp);
- vdropl(vp);
+ vdropl_recycle(vp);
vn_finished_write(mp);
goto next_iter_unlocked;
}
counter_u64_add(recycles_count, 1);
vgonel(vp);
VOP_UNLOCK(vp);
- vdropl(vp);
+ vdropl_recycle(vp);
vn_finished_write(mp);
done++;
next_iter_unlocked:
@@ -1613,7 +1615,7 @@ vtryrecycle(struct vnode *vp)
CTR2(KTR_VFS,
"%s: impossible to recycle, vp %p lock is already held",
__func__, vp);
- vdrop(vp);
+ vdrop_recycle(vp);
return (EWOULDBLOCK);
}
/*
@@ -1624,7 +1626,7 @@ vtryrecycle(struct vnode *vp)
CTR2(KTR_VFS,
"%s: impossible to recycle, cannot start the write for %p",
__func__, vp);
- vdrop(vp);
+ vdrop_recycle(vp);
return (EBUSY);
}
/*
@@ -1636,7 +1638,7 @@ vtryrecycle(struct vnode *vp)
VI_LOCK(vp);
if (vp->v_usecount) {
VOP_UNLOCK(vp);
- vdropl(vp);
+ vdropl_recycle(vp);
vn_finished_write(vnmp);
CTR2(KTR_VFS,
"%s: impossible to recycle, %p is already referenced",
@@ -1648,7 +1650,7 @@ vtryrecycle(struct vnode *vp)
vgonel(vp);
}
VOP_UNLOCK(vp);
- vdropl(vp);
+ vdropl_recycle(vp);
vn_finished_write(vnmp);
return (0);
}
@@ -3598,8 +3600,8 @@ vdrop(struct vnode *vp)
vdropl(vp);
}
-void
-vdropl(struct vnode *vp)
+static void __always_inline
+vdropl_impl(struct vnode *vp, bool enqueue)
{
Hi,
It seems like enqueue is completely unused within the function. Was
there a hunk missing from the final commit?
Cheers,
Mitchell
ASSERT_VI_LOCKED(vp, __func__);
@@ -3627,6 +3629,39 @@ vdropl(struct vnode *vp)
vdbatch_enqueue(vp);
}
+void
+vdropl(struct vnode *vp)
+{
+
+ vdropl_impl(vp, true);
+}
+
+/*
+ * vdrop a vnode when recycling
+ *
+ * This is a special case routine only to be used when recycling, differs from
+ * regular vdrop by not requeieing the vnode on LRU.
+ *
+ * Consider a case where vtryrecycle continuously fails with all vnodes (due to
+ * e.g., frozen writes on the filesystem), filling the batch and causing it to
+ * be requeued. Then vnlru will end up revisiting the same vnodes. This is a
+ * loop which can last for as long as writes are frozen.
+ */
+static void
+vdropl_recycle(struct vnode *vp)
+{
+
+ vdropl_impl(vp, false);
+}
+
+static void
+vdrop_recycle(struct vnode *vp)
+{
+
+ VI_LOCK(vp);
+ vdropl_recycle(vp);
+}
+
/*
* Call VOP_INACTIVE on the vnode and manage the DOINGINACT and OWEINACT
* flags. DOINGINACT prevents us from recursing in calls to vinactive.
