Module Name: src
Committed By: riastradh
Date: Sun Feb 23 08:40:37 UTC 2020
Modified Files:
src/sys/ufs/lfs: lfs_extern.h lfs_segment.c lfs_subr.c
Log Message:
Break deadlock in PR kern/52301.
The lock order is lfs_writer -> lfs_seglock. The problem in 52301 is
that lfs_segwrite violates this lock order by sometimes doing
lfs_seglock -> lfs_writer, either (a) when doing a checkpoint or (b),
opportunistically, when there are no dirops pending. Both cases can
deadlock, because dirops sometimes take the seglock (lfs_truncate,
lfs_valloc, lfs_vfree):
(a) There may be dirops pending, and they may be waiting for the
seglock, so we can't wait for them to complete while holding the
seglock.
(b) The test for fs->lfs_dirops == 0 happens unlocked, and the state
may change by the time lfs_writer_enter acquires lfs_lock.
To resolve this in each case:
(a) Do lfs_writer_enter before lfs_seglock, since we will need it
unconditionally anyway. The worst performance impact of this should
be that some dirops get delayed a little bit.
(b) Create a new lfs_writer_tryenter to use at this point so that the
test for fs->lfs_dirops == 0 and the acquisition of lfs_writer happen
atomically under lfs_lock.
To generate a diff of this commit:
cvs rdiff -u -r1.115 -r1.116 src/sys/ufs/lfs/lfs_extern.h
cvs rdiff -u -r1.284 -r1.285 src/sys/ufs/lfs/lfs_segment.c
cvs rdiff -u -r1.98 -r1.99 src/sys/ufs/lfs/lfs_subr.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/ufs/lfs/lfs_extern.h
diff -u src/sys/ufs/lfs/lfs_extern.h:1.115 src/sys/ufs/lfs/lfs_extern.h:1.116
--- src/sys/ufs/lfs/lfs_extern.h:1.115 Tue Feb 18 20:23:17 2020
+++ src/sys/ufs/lfs/lfs_extern.h Sun Feb 23 08:40:37 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: lfs_extern.h,v 1.115 2020/02/18 20:23:17 chs Exp $ */
+/* $NetBSD: lfs_extern.h,v 1.116 2020/02/23 08:40:37 riastradh Exp $ */
/*-
* Copyright (c) 1999, 2000, 2001, 2002, 2003 The NetBSD Foundation, Inc.
@@ -211,6 +211,7 @@ int lfs_seglock(struct lfs *, unsigned l
void lfs_segunlock(struct lfs *);
void lfs_segunlock_relock(struct lfs *);
int lfs_writer_enter(struct lfs *, const char *);
+int lfs_writer_tryenter(struct lfs *);
void lfs_writer_leave(struct lfs *);
void lfs_wakeup_cleaner(struct lfs *);
Index: src/sys/ufs/lfs/lfs_segment.c
diff -u src/sys/ufs/lfs/lfs_segment.c:1.284 src/sys/ufs/lfs/lfs_segment.c:1.285
--- src/sys/ufs/lfs/lfs_segment.c:1.284 Sun Feb 23 08:40:08 2020
+++ src/sys/ufs/lfs/lfs_segment.c Sun Feb 23 08:40:37 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: lfs_segment.c,v 1.284 2020/02/23 08:40:08 riastradh Exp $ */
+/* $NetBSD: lfs_segment.c,v 1.285 2020/02/23 08:40:37 riastradh Exp $ */
/*-
* Copyright (c) 1999, 2000, 2001, 2002, 2003 The NetBSD Foundation, Inc.
@@ -60,7 +60,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: lfs_segment.c,v 1.284 2020/02/23 08:40:08 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: lfs_segment.c,v 1.285 2020/02/23 08:40:37 riastradh Exp $");
#ifdef DEBUG
# define vndebug(vp, str) do { \
@@ -621,6 +621,15 @@ lfs_segwrite(struct mount *mp, int flags
*/
do_ckp = LFS_SHOULD_CHECKPOINT(fs, flags);
+ /*
+ * If we know we're gonna need the writer lock, take it now to
+ * preserve the lock order lfs_writer -> lfs_seglock.
+ */
+ if (do_ckp) {
+ lfs_writer_enter(fs, "ckpwriter");
+ writer_set = 1;
+ }
+
/* We can't do a partial write and checkpoint at the same time. */
if (do_ckp)
flags &= ~SEGM_SINGLE;
@@ -650,11 +659,10 @@ lfs_segwrite(struct mount *mp, int flags
break;
}
- if (do_ckp || fs->lfs_dirops == 0) {
- if (!writer_set) {
- lfs_writer_enter(fs, "lfs writer");
- writer_set = 1;
- }
+ if (do_ckp ||
+ (writer_set = lfs_writer_tryenter(fs)) != 0) {
+ KASSERT(writer_set);
+ KASSERT(fs->lfs_writer);
error = lfs_writevnodes(fs, mp, sp, VN_DIROP);
if (um_error == 0)
um_error = error;
Index: src/sys/ufs/lfs/lfs_subr.c
diff -u src/sys/ufs/lfs/lfs_subr.c:1.98 src/sys/ufs/lfs/lfs_subr.c:1.99
--- src/sys/ufs/lfs/lfs_subr.c:1.98 Sun Feb 23 08:38:58 2020
+++ src/sys/ufs/lfs/lfs_subr.c Sun Feb 23 08:40:37 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: lfs_subr.c,v 1.98 2020/02/23 08:38:58 riastradh Exp $ */
+/* $NetBSD: lfs_subr.c,v 1.99 2020/02/23 08:40:37 riastradh Exp $ */
/*-
* Copyright (c) 1999, 2000, 2001, 2002, 2003 The NetBSD Foundation, Inc.
@@ -60,7 +60,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: lfs_subr.c,v 1.98 2020/02/23 08:38:58 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: lfs_subr.c,v 1.99 2020/02/23 08:40:37 riastradh Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -575,7 +575,7 @@ lfs_writer_enter(struct lfs *fs, const c
{
int error = 0;
- ASSERT_MAYBE_SEGLOCK(fs);
+ ASSERT_NO_SEGLOCK(fs);
mutex_enter(&lfs_lock);
/* disallow dirops during flush */
@@ -596,6 +596,21 @@ lfs_writer_enter(struct lfs *fs, const c
return error;
}
+int
+lfs_writer_tryenter(struct lfs *fs)
+{
+ int writer_set;
+
+ ASSERT_MAYBE_SEGLOCK(fs);
+ mutex_enter(&lfs_lock);
+ writer_set = (fs->lfs_dirops == 0);
+ if (writer_set)
+ fs->lfs_writer++;
+ mutex_exit(&lfs_lock);
+
+ return writer_set;
+}
+
void
lfs_writer_leave(struct lfs *fs)
{
