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) {