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>