Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys/ufs Make LFS dirops get their vnode first, before increm...
details: https://anonhg.NetBSD.org/src/rev/23af510bfca6
branches: trunk
changeset: 579724:23af510bfca6
user: perseant <perseant%NetBSD.org@localhost>
date: Wed Mar 23 00:12:51 2005 +0000
description:
Make LFS dirops get their vnode first, before incrementing the dirop count,
to prevent a deadlock trying to call VOP_PUTPAGES() on a VDIROP vnode.
This can happen when a stacked filesystem is mounted on top of an LFS: an
LFS dirop needs to get a vnode, which is available from the upper layer.
The corresponding lower layer vnode, however, is VDIROP, so the upper layer
can't be cleaned out since its VOP_PUTPAGES() is passed through to the lower
layer, which waits for dirops to drain before it can proceed. Deadlock.
Tweak ufs_makeinode() and ufs_mkdir() to pass the a_vpp argument through
to VOP_VALLOC().
Partially addresses PR # 26043, though it probably does not completely fix
the problem described there.
diffstat:
sys/ufs/lfs/lfs_alloc.c | 35 +------
sys/ufs/lfs/lfs_vnops.c | 208 +++++++++++++++++++++++++++--------------------
sys/ufs/ufs/ufs_vnops.c | 12 +-
3 files changed, 130 insertions(+), 125 deletions(-)
diffs (truncated from 521 to 300 lines):
diff -r 59c374e0573e -r 23af510bfca6 sys/ufs/lfs/lfs_alloc.c
--- a/sys/ufs/lfs/lfs_alloc.c Tue Mar 22 23:55:46 2005 +0000
+++ b/sys/ufs/lfs/lfs_alloc.c Wed Mar 23 00:12:51 2005 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: lfs_alloc.c,v 1.76 2005/03/08 00:18:19 perseant Exp $ */
+/* $NetBSD: lfs_alloc.c,v 1.77 2005/03/23 00:12:51 perseant Exp $ */
/*-
* Copyright (c) 1999, 2000, 2001, 2002, 2003 The NetBSD Foundation, Inc.
@@ -67,7 +67,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: lfs_alloc.c,v 1.76 2005/03/08 00:18:19 perseant Exp $");
+__KERNEL_RCSID(0, "$NetBSD: lfs_alloc.c,v 1.77 2005/03/23 00:12:51 perseant Exp $");
#if defined(_KERNEL_OPT)
#include "opt_quota.h"
@@ -294,7 +294,6 @@
fs = VTOI(ap->a_pvp)->i_lfs;
if (fs->lfs_ronly)
return EROFS;
- *ap->a_vpp = NULL;
lfs_seglock(fs, SEGM_PROT);
@@ -352,17 +351,8 @@
{
struct inode *ip;
struct vnode *vp;
- IFILE *ifp;
- struct buf *bp, *cbp;
- int error;
- CLEANERINFO *cip;
- error = getnewvnode(VT_LFS, pvp->v_mount, lfs_vnodeop_p, &vp);
- DLOG((DLOG_ALLOC, "lfs_ialloc: ino %d vp %p error %d\n", new_ino,
- vp, error));
- if (error)
- goto errout;
-
+ vp = *vpp;
lockmgr(&ufs_hashlock, LK_EXCLUSIVE, 0);
/* Create an inode to associate with the vnode. */
lfs_vcreate(pvp->v_mount, new_ino, vp);
@@ -382,13 +372,13 @@
ufs_ihashins(ip);
lockmgr(&ufs_hashlock, LK_RELEASE, 0);
- ufs_vinit(vp->v_mount, lfs_specop_p, lfs_fifoop_p, &vp);
+ ufs_vinit(vp->v_mount, lfs_specop_p, lfs_fifoop_p, vpp);
+ vp = *vpp;
ip = VTOI(vp);
memset(ip->i_lfs_fragsize, 0, NDADDR * sizeof(*ip->i_lfs_fragsize));
uvm_vnp_setsize(vp, 0);
- *vpp = vp;
lfs_mark_vnode(vp);
genfs_node_init(vp, &lfs_genfsops);
VREF(ip->i_devvp);
@@ -396,21 +386,6 @@
fs->lfs_fmod = 1;
++fs->lfs_nfiles;
return (0);
-
- errout:
- /*
- * Put the new inum back on the free list.
- */
- lfs_seglock(fs, SEGM_PROT);
- LFS_IENTRY(ifp, fs, new_ino, bp);
- ifp->if_daddr = LFS_UNUSED_DADDR;
- LFS_GET_HEADFREE(fs, cip, cbp, &(ifp->if_nextfree));
- LFS_PUT_HEADFREE(fs, cip, cbp, new_ino);
- (void) LFS_BWRITE_LOG(bp); /* Ifile */
- lfs_segunlock(fs);
-
- *vpp = NULLVP;
- return (error);
}
/* Create a new vnode/inode pair and initialize what fields we can. */
diff -r 59c374e0573e -r 23af510bfca6 sys/ufs/lfs/lfs_vnops.c
--- a/sys/ufs/lfs/lfs_vnops.c Tue Mar 22 23:55:46 2005 +0000
+++ b/sys/ufs/lfs/lfs_vnops.c Wed Mar 23 00:12:51 2005 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: lfs_vnops.c,v 1.137 2005/03/08 04:49:35 simonb Exp $ */
+/* $NetBSD: lfs_vnops.c,v 1.138 2005/03/23 00:12:51 perseant Exp $ */
/*-
* Copyright (c) 1999, 2000, 2001, 2002, 2003 The NetBSD Foundation, Inc.
@@ -67,7 +67,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: lfs_vnops.c,v 1.137 2005/03/08 04:49:35 simonb Exp $");
+__KERNEL_RCSID(0, "$NetBSD: lfs_vnops.c,v 1.138 2005/03/23 00:12:51 perseant Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -362,37 +362,39 @@
* These macros are used to bracket UFS directory ops, so that we can
* identify all the pages touched during directory ops which need to
* be ordered and flushed atomically, so that they may be recovered.
- */
-/*
- * XXX KS - Because we have to mark nodes VDIROP in order to prevent
+ *
+ * Because we have to mark nodes VDIROP in order to prevent
* the cache from reclaiming them while a dirop is in progress, we must
* also manage the number of nodes so marked (otherwise we can run out).
* We do this by setting lfs_dirvcount to the number of marked vnodes; it
* is decremented during segment write, when VDIROP is taken off.
*/
-#define SET_DIROP(vp) SET_DIROP2((vp), NULL)
-#define SET_DIROP2(vp, vp2) lfs_set_dirop((vp), (vp2))
+#define MARK_VNODE(vp) lfs_mark_vnode(vp)
+#define UNMARK_VNODE(vp) lfs_unmark_vnode(vp)
+#define SET_DIROP_CREATE(dvp, vpp) lfs_set_dirop_create((dvp), (vpp))
+#define SET_DIROP_REMOVE(dvp, vp) lfs_set_dirop((dvp), (vp))
+static int lfs_set_dirop_create(struct vnode *, struct vnode **);
static int lfs_set_dirop(struct vnode *, struct vnode *);
static int
-lfs_set_dirop(struct vnode *vp, struct vnode *vp2)
+lfs_set_dirop(struct vnode *dvp, struct vnode *vp)
{
struct lfs *fs;
int error;
- KASSERT(VOP_ISLOCKED(vp));
- KASSERT(vp2 == NULL || VOP_ISLOCKED(vp2));
+ KASSERT(VOP_ISLOCKED(dvp));
+ KASSERT(vp == NULL || VOP_ISLOCKED(vp));
- fs = VTOI(vp)->i_lfs;
+ fs = VTOI(dvp)->i_lfs;
/*
* LFS_NRESERVE calculates direct and indirect blocks as well
* as an inode block; an overestimate in most cases.
*/
- if ((error = lfs_reserve(fs, vp, vp2, LFS_NRESERVE(fs))) != 0)
+ if ((error = lfs_reserve(fs, dvp, vp, LFS_NRESERVE(fs))) != 0)
return (error);
if (fs->lfs_dirops == 0)
- lfs_check(vp, LFS_UNUSED_LBN, 0);
+ lfs_check(dvp, LFS_UNUSED_LBN, 0);
restart:
simple_lock(&fs->lfs_interlock);
if (fs->lfs_writer) {
@@ -427,36 +429,94 @@
simple_unlock(&fs->lfs_interlock);
/* Hold a reference so SET_ENDOP will be happy */
- vref(vp);
- if (vp2)
- vref(vp2);
+ vref(dvp);
+ if (vp) {
+ vref(vp);
+ MARK_VNODE(vp);
+ }
+ MARK_VNODE(dvp);
return 0;
unreserve:
- lfs_reserve(fs, vp, vp2, -LFS_NRESERVE(fs));
+ lfs_reserve(fs, dvp, vp, -LFS_NRESERVE(fs));
return error;
}
-#define SET_ENDOP(fs, vp, str) SET_ENDOP2((fs), (vp), NULL, (str))
-#define SET_ENDOP2(fs, vp, vp2, str) { \
- --(fs)->lfs_dirops; \
- if (!(fs)->lfs_dirops) { \
- if ((fs)->lfs_nadirop) { \
- panic("SET_ENDOP: %s: no dirops but nadirop=%d", \
- (str), (fs)->lfs_nadirop); \
- } \
- wakeup(&(fs)->lfs_writer); \
- lfs_check((vp),LFS_UNUSED_LBN,0); \
- } \
- lfs_reserve((fs), vp, vp2, -LFS_NRESERVE(fs)); /* XXX */ \
- vrele(vp); \
- if (vp2) \
- vrele(vp2); \
+/*
+ * Get a new vnode *before* adjusting the dirop count, to avoid a deadlock
+ * in getnewvnode(), if we have a stacked filesystem mounted on top
+ * of us.
+ *
+ * NB: this means we have to clear the new vnodes on error. Fortunately
+ * SET_ENDOP is there to do that for us.
+ */
+static int
+lfs_set_dirop_create(struct vnode *dvp, struct vnode **vpp)
+{
+ int error;
+ struct lfs *fs;
+
+ fs = VFSTOUFS(dvp->v_mount)->um_lfs;
+ if (fs->lfs_ronly)
+ return EROFS;
+ if (vpp && (error = getnewvnode(VT_LFS, dvp->v_mount, lfs_vnodeop_p, vpp))) {
+ DLOG((DLOG_ALLOC, "lfs_set_dirop_create: dvp %p error %d\n",
+ dvp, error));
+ return error;
+ }
+ if ((error = lfs_set_dirop(dvp, NULL)) != 0) {
+ if (vpp) {
+ ungetnewvnode(*vpp);
+ *vpp = NULL;
+ }
+ return error;
+ }
+ return 0;
}
-#define MARK_VNODE(vp) lfs_mark_vnode(vp)
-#define UNMARK_VNODE(vp) lfs_unmark_vnode(vp)
+#define SET_ENDOP_BASE(fs, dvp, str) \
+ do { \
+ simple_lock(&(fs)->lfs_interlock); \
+ --(fs)->lfs_dirops; \
+ if (!(fs)->lfs_dirops) { \
+ if ((fs)->lfs_nadirop) { \
+ panic("SET_ENDOP: %s: no dirops but " \
+ " nadirop=%d", (str), \
+ (fs)->lfs_nadirop); \
+ } \
+ wakeup(&(fs)->lfs_writer); \
+ simple_unlock(&(fs)->lfs_interlock); \
+ lfs_check((dvp), LFS_UNUSED_LBN, 0); \
+ } else \
+ simple_unlock(&(fs)->lfs_interlock); \
+ } while(0)
+#define SET_ENDOP_CREATE(fs, dvp, nvpp, str) \
+ do { \
+ UNMARK_VNODE(dvp); \
+ if (nvpp && *nvpp) \
+ UNMARK_VNODE(*nvpp); \
+ /* Check for error return to stem vnode leakage */ \
+ if (nvpp && *nvpp && !((*nvpp)->v_flag & VDIROP)) \
+ ungetnewvnode(*(nvpp)); \
+ SET_ENDOP_BASE((fs), (dvp), (str)); \
+ lfs_reserve((fs), (dvp), NULL, -LFS_NRESERVE(fs)); \
+ vrele(dvp); \
+ } while(0)
+#define SET_ENDOP_CREATE_AP(ap, str) \
+ SET_ENDOP_CREATE(VTOI((ap)->a_dvp)->i_lfs, (ap)->a_dvp, \
+ (ap)->a_vpp, (str))
+#define SET_ENDOP_REMOVE(fs, dvp, ovp, str) \
+ do { \
+ UNMARK_VNODE(dvp); \
+ if (ovp) \
+ UNMARK_VNODE(ovp); \
+ SET_ENDOP_BASE((fs), (dvp), (str)); \
+ lfs_reserve((fs), (dvp), (ovp), -LFS_NRESERVE(fs)); \
+ vrele(dvp); \
+ if (ovp) \
+ vrele(ovp); \
+ } while(0)
void
lfs_mark_vnode(struct vnode *vp)
@@ -501,16 +561,12 @@
} */ *ap = v;
int error;
- if ((error = SET_DIROP(ap->a_dvp)) != 0) {
+ if ((error = SET_DIROP_CREATE(ap->a_dvp, ap->a_vpp)) != 0) {
vput(ap->a_dvp);
return error;
}
- MARK_VNODE(ap->a_dvp);
error = ufs_symlink(ap);
- UNMARK_VNODE(ap->a_dvp);
- if (*(ap->a_vpp))
- UNMARK_VNODE(*(ap->a_vpp));
- SET_ENDOP(VTOI(ap->a_dvp)->i_lfs,ap->a_dvp,"symlink");
+ SET_ENDOP_CREATE_AP(ap, "symlink");
return (error);
}
@@ -530,19 +586,15 @@
struct mount *mp;
ino_t ino;
- if ((error = SET_DIROP(ap->a_dvp)) != 0) {
+ if ((error = SET_DIROP_CREATE(ap->a_dvp, ap->a_vpp)) != 0) {
vput(ap->a_dvp);
return error;
}
- MARK_VNODE(ap->a_dvp);
error = ufs_makeinode(MAKEIMODE(vap->va_type, vap->va_mode),
ap->a_dvp, vpp, ap->a_cnp);
- UNMARK_VNODE(ap->a_dvp);
Home |
Main Index |
Thread Index |
Old Index