Date: Wed, 14 Sep 2005 10:31:44 GMT From: Scott Long <scottl@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 83592 for review Message-ID: <200509141031.j8EAVi84048492@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=83592 Change 83592 by scottl@scottl-junior on 2005/09/14 10:30:45 Move transaction management out of the low-level FFS routines and instead just pass a transaction pointer into them that they can then pass onto the journal API. This is only implemented for ffs_update and UFS_UPDATE at the moment since that is the most simple case. Add a large comment block to ffs_update about a possible deadlock in low memory conditions. Affected files ... .. //depot/projects/soc2005/ufsj/src/sys/ufs/ffs/ffs_alloc.c#3 edit .. //depot/projects/soc2005/ufsj/src/sys/ufs/ffs/ffs_extern.h#2 edit .. //depot/projects/soc2005/ufsj/src/sys/ufs/ffs/ffs_inode.c#4 edit .. //depot/projects/soc2005/ufsj/src/sys/ufs/ffs/ffs_softdep.c#2 edit .. //depot/projects/soc2005/ufsj/src/sys/ufs/ffs/ffs_vfsops.c#4 edit .. //depot/projects/soc2005/ufsj/src/sys/ufs/ffs/ffs_vnops.c#3 edit .. //depot/projects/soc2005/ufsj/src/sys/ufs/ufs/ufs_inode.c#3 edit .. //depot/projects/soc2005/ufsj/src/sys/ufs/ufs/ufs_lookup.c#2 edit .. //depot/projects/soc2005/ufsj/src/sys/ufs/ufs/ufs_vnops.c#3 edit .. //depot/projects/soc2005/ufsj/src/sys/ufs/ufs/ufsmount.h#3 edit Differences ... ==== //depot/projects/soc2005/ufsj/src/sys/ufs/ffs/ffs_alloc.c#3 (text+ko) ==== @@ -607,7 +607,7 @@ } else { ip->i_flag |= IN_CHANGE | IN_UPDATE; if (!doasyncfree) - ffs_update(vp, 1); + ffs_update(vp, 1, NULL); } if (ssize < len) { if (doasyncfree) @@ -814,7 +814,7 @@ } else { ip->i_flag |= IN_CHANGE | IN_UPDATE; if (!doasyncfree) - ffs_update(vp, 1); + ffs_update(vp, 1, NULL); } if (ssize < len) { if (doasyncfree) ==== //depot/projects/soc2005/ufsj/src/sys/ufs/ffs/ffs_extern.h#2 (text+ko) ==== @@ -47,6 +47,7 @@ struct vnode; struct vop_fsync_args; struct vop_reallocblks_args; +struct ufsj_transaction; int ffs_alloc(struct inode *, ufs2_daddr_t, ufs2_daddr_t, int, struct ucred *, ufs2_daddr_t *); @@ -80,7 +81,7 @@ void ffs_snapshot_unmount(struct mount *mp); int ffs_syncvnode(struct vnode *vp, int waitfor); int ffs_truncate(struct vnode *, off_t, int, struct ucred *, struct thread *); -int ffs_update(struct vnode *, int); +int ffs_update(struct vnode *, int, struct ufsj_transaction *); int ffs_valloc(struct vnode *, int, struct ucred *, struct vnode **); int ffs_vfree(struct vnode *, ino_t, int); ==== //depot/projects/soc2005/ufsj/src/sys/ufs/ffs/ffs_inode.c#4 (text+ko) ==== @@ -72,17 +72,15 @@ * set, then wait for the write to complete. */ int -ffs_update(vp, waitfor) +ffs_update(vp, waitfor, jtrn) struct vnode *vp; int waitfor; + ufsj_thandle_t jtrn; { struct fs *fs; struct buf *bp; struct inode *ip; int error; -#ifdef UFS_JOURNAL - ufsj_thandle_t jnl; -#endif ASSERT_VOP_LOCKED(vp, "ffs_update"); ufs_itimes(vp); @@ -111,11 +109,6 @@ } /* We only do writes from this point on */ -#ifdef UFS_JOURNAL - if (DOINGJOURNAL(vp)) - ufsj_start_transaction(ip->i_ump, &jnl, J_TYPE_WRITE); -#endif - if (DOINGSOFTDEP(vp)) softdep_update_inodeblock(ip, bp, waitfor); else if (ip->i_effnlink != ip->i_nlink) @@ -129,31 +122,38 @@ if (waitfor && !DOINGASYNC(vp)) { #ifdef UFS_JOURNAL if (DOINGJOURNAL(vp)) { - ufsj_write_blocks(jnl, bp); - ufsj_end_transaction(jnl); - return(0); /* BAW: Correct return value? */ - } else +#ifdef INVARIANTS + panic("Journaling not allowed on sync filesystems"); +#endif + } #endif - return (bwrite(bp)); + return (bwrite(bp)); } else if (vm_page_count_severe() || buf_dirty_count_severe()) { #ifdef UFS_JOURNAL - if (DOINGJOURNAL(vp)) { - ufsj_write_blocks(jnl, bp); - ufsj_end_transaction(jnl); - return(0); /* BAW: Correct return value? */ - } else + if (DOINGJOURNAL(vp)) + ufsj_write_blocks(jtrn, bp); #endif - return (bwrite(bp)); + /* + * XXX This case is meant to slow down the call and ensure + * that buffers are freed by the time it completes. + * We can't satisfy this with journaling enabled because * the buffer will be pinned until the transaction + * ends. We don't know from here when the transaction + * will end because there might be more sub-operations to + * put into it. + * Buffer management really belongs in the layers that + * manage buffers, not in the buffer consumer code. + * This case should be removed and replaced with + * appropriate handling in the journal and buf/VM layers. + */ + return (bwrite(bp)); } else { if (bp->b_bufsize == fs->fs_bsize) bp->b_flags |= B_CLUSTEROK; #ifdef UFS_JOURNAL - if (DOINGJOURNAL(vp)) { - ufsj_write_blocks(jnl, bp); - ufsj_end_transaction(jnl); - } else + if (DOINGJOURNAL(vp)) + ufsj_write_blocks(jtrn, bp); #endif - bdwrite(bp); + bdwrite(bp); return (0); } } @@ -239,7 +239,7 @@ ip->i_din2->di_extb[i] = 0; } ip->i_flag |= IN_CHANGE | IN_UPDATE; - if ((error = ffs_update(vp, 1))) + if ((error = ffs_update(vp, 1, NULL))) return (error); for (i = 0; i < NXADDR; i++) { if (oldblks[i] == 0) @@ -266,13 +266,13 @@ ip->i_flag |= IN_CHANGE | IN_UPDATE; if (needextclean) softdep_setup_freeblocks(ip, length, IO_EXT); - return (ffs_update(vp, 1)); + return (ffs_update(vp, 1, NULL)); } if (ip->i_size == length) { ip->i_flag |= IN_CHANGE | IN_UPDATE; if (needextclean) softdep_setup_freeblocks(ip, length, IO_EXT); - return (ffs_update(vp, 0)); + return (ffs_update(vp, 0, NULL)); } if (fs->fs_ronly) panic("ffs_truncate: read-only filesystem"); @@ -311,7 +311,7 @@ vinvalbuf(vp, needextclean ? 0 : V_NORMAL, td, 0, 0); vnode_pager_setsize(vp, 0); ip->i_flag |= IN_CHANGE | IN_UPDATE; - return (ffs_update(vp, 0)); + return (ffs_update(vp, 0, NULL)); } } osize = ip->i_size; @@ -335,7 +335,7 @@ else bawrite(bp); ip->i_flag |= IN_CHANGE | IN_UPDATE; - return (ffs_update(vp, 1)); + return (ffs_update(vp, 1, NULL)); } /* * Shorten the size of the file. If the file is not being @@ -413,7 +413,7 @@ DIP_SET(ip, i_db[i], 0); } ip->i_flag |= IN_CHANGE | IN_UPDATE; - allerror = ffs_update(vp, 1); + allerror = ffs_update(vp, 1, NULL); /* * Having written the new inode to disk, save its new configuration ==== //depot/projects/soc2005/ufsj/src/sys/ufs/ffs/ffs_softdep.c#2 (text+ko) ==== @@ -4863,7 +4863,7 @@ * then we do the slower ffs_syncvnode of the directory. */ if (flushparent) { - if ((error = ffs_update(pvp, 1)) != 0) { + if ((error = ffs_update(pvp, 1, NULL)) != 0) { vput(pvp); return (error); } @@ -5300,7 +5300,7 @@ */ if (dap->da_state & MKDIR_PARENT) { FREE_LOCK(&lk); - if ((error = ffs_update(pvp, 1)) != 0) + if ((error = ffs_update(pvp, 1, NULL)) != 0) break; ACQUIRE_LOCK(&lk); /* @@ -5457,7 +5457,7 @@ */ if (!(curthread->td_pflags & TDP_COWINPROGRESS)) { UFS_UNLOCK(ump); - error = ffs_update(vp, 1); + error = ffs_update(vp, 1, NULL); UFS_LOCK(ump); if (error != 0) return (0); ==== //depot/projects/soc2005/ufsj/src/sys/ufs/ffs/ffs_vfsops.c#4 (text+ko) ==== ==== //depot/projects/soc2005/ufsj/src/sys/ufs/ffs/ffs_vnops.c#3 (text+ko) ==== @@ -327,7 +327,7 @@ } VI_UNLOCK(vp); splx(s); - return (ffs_update(vp, wait)); + return (ffs_update(vp, wait, NULL)); } static int @@ -744,7 +744,7 @@ uio->uio_resid = resid; } } else if (resid > uio->uio_resid && (ioflag & IO_SYNC)) - error = ffs_update(vp, 1); + error = ffs_update(vp, 1, NULL); return (error); } @@ -1066,7 +1066,7 @@ uio->uio_resid = resid; } } else if (resid > uio->uio_resid && (ioflag & IO_SYNC)) - error = ffs_update(vp, 1); + error = ffs_update(vp, 1, NULL); return (error); } ==== //depot/projects/soc2005/ufsj/src/sys/ufs/ufs/ufs_inode.c#3 (text+ko) ==== @@ -115,7 +115,7 @@ ip->i_flag &= ~IN_ACCESS; } else { (void) vn_write_suspend_wait(vp, NULL, V_WAIT); - UFS_UPDATE(vp, 0); + UFS_UPDATE(vp, 0, NULL); } } out: @@ -153,7 +153,7 @@ vnode_destroy_vobject(vp); if (ip->i_flag & IN_LAZYMOD) { ip->i_flag |= IN_MODIFIED; - UFS_UPDATE(vp, 0); + UFS_UPDATE(vp, 0, NULL); } /* * Remove the inode from its hash chain. ==== //depot/projects/soc2005/ufsj/src/sys/ufs/ufs/ufs_lookup.c#2 (text+ko) ==== @@ -758,7 +758,7 @@ if (softdep_setup_directory_add(bp, dp, dp->i_offset, dirp->d_ino, newdirbp, 1) == 0) { bdwrite(bp); - return (UFS_UPDATE(dvp, 0)); + return (UFS_UPDATE(dvp, 0, NULL)); } /* We have just allocated a directory block in an * indirect block. Rather than tracking when it gets @@ -781,10 +781,10 @@ } if (DOINGASYNC(dvp)) { bdwrite(bp); - return (UFS_UPDATE(dvp, 0)); + return (UFS_UPDATE(dvp, 0, NULL)); } error = bwrite(bp); - ret = UFS_UPDATE(dvp, 1); + ret = UFS_UPDATE(dvp, 1, NULL); if (error == 0) return (ret); return (error); ==== //depot/projects/soc2005/ufsj/src/sys/ufs/ufs/ufs_vnops.c#3 (text+ko) ==== @@ -569,7 +569,7 @@ ip->i_din2->di_birthtime = vap->va_birthtime.tv_sec; ip->i_din2->di_birthnsec = vap->va_birthtime.tv_nsec; } - error = UFS_UPDATE(vp, 0); + error = UFS_UPDATE(vp, 0, NULL); if (error) return (error); } @@ -810,7 +810,7 @@ ip->i_flag |= IN_CHANGE; if (DOINGSOFTDEP(vp)) softdep_change_linkcnt(ip); - error = UFS_UPDATE(vp, !(DOINGSOFTDEP(vp) | DOINGASYNC(vp))); + error = UFS_UPDATE(vp, !(DOINGSOFTDEP(vp) | DOINGASYNC(vp)), NULL); if (!error) { ufs_makedirentry(ip, cnp, &newdir); error = ufs_direnter(tdvp, vp, &newdir, cnp, NULL); @@ -1023,7 +1023,7 @@ if (DOINGSOFTDEP(fvp)) softdep_change_linkcnt(ip); if ((error = UFS_UPDATE(fvp, !(DOINGSOFTDEP(fvp) | - DOINGASYNC(fvp)))) != 0) { + DOINGASYNC(fvp)), NULL)) != 0) { VOP_UNLOCK(fvp, 0, td); goto bad; } @@ -1089,7 +1089,7 @@ if (DOINGSOFTDEP(tdvp)) softdep_change_linkcnt(dp); error = UFS_UPDATE(tdvp, !(DOINGSOFTDEP(tdvp) | - DOINGASYNC(tdvp))); + DOINGASYNC(tdvp)), NULL); if (error) goto bad; } @@ -1103,7 +1103,7 @@ dp->i_flag |= IN_CHANGE; if (DOINGSOFTDEP(tdvp)) softdep_change_linkcnt(dp); - (void)UFS_UPDATE(tdvp, 1); + (void)UFS_UPDATE(tdvp, 1, NULL); } goto bad; } @@ -1472,7 +1472,7 @@ dp->i_flag |= IN_CHANGE; if (DOINGSOFTDEP(dvp)) softdep_change_linkcnt(dp); - error = UFS_UPDATE(tvp, !(DOINGSOFTDEP(dvp) | DOINGASYNC(dvp))); + error = UFS_UPDATE(tvp, !(DOINGSOFTDEP(dvp) | DOINGASYNC(dvp)), NULL); if (error) goto bad; #ifdef MAC @@ -1554,7 +1554,7 @@ } } if ((error = UFS_UPDATE(tvp, !(DOINGSOFTDEP(tvp) | - DOINGASYNC(tvp)))) != 0) { + DOINGASYNC(tvp)), NULL)) != 0) { (void)bwrite(bp); goto bad; } @@ -2304,7 +2304,7 @@ /* * Make sure inode goes to disk before directory entry. */ - error = UFS_UPDATE(tvp, !(DOINGSOFTDEP(tvp) | DOINGASYNC(tvp))); + error = UFS_UPDATE(tvp, !(DOINGSOFTDEP(tvp) | DOINGASYNC(tvp)), NULL); if (error) goto bad; #ifdef MAC ==== //depot/projects/soc2005/ufsj/src/sys/ufs/ufs/ufsmount.h#3 (text+ko) ==== @@ -55,6 +55,7 @@ struct uio; struct vnode; struct ufs_extattr_per_mount; +struct ufsj_transaction; /* This structure describes the UFS specific mount structure data. */ struct ufsmount { @@ -81,7 +82,7 @@ int (*um_balloc)(struct vnode *, off_t, int, struct ucred *, int, struct buf **); int (*um_blkatoff)(struct vnode *, off_t, char **, struct buf **); int (*um_truncate)(struct vnode *, off_t, int, struct ucred *, struct thread *); - int (*um_update)(struct vnode *, int); + int (*um_update)(struct vnode *, int, struct ufsj_transaction *); int (*um_valloc)(struct vnode *, int, struct ucred *, struct vnode **); int (*um_vfree)(struct vnode *, ino_t, int); void (*um_ifree)(struct ufsmount *, struct inode *); @@ -90,7 +91,7 @@ #define UFS_BALLOC(aa, bb, cc, dd, ee, ff) VFSTOUFS((aa)->v_mount)->um_balloc(aa, bb, cc, dd, ee, ff) #define UFS_BLKATOFF(aa, bb, cc, dd) VFSTOUFS((aa)->v_mount)->um_blkatoff(aa, bb, cc, dd) #define UFS_TRUNCATE(aa, bb, cc, dd, ee) VFSTOUFS((aa)->v_mount)->um_truncate(aa, bb, cc, dd, ee) -#define UFS_UPDATE(aa, bb) VFSTOUFS((aa)->v_mount)->um_update(aa, bb) +#define UFS_UPDATE(aa, bb, cc) VFSTOUFS((aa)->v_mount)->um_update(aa, bb, cc) #define UFS_VALLOC(aa, bb, cc, dd) VFSTOUFS((aa)->v_mount)->um_valloc(aa, bb, cc, dd) #define UFS_VFREE(aa, bb, cc) VFSTOUFS((aa)->v_mount)->um_vfree(aa, bb, cc) #define UFS_IFREE(aa, bb) ((aa)->um_ifree(aa, bb))
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200509141031.j8EAVi84048492>