vinvalbuf, called from vclean, could cause a locking-against-self

deadlock in VOP_FSYNC() if the unreferenced vnode picked for
reclamation happened to be stacked on top of a vnode the process
already had locked.  This could happen if the same filesystem was
accessed both through a union mount and directly; it seemed to happen
most frequently when the direct access was through NFS.

Avoid this deadlock by changing vinvalbuf to pass a new FSYNC_RECLAIM
flag bit to VOP_FSYNC() to indicate that a reclaim is in progress and
only a `shallow' fsync is necessary.

Do nothing in *_fsync() in umapfs, nullfs, and unionfs when
FSYNC_RECLAIM is set; the underlying vnodes will shortly be released
in *_reclaim and may be reclaimed (and fsync'ed) later.
This commit is contained in:
sommerfe 1999-03-22 17:24:19 +00:00
parent 6d0f67d6aa
commit 36dc99adac
4 changed files with 78 additions and 11 deletions

View File

@ -1,4 +1,4 @@
/* $NetBSD: vfs_subr.c,v 1.98 1999/02/09 01:57:05 wrstuden Exp $ */ /* $NetBSD: vfs_subr.c,v 1.99 1999/03/22 17:24:19 sommerfe Exp $ */
/*- /*-
* Copyright (c) 1997, 1998 The NetBSD Foundation, Inc. * Copyright (c) 1997, 1998 The NetBSD Foundation, Inc.
@ -529,7 +529,8 @@ vinvalbuf(vp, flags, cred, p, slpflag, slptimeo)
int s, error; int s, error;
if (flags & V_SAVE) { if (flags & V_SAVE) {
if ((error = VOP_FSYNC(vp, cred, FSYNC_WAIT, p)) != 0) error = VOP_FSYNC(vp, cred, FSYNC_WAIT|FSYNC_RECLAIM, p);
if (error != 0)
return (error); return (error);
if (vp->v_dirtyblkhd.lh_first != NULL) if (vp->v_dirtyblkhd.lh_first != NULL)
panic("vinvalbuf: dirty bufs"); panic("vinvalbuf: dirty bufs");

View File

@ -1,4 +1,4 @@
/* $NetBSD: null_vnops.c,v 1.13 1998/03/01 02:21:43 fvdl Exp $ */ /* $NetBSD: null_vnops.c,v 1.14 1999/03/22 17:24:21 sommerfe Exp $ */
/* /*
* Copyright (c) 1992, 1993 * Copyright (c) 1992, 1993
@ -39,7 +39,7 @@
* *
* Ancestors: * Ancestors:
* @(#)lofs_vnops.c 1.2 (Berkeley) 6/18/92 * @(#)lofs_vnops.c 1.2 (Berkeley) 6/18/92
* $Id: null_vnops.c,v 1.13 1998/03/01 02:21:43 fvdl Exp $ * $Id: null_vnops.c,v 1.14 1999/03/22 17:24:21 sommerfe Exp $
* ...and... * ...and...
* @(#)null_vnodeops.c 1.20 92/07/07 UCLA Ficus project * @(#)null_vnodeops.c 1.20 92/07/07 UCLA Ficus project
*/ */
@ -199,7 +199,7 @@ int null_strategy __P((void *));
int null_bwrite __P((void *)); int null_bwrite __P((void *));
int null_lock __P((void *)); int null_lock __P((void *));
int null_unlock __P((void *)); int null_unlock __P((void *));
int null_islocked __P((void *)); int null_fsync __P((void *));
int null_lookup __P((void *)); int null_lookup __P((void *));
int null_setattr __P((void *)); int null_setattr __P((void *));
int null_access __P((void *)); int null_access __P((void *));
@ -536,6 +536,32 @@ null_unlock(v)
return (null_bypass(ap)); return (null_bypass(ap));
} }
/*
* If vinvalbuf is calling us, it's a "shallow fsync" -- don't bother
* syncing the underlying vnodes, since (a) they'll be fsync'ed when
* reclaimed and (b) we could deadlock if they're locked; otherwise,
* pass it through to the underlying layer.
*/
int
null_fsync(v)
void *v;
{
struct vop_fsync_args /* {
struct vnode *a_vp;
struct ucred *a_cred;
int a_flags;
struct proc *a_p;
} */ *ap = v;
if (ap->a_flags & FSYNC_RECLAIM) {
return 0;
}
return (null_bypass(ap));
}
int int
null_inactive(v) null_inactive(v)
void *v; void *v;
@ -655,7 +681,7 @@ null_bwrite(v)
*/ */
int (**null_vnodeop_p) __P((void *)); int (**null_vnodeop_p) __P((void *));
struct vnodeopv_entry_desc null_vnodeop_entries[] = { struct vnodeopv_entry_desc null_vnodeop_entries[] = {
{ &vop_default_desc, null_bypass }, { &vop_default_desc, null_bypass },
{ &vop_lookup_desc, null_lookup }, { &vop_lookup_desc, null_lookup },
{ &vop_setattr_desc, null_setattr }, { &vop_setattr_desc, null_setattr },
@ -663,6 +689,7 @@ struct vnodeopv_entry_desc null_vnodeop_entries[] = {
{ &vop_access_desc, null_access }, { &vop_access_desc, null_access },
{ &vop_lock_desc, null_lock }, { &vop_lock_desc, null_lock },
{ &vop_unlock_desc, null_unlock }, { &vop_unlock_desc, null_unlock },
{ &vop_fsync_desc, null_fsync },
{ &vop_inactive_desc, null_inactive }, { &vop_inactive_desc, null_inactive },
{ &vop_reclaim_desc, null_reclaim }, { &vop_reclaim_desc, null_reclaim },
{ &vop_print_desc, null_print }, { &vop_print_desc, null_print },

View File

@ -1,4 +1,4 @@
/* $NetBSD: umap_vnops.c,v 1.11 1999/03/19 21:46:26 perseant Exp $ */ /* $NetBSD: umap_vnops.c,v 1.12 1999/03/22 17:24:22 sommerfe Exp $ */
/* /*
* Copyright (c) 1992, 1993 * Copyright (c) 1992, 1993
@ -67,6 +67,7 @@ int umap_strategy __P((void *));
int umap_bwrite __P((void *)); int umap_bwrite __P((void *));
int umap_lock __P((void *)); int umap_lock __P((void *));
int umap_unlock __P((void *)); int umap_unlock __P((void *));
int umap_fsync __P((void *));
extern int null_bypass __P((void *)); extern int null_bypass __P((void *));
@ -85,6 +86,7 @@ struct vnodeopv_entry_desc umap_vnodeop_entries[] = {
{ &vop_getattr_desc, umap_getattr }, { &vop_getattr_desc, umap_getattr },
{ &vop_lock_desc, umap_lock }, { &vop_lock_desc, umap_lock },
{ &vop_unlock_desc, umap_unlock }, { &vop_unlock_desc, umap_unlock },
{ &vop_fsync_desc, umap_fsync },
{ &vop_inactive_desc, umap_inactive }, { &vop_inactive_desc, umap_inactive },
{ &vop_reclaim_desc, umap_reclaim }, { &vop_reclaim_desc, umap_reclaim },
{ &vop_print_desc, umap_print }, { &vop_print_desc, umap_print },
@ -338,6 +340,30 @@ umap_unlock(v)
return (null_bypass(ap)); return (null_bypass(ap));
} }
/*
* If vinvalbuf is calling us, it's a "shallow fsync" -- don't bother
* syncing the underlying vnodes, since (a) they'll be fsync'ed when
* reclaimed and (b) we could deadlock if they're locked; otherwise,
* pass it through to the underlying layer.
*/
int
umap_fsync(v)
void *v;
{
struct vop_fsync_args /* {
struct vnode *a_vp;
struct ucred *a_cred;
int a_flags;
struct proc *a_p;
} */ *ap = v;
if (ap->a_flags & FSYNC_RECLAIM)
return 0;
return (umap_bypass(ap));
}
/* /*
* We handle getattr to change the fsid. * We handle getattr to change the fsid.
*/ */

View File

@ -1,4 +1,4 @@
/* $NetBSD: union_vnops.c,v 1.42 1998/06/05 19:53:00 kleink Exp $ */ /* $NetBSD: union_vnops.c,v 1.43 1999/03/22 17:24:22 sommerfe Exp $ */
/* /*
* Copyright (c) 1992, 1993, 1994, 1995 Jan-Simon Pendry. * Copyright (c) 1992, 1993, 1994, 1995 Jan-Simon Pendry.
@ -1026,12 +1026,25 @@ union_fsync(v)
struct vop_fsync_args /* { struct vop_fsync_args /* {
struct vnode *a_vp; struct vnode *a_vp;
struct ucred *a_cred; struct ucred *a_cred;
int a_waitfor; int a_flags;
struct proc *a_p; struct proc *a_p;
} */ *ap = v; } */ *ap = v;
int error = 0; int error = 0;
struct proc *p = ap->a_p; struct proc *p;
struct vnode *targetvp = OTHERVP(ap->a_vp); struct vnode *targetvp;
/*
* If vinvalbuf is calling us, it's a "shallow fsync" -- don't
* bother syncing the underlying vnodes, since (a) they'll be
* fsync'ed when reclaimed and (b) we could deadlock if
* they're locked; otherwise, pass it through to the
* underlying layer.
*/
if (ap->a_flags & FSYNC_RECLAIM)
return 0;
targetvp = OTHERVP(ap->a_vp);
p = ap->a_p;
if (targetvp != NULLVP) { if (targetvp != NULLVP) {
int dolock = (targetvp == LOWERVP(ap->a_vp)); int dolock = (targetvp == LOWERVP(ap->a_vp));