Date: Wed, 16 Aug 2000 01:10:07 -0700 (PDT) From: Sheldon Hearn <sheldonh@uunet.co.za> To: freebsd-bugs@FreeBSD.org Subject: Re: kern/20632: stacking mount_null causes an error: mount_null: Resource deadlock avoided Message-ID: <200008160810.BAA92657@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/20632; it has been noted by GNATS.
From: Sheldon Hearn <sheldonh@uunet.co.za>
To: akr@m17n.org
Cc: FreeBSD-gnats-submit@FreeBSD.ORG
Subject: Re: kern/20632: stacking mount_null causes an error: mount_null: Resource deadlock avoided
Date: Wed, 16 Aug 2000 10:01:58 +0200
On Wed, 16 Aug 2000 12:45:21 +0900, akr@m17n.org wrote:
> If null file system is stacked by mount_null, following error is caused.
>
> mount_null: Resource deadlock avoided
>
> Then, umount cannot unmount the file system with same error and some
> commands hangs forever.
NULLFS is known to be broken. You might want to try out the following
patch (unfortunately I forget who sent it, but I'll have a look) and
report back.
Ciao,
Sheldon.
Common subdirectories: ./nullfs.orig/CVS and ./nullfs/CVS
diff -c ./nullfs.orig/null_vfsops.c ./nullfs/null_vfsops.c
*** ./nullfs.orig/null_vfsops.c Fri Aug 4 16:38:49 2000
--- ./nullfs/null_vfsops.c Sun Aug 6 19:58:23 2000
***************
*** 57,66 ****
static MALLOC_DEFINE(M_NULLFSMNT, "NULLFS mount", "NULLFS mount structure");
- static int nullfs_fhtovp __P((struct mount *mp, struct fid *fidp,
- struct vnode **vpp));
static int nullfs_checkexp __P((struct mount *mp, struct sockaddr *nam,
int *extflagsp, struct ucred **credanonp));
static int nullfs_mount __P((struct mount *mp, char *path, caddr_t data,
struct nameidata *ndp, struct proc *p));
static int nullfs_quotactl __P((struct mount *mp, int cmd, uid_t uid,
--- 57,69 ----
static MALLOC_DEFINE(M_NULLFSMNT, "NULLFS mount", "NULLFS mount structure");
static int nullfs_checkexp __P((struct mount *mp, struct sockaddr *nam,
int *extflagsp, struct ucred **credanonp));
+ static int nullfs_extattrctl __P((struct mount *mp, int cmd,
+ const char *attrname, caddr_t arg,
+ struct proc *p));
+ static int nullfs_fhtovp __P((struct mount *mp, struct fid *fidp,
+ struct vnode **vpp));
static int nullfs_mount __P((struct mount *mp, char *path, caddr_t data,
struct nameidata *ndp, struct proc *p));
static int nullfs_quotactl __P((struct mount *mp, int cmd, uid_t uid,
***************
*** 372,378 ****
struct proc *p;
{
/*
! * XXX - Assumes no data cached at null layer.
*/
return (0);
}
--- 375,383 ----
struct proc *p;
{
/*
! * XXX There are no buffers to flush, as we don't
! * have VOP_BMAP. But there are pages. Don't
! * know how to flush pages.
*/
return (0);
}
***************
*** 383,390 ****
ino_t ino;
struct vnode **vpp;
{
! return VFS_VGET(MOUNTTONULLMOUNT(mp)->nullm_vfs, ino, vpp);
}
static int
--- 388,399 ----
ino_t ino;
struct vnode **vpp;
{
+ int error;
+ error = VFS_VGET(MOUNTTONULLMOUNT(mp)->nullm_vfs, ino, vpp);
+ if (error)
+ return (error);
! return (null_node_create(mp, *vpp, vpp));
}
static int
***************
*** 393,400 ****
struct fid *fidp;
struct vnode **vpp;
{
! return VFS_FHTOVP(MOUNTTONULLMOUNT(mp)->nullm_vfs, fidp, vpp);
}
static int
--- 402,413 ----
struct fid *fidp;
struct vnode **vpp;
{
+ int error;
+ error = VFS_FHTOVP(MOUNTTONULLMOUNT(mp)->nullm_vfs, fidp, vpp);
+ if (error)
+ return (error);
! return (null_node_create(mp, *vpp, vpp));
}
static int
diff -c ./nullfs.orig/null_vnops.c ./nullfs/null_vnops.c
*** ./nullfs.orig/null_vnops.c Fri Aug 4 16:38:49 2000
--- ./nullfs/null_vnops.c Sun Aug 6 20:05:18 2000
***************
*** 37,43 ****
*
* Ancestors:
* @(#)lofs_vnops.c 1.2 (Berkeley) 6/18/92
- * $FreeBSD: src/sys/miscfs/nullfs/null_vnops.c,v 1.38 1999/12/11 16:12:56 eivind Exp $
* ...and...
* @(#)null_vnodeops.c 1.20 92/07/07 UCLA Ficus project
*
--- 37,42 ----
***************
*** 183,188 ****
--- 182,193 ----
#include <sys/namei.h>
#include <sys/malloc.h>
#include <sys/buf.h>
+
+ #include <vm/vm.h>
+ #include <vm/vm_extern.h>
+ #include <vm/vm_object.h>
+ #include <vm/vnode_pager.h>
+
#include <miscfs/nullfs/null.h>
static int null_bug_bypass = 0; /* for debugging: enables bypass printf'ing */
***************
*** 191,203 ****
--- 196,341 ----
static int null_access __P((struct vop_access_args *ap));
static int null_getattr __P((struct vop_getattr_args *ap));
+ static int null_getpages __P((struct vop_getpages_args *ap));
static int null_inactive __P((struct vop_inactive_args *ap));
static int null_lock __P((struct vop_lock_args *ap));
static int null_lookup __P((struct vop_lookup_args *ap));
+ static int null_open __P((struct vop_open_args *));
static int null_print __P((struct vop_print_args *ap));
+ static int null_putpages __P((struct vop_putpages_args *));
static int null_reclaim __P((struct vop_reclaim_args *ap));
+ static int null_rename __P((struct vop_rename_args *));
static int null_setattr __P((struct vop_setattr_args *ap));
+ static int null_strategy __P((struct vop_strategy_args *ap));
static int null_unlock __P((struct vop_unlock_args *ap));
+ static int null_write __P((struct vop_write_args *ap));
+
+ /*
+ * We handle this to eliminate null FS to lower FS
+ * file moving. Don't know why we don't allow this,
+ * possibly we should.
+ */
+ static int
+ null_rename(ap)
+ struct vop_rename_args /* {
+ struct vnode *a_fdvp;
+ struct vnode *a_fvp;
+ struct componentname *a_fcnp;
+ struct vnode *a_tdvp;
+ struct vnode *a_tvp;
+ struct componentname *a_tcnp;
+ } */ *ap;
+ {
+ struct vnode *tdvp = ap->a_tdvp;
+ struct vnode *fvp = ap->a_fvp;
+ struct vnode *fdvp = ap->a_fdvp;
+ struct vnode *tvp = ap->a_tvp;
+
+ /* Check for cross-device rename. */
+ if ((fvp->v_mount != tdvp->v_mount) ||
+ (tvp && (fvp->v_mount != tvp->v_mount))) {
+ if (tdvp == tvp)
+ vrele(tdvp);
+ else
+ vput(tdvp);
+ if (tvp)
+ vput(tvp);
+ vrele(fdvp);
+ vrele(fvp);
+ return (EXDEV);
+ }
+
+ return (null_bypass((struct vop_generic_args *)ap));
+ }
+
+ /*
+ * We handle this to create vm_object for lowervp as
+ * vn_open does for us.
+ */
+ int
+ null_open(ap)
+ struct vop_open_args *ap;
+ {
+ int error;
+
+ /* Process operation */
+ error = null_bypass((struct vop_generic_args *)ap);
+ if (error)
+ return (error);
+
+ /* Create object, vfs_object_create() check itself if vnode
+ * already have one or if it can't have one */
+ vfs_object_create (NULLVPTOLOWERVP(ap->a_vp), ap->a_p, ap->a_cred);
+
+ return (error);
+ }
+
+ /*
+ * Panic VOP_STRATEGY operation. Shouldn't happen, as we
+ * don't have VOP_BMAP.
+ */
+ static int
+ null_strategy(ap)
+ struct vop_strategy_args *ap;
+ {
+ panic("null_strategy: NOT IMPLEMENTED!");
+
+ return (EOPNOTSUPP);
+ }
+
+ /*
+ * We need to look for file size changes, to keep
+ * vnode_pager up with correct file size.
+ */
+ static int
+ null_write(ap)
+ struct vop_write_args /* {
+ struct vnode *a_vp;
+ struct uio *a_uio;
+ int a_ioflag;
+ struct ucred *a_cred;
+ } */ *ap;
+ {
+ struct vnode *vp = ap->a_vp;
+ struct uio *uio = ap->a_uio;
+ off_t newsize;
+ off_t oldsize;
+ int error;
+
+ /* Process write operation */
+ error = null_bypass((struct vop_generic_args *)ap);
+
+ /* Update vm_object size, if exist and file size changed */
+ if (vp->v_object) {
+ oldsize = vp->v_object->un_pager.vnp.vnp_size;
+ newsize = uio->uio_offset;
+ if (newsize > oldsize)
+ vnode_pager_setsize(vp, newsize);
+ }
+
+ return (error);
+ }
+
+ /*
+ * We use generic (put|get)pages routines and no VOP_BMAP operation,
+ * this force vnode_pager to use VOP_READ and VOP_WRITE
+ * to access pages.
+ */
+ int
+ null_getpages(ap)
+ struct vop_getpages_args *ap;
+ {
+ return vnode_pager_generic_getpages(ap->a_vp, ap->a_m, ap->a_count,
+ ap->a_reqpage);
+ }
+
+ int
+ null_putpages(ap)
+ struct vop_putpages_args *ap;
+ {
+ return vnode_pager_generic_putpages(ap->a_vp, ap->a_m, ap->a_count,
+ ap->a_sync, ap->a_rtvals);
+ }
/*
* This is the 10-Apr-92 bypass routine.
***************
*** 389,394 ****
--- 527,534 ----
/*
* Setattr call. Disallow write attempts if the layer is mounted read-only.
+ * Also look for file size change, and do appropriate vm_object
+ * modification.
*/
int
null_setattr(ap)
***************
*** 402,407 ****
--- 542,548 ----
{
struct vnode *vp = ap->a_vp;
struct vattr *vap = ap->a_vap;
+ int error;
if ((vap->va_flags != VNOVAL || vap->va_uid != (uid_t)VNOVAL ||
vap->va_gid != (gid_t)VNOVAL || vap->va_atime.tv_sec != VNOVAL ||
***************
*** 430,436 ****
return (EROFS);
}
}
! return (null_bypass((struct vop_generic_args *)ap));
}
/*
--- 571,590 ----
return (EROFS);
}
}
!
! error = null_bypass((struct vop_generic_args *)ap);
! if (error || vp->v_object == NULL || vap->va_size == VNOVAL)
! return (error);
!
! /* If they have changed file size, tell to vnode_pager.
! * XXX If they have shrink the file, do we need to
! * invalidate buffers with vtruncbuf()? I think
! * no.
! */
! if (vap->va_size != vp->v_object->un_pager.vnp.vnp_size)
! vnode_pager_setsize(vp, vap->va_size);
!
! return (error);
}
/*
***************
*** 449,457 ****
--- 603,618 ----
if ((error = null_bypass((struct vop_generic_args *)ap)) != 0)
return (error);
+
+ /* Fake fsid, also done in vn_stat(), but... */
+ ap->a_vap->va_fsid = ap->a_vp->v_mount->mnt_stat.f_fsid.val[0];
+
return (0);
}
+ /*
+ * Handle to disallow write access if mounted read-only.
+ */
static int
null_access(ap)
struct vop_access_args /* {
***************
*** 519,528 ****
--- 680,700 ----
} */ *ap;
{
vop_nounlock(ap);
+
+ /* Support unlocking of inactive vnodes */
+ if (NULLVPTOLOWERVP(ap->a_vp) == NULLVP)
+ return (0);
+
ap->a_flags &= ~LK_INTERLOCK;
return (null_bypass((struct vop_generic_args *)ap));
}
+ /*
+ * We have to vrele lowervp as soon as possible. Otherway
+ * we can never discover that someone has removed file
+ * on lower FS. This technique removes advantages of hash,
+ * but i don't know other way.
+ */
static int
null_inactive(ap)
struct vop_inactive_args /* {
***************
*** 533,557 ****
struct vnode *vp = ap->a_vp;
struct null_node *xp = VTONULL(vp);
struct vnode *lowervp = xp->null_lowervp;
! /*
! * Do nothing (and _don't_ bypass).
! * Wait to vrele lowervp until reclaim,
! * so that until then our null_node is in the
! * cache and reusable.
! * We still have to tell the lower layer the vnode
! * is now inactive though.
! *
! * NEEDSWORK: Someday, consider inactive'ing
! * the lowervp and then trying to reactivate it
! * with capabilities (v_id)
! * like they do in the name lookup cache code.
! * That's too much work for now.
! */
! VOP_INACTIVE(lowervp, ap->a_p);
VOP_UNLOCK(ap->a_vp, 0, ap->a_p);
return (0);
}
static int
null_reclaim(ap)
struct vop_reclaim_args /* {
--- 705,729 ----
struct vnode *vp = ap->a_vp;
struct null_node *xp = VTONULL(vp);
struct vnode *lowervp = xp->null_lowervp;
!
! /* Release lowervp totaly */
! VOP_UNLOCK(lowervp, 0, ap->a_p);
! vrele (lowervp);
!
! /* Forget all we knew, remove hash entry */
! xp->null_lowervp = NULLVP;
! LIST_REMOVE(xp, null_hash);
!
! /* Unlock our vnode */
VOP_UNLOCK(ap->a_vp, 0, ap->a_p);
+
return (0);
}
+ /*
+ * We can free memory in null_inactive, but we do this
+ * here. (Possible to guard vp->v_data to point somewhere)
+ */
static int
null_reclaim(ap)
struct vop_reclaim_args /* {
***************
*** 560,578 ****
} */ *ap;
{
struct vnode *vp = ap->a_vp;
- struct null_node *xp = VTONULL(vp);
- struct vnode *lowervp = xp->null_lowervp;
- /*
- * Note: in vop_reclaim, vp->v_op == dead_vnodeop_p,
- * so we can't call VOPs on ourself.
- */
- /* After this assignment, this node will not be re-used. */
- xp->null_lowervp = NULLVP;
- LIST_REMOVE(xp, null_hash);
FREE(vp->v_data, M_TEMP);
vp->v_data = NULL;
! vrele (lowervp);
return (0);
}
--- 732,741 ----
} */ *ap;
{
struct vnode *vp = ap->a_vp;
FREE(vp->v_data, M_TEMP);
vp->v_data = NULL;
!
return (0);
}
***************
*** 593,607 ****
--- 756,778 ----
vop_t **null_vnodeop_p;
static struct vnodeopv_entry_desc null_vnodeop_entries[] = {
{ &vop_default_desc, (vop_t *) null_bypass },
+
{ &vop_access_desc, (vop_t *) null_access },
+ { &vop_bmap_desc, (vop_t *) vop_eopnotsupp },
{ &vop_getattr_desc, (vop_t *) null_getattr },
+ { &vop_getpages_desc, (vop_t *) null_getpages },
{ &vop_inactive_desc, (vop_t *) null_inactive },
{ &vop_lock_desc, (vop_t *) null_lock },
{ &vop_lookup_desc, (vop_t *) null_lookup },
+ { &vop_open_desc, (vop_t *) null_open },
{ &vop_print_desc, (vop_t *) null_print },
+ { &vop_putpages_desc, (vop_t *) null_putpages },
{ &vop_reclaim_desc, (vop_t *) null_reclaim },
+ { &vop_rename_desc, (vop_t *) null_rename },
{ &vop_setattr_desc, (vop_t *) null_setattr },
+ { &vop_strategy_desc, (vop_t *) null_strategy },
{ &vop_unlock_desc, (vop_t *) null_unlock },
+ { &vop_write_desc, (vop_t *) null_write },
{ NULL, NULL }
};
static struct vnodeopv_desc null_vnodeop_opv_desc =
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200008160810.BAA92657>