From 61846fc4dcd926558b1acb5bbeb320837e552c05 Mon Sep 17 00:00:00 2001 From: Konstantin Belousov Date: Sat, 14 Nov 2020 05:10:39 +0000 Subject: [PATCH] Add a framework that tracks exclusive vnode lock generation count for UFS. This count is memoized together with the lookup metadata in directory inode, and we assert that accesses to lookup metadata are done under the same lock generation as they were stored. Enabled under DIAGNOSTICS. UFS saves additional data for parent dirent when doing lookup (i_offset, i_count, i_endoff), and this data is used later by VOPs operating on dirents. If parent vnode exclusive lock is dropped and re-acquired between lookup and the VOP call, we corrupt directories. Framework asserts that corruption cannot occur that way, by tracking vnode lock generation counter. Updates to inode dirent members also save the counter, while users compare current and saved counters values. Also, fix a case in ufs_lookup_ino() where i_offset and i_count could be updated under shared lock. It is not a bug on its own since dvp i_offset results from such lookup cannot be used, but it causes false positive in the checker. In collaboration with: pho Reviewed by: mckusick (previous version), markj Tested by: markj (syzkaller), pho Sponsored by: The FreeBSD Foundation Differential revision: https://reviews.freebsd.org/D26136 --- sys/ufs/ffs/ffs_alloc.c | 2 +- sys/ufs/ffs/ffs_softdep.c | 24 ++-- sys/ufs/ffs/ffs_vfsops.c | 3 + sys/ufs/ffs/ffs_vnops.c | 69 +++++++++-- sys/ufs/ufs/inode.h | 47 ++++++++ sys/ufs/ufs/ufs_lookup.c | 239 ++++++++++++++++++++++++++++---------- sys/ufs/ufs/ufs_vnops.c | 8 +- 7 files changed, 305 insertions(+), 87 deletions(-) diff --git a/sys/ufs/ffs/ffs_alloc.c b/sys/ufs/ffs/ffs_alloc.c index 7ad39e63bb1c..be93dd16cd0a 100644 --- a/sys/ufs/ffs/ffs_alloc.c +++ b/sys/ufs/ffs/ffs_alloc.c @@ -3468,7 +3468,7 @@ sysctl_ffs_fsck(SYSCTL_HANDLER_ARGS) break; } dp = VTOI(dvp); - dp->i_offset = 12; /* XXX mastertemplate.dot_reclen */ + SET_I_OFFSET(dp, 12); /* XXX mastertemplate.dot_reclen */ error = ufs_dirrewrite(dp, VTOI(fdvp), (ino_t)cmd.size, DT_DIR, 0); cache_purge(fdvp); diff --git a/sys/ufs/ffs/ffs_softdep.c b/sys/ufs/ffs/ffs_softdep.c index 6b9c6c30629f..3afd0323ccda 100644 --- a/sys/ufs/ffs/ffs_softdep.c +++ b/sys/ufs/ffs/ffs_softdep.c @@ -8764,11 +8764,11 @@ softdep_change_directoryentry_offset(bp, dp, base, oldloc, newloc, entrysize) if (MOUNTEDSUJ(mp)) { flags = DEPALLOC; jmvref = newjmvref(dp, de->d_ino, - dp->i_offset + (oldloc - base), - dp->i_offset + (newloc - base)); + I_OFFSET(dp) + (oldloc - base), + I_OFFSET(dp) + (newloc - base)); } - lbn = lblkno(ump->um_fs, dp->i_offset); - offset = blkoff(ump->um_fs, dp->i_offset); + lbn = lblkno(ump->um_fs, I_OFFSET(dp)); + offset = blkoff(ump->um_fs, I_OFFSET(dp)); oldoffset = offset + (oldloc - base); newoffset = offset + (newloc - base); ACQUIRE_LOCK(ump); @@ -9280,7 +9280,7 @@ newdirrem(bp, dp, ip, isrmdir, prevdirremp) jremref = dotremref = dotdotremref = NULL; if (DOINGSUJ(dvp)) { if (isrmdir) { - jremref = newjremref(dirrem, dp, ip, dp->i_offset, + jremref = newjremref(dirrem, dp, ip, I_OFFSET(dp), ip->i_effnlink + 2); dotremref = newjremref(dirrem, ip, ip, DOT_OFFSET, ip->i_effnlink + 1); @@ -9288,12 +9288,12 @@ newdirrem(bp, dp, ip, isrmdir, prevdirremp) dp->i_effnlink + 1); dotdotremref->jr_state |= MKDIR_PARENT; } else - jremref = newjremref(dirrem, dp, ip, dp->i_offset, + jremref = newjremref(dirrem, dp, ip, I_OFFSET(dp), ip->i_effnlink + 1); } ACQUIRE_LOCK(ump); - lbn = lblkno(ump->um_fs, dp->i_offset); - offset = blkoff(ump->um_fs, dp->i_offset); + lbn = lblkno(ump->um_fs, I_OFFSET(dp)); + offset = blkoff(ump->um_fs, I_OFFSET(dp)); pagedep_lookup(UFSTOVFS(ump), bp, dp->i_number, lbn, DEPALLOC, &pagedep); dirrem->dm_pagedep = pagedep; @@ -9304,7 +9304,7 @@ newdirrem(bp, dp, ip, isrmdir, prevdirremp) * the jremref is preserved for any potential diradd in this * location. This can not coincide with a rmdir. */ - if (dp->i_offset == DOTDOT_OFFSET) { + if (I_OFFSET(dp) == DOTDOT_OFFSET) { if (isrmdir) panic("newdirrem: .. directory change during remove?"); jremref = cancel_mkdir_dotdot(dp, dirrem, jremref); @@ -9405,7 +9405,7 @@ softdep_setup_directory_change(bp, dp, ip, newinum, isrmdir) mp = ITOVFS(dp); ump = VFSTOUFS(mp); - offset = blkoff(ump->um_fs, dp->i_offset); + offset = blkoff(ump->um_fs, I_OFFSET(dp)); KASSERT(MOUNTEDSOFTDEP(mp) != 0, ("softdep_setup_directory_change called on non-softdep filesystem")); @@ -9508,7 +9508,7 @@ softdep_setup_directory_change(bp, dp, ip, newinum, isrmdir) KASSERT(jaddref != NULL && jaddref->ja_parent == dp->i_number, ("softdep_setup_directory_change: bad jaddref %p", jaddref)); - jaddref->ja_diroff = dp->i_offset; + jaddref->ja_diroff = I_OFFSET(dp); jaddref->ja_diradd = dap; LIST_INSERT_HEAD(&pagedep->pd_diraddhd[DIRADDHASH(offset)], dap, da_pdlist); @@ -9527,7 +9527,7 @@ softdep_setup_directory_change(bp, dp, ip, newinum, isrmdir) * committed when need to move the dot and dotdot references to * this new name. */ - if (inodedep->id_mkdiradd && dp->i_offset != DOTDOT_OFFSET) + if (inodedep->id_mkdiradd && I_OFFSET(dp) != DOTDOT_OFFSET) merge_diradd(inodedep, dap); FREE_LOCK(ump); } diff --git a/sys/ufs/ffs/ffs_vfsops.c b/sys/ufs/ffs/ffs_vfsops.c index ac61a9617651..0e275c1c5551 100644 --- a/sys/ufs/ffs/ffs_vfsops.c +++ b/sys/ufs/ffs/ffs_vfsops.c @@ -2001,6 +2001,9 @@ ffs_vgetf(mp, ino, flags, vpp, ffs_flags) ip->i_nextclustercg = -1; ip->i_flag = fs->fs_magic == FS_UFS1_MAGIC ? 0 : IN_UFS2; ip->i_mode = 0; /* ensure error cases below throw away vnode */ +#ifdef DIAGNOSTIC + ufs_init_trackers(ip); +#endif #ifdef QUOTA { int i; diff --git a/sys/ufs/ffs/ffs_vnops.c b/sys/ufs/ffs/ffs_vnops.c index b86266aebde0..bb7a6858c863 100644 --- a/sys/ufs/ffs/ffs_vnops.c +++ b/sys/ufs/ffs/ffs_vnops.c @@ -434,16 +434,20 @@ ffs_lock(ap) struct vop_lock1_args /* { struct vnode *a_vp; int a_flags; - struct thread *a_td; char *file; int line; } */ *ap; { +#if !defined(NO_FFS_SNAPSHOT) || defined(DIAGNOSTIC) + struct vnode *vp = ap->a_vp; +#endif /* !NO_FFS_SNAPSHOT || DIAGNOSTIC */ +#ifdef DIAGNOSTIC + struct inode *ip; +#endif /* DIAGNOSTIC */ + int result; #ifndef NO_FFS_SNAPSHOT - struct vnode *vp; int flags; struct lock *lkp; - int result; /* * Adaptive spinning mixed with SU leads to trouble. use a giant hammer @@ -456,12 +460,11 @@ ffs_lock(ap) case LK_SHARED: case LK_UPGRADE: case LK_EXCLUSIVE: - vp = ap->a_vp; flags = ap->a_flags; for (;;) { #ifdef DEBUG_VFS_LOCKS VNPASS(vp->v_holdcnt != 0, vp); -#endif +#endif /* DEBUG_VFS_LOCKS */ lkp = vp->v_vnlock; result = lockmgr_lock_flags(lkp, flags, &VI_MTX(vp)->lock_object, ap->a_file, ap->a_line); @@ -483,28 +486,67 @@ ffs_lock(ap) flags = (flags & ~LK_TYPE_MASK) | LK_EXCLUSIVE; flags &= ~LK_INTERLOCK; } +#ifdef DIAGNOSTIC + switch (ap->a_flags & LK_TYPE_MASK) { + case LK_UPGRADE: + case LK_EXCLUSIVE: + if (result == 0 && vp->v_vnlock->lk_recurse == 0) { + ip = VTOI(vp); + if (ip != NULL) + ip->i_lock_gen++; + } + } +#endif /* DIAGNOSTIC */ break; default: +#ifdef DIAGNOSTIC + if ((ap->a_flags & LK_TYPE_MASK) == LK_DOWNGRADE) { + ip = VTOI(vp); + if (ip != NULL) + ufs_unlock_tracker(ip); + } +#endif /* DIAGNOSTIC */ result = VOP_LOCK1_APV(&ufs_vnodeops, ap); + break; } - return (result); -#else +#else /* NO_FFS_SNAPSHOT */ /* * See above for an explanation. */ if ((ap->a_flags & LK_NODDLKTREAT) != 0) ap->a_flags |= LK_ADAPTIVE; - return (VOP_LOCK1_APV(&ufs_vnodeops, ap)); -#endif +#ifdef DIAGNOSTIC + if ((ap->a_flags & LK_TYPE_MASK) == LK_DOWNGRADE) { + ip = VTOI(vp); + if (ip != NULL) + ufs_unlock_tracker(ip); + } +#endif /* DIAGNOSTIC */ + result = VOP_LOCK1_APV(&ufs_vnodeops, ap); +#endif /* NO_FFS_SNAPSHOT */ +#ifdef DIAGNOSTIC + switch (ap->a_flags & LK_TYPE_MASK) { + case LK_UPGRADE: + case LK_EXCLUSIVE: + if (result == 0 && vp->v_vnlock->lk_recurse == 0) { + ip = VTOI(vp); + if (ip != NULL) + ip->i_lock_gen++; + } + } +#endif /* DIAGNOSTIC */ + return (result); } #ifdef INVARIANTS static int ffs_unlock_debug(struct vop_unlock_args *ap) { - struct vnode *vp = ap->a_vp; - struct inode *ip = VTOI(vp); + struct vnode *vp; + struct inode *ip; + vp = ap->a_vp; + ip = VTOI(vp); if (ip->i_flag & UFS_INODE_FLAG_LAZY_MASK_ASSERTABLE) { if ((vp->v_mflag & VMP_LAZYLIST) == 0) { VI_LOCK(vp); @@ -514,6 +556,11 @@ ffs_unlock_debug(struct vop_unlock_args *ap) VI_UNLOCK(vp); } } +#ifdef DIAGNOSTIC + if (VOP_ISLOCKED(vp) == LK_EXCLUSIVE && ip != NULL && + vp->v_vnlock->lk_recurse == 0) + ufs_unlock_tracker(ip); +#endif return (VOP_UNLOCK_APV(&ufs_vnodeops, ap)); } #endif diff --git a/sys/ufs/ufs/inode.h b/sys/ufs/ufs/inode.h index 70f7ab5facf7..16db8d6d5cea 100644 --- a/sys/ufs/ufs/inode.h +++ b/sys/ufs/ufs/inode.h @@ -44,12 +44,24 @@ #include #include #include +#ifdef DIAGNOSTIC +#include +#endif /* * This must agree with the definition in . */ #define doff_t int32_t +#ifdef DIAGNOSTIC +struct iown_tracker { + struct thread *tr_owner; + struct stack tr_st; + struct stack tr_unlock; + int tr_gen; +}; +#endif + /* * The inode is used to describe each active (or recently active) file in the * UFS filesystem. It is composed of two types of information. The first part @@ -94,6 +106,12 @@ struct inode { doff_t i_endoff; /* End of useful stuff in directory. */ doff_t i_diroff; /* Offset in dir, where we found last entry. */ doff_t i_offset; /* Offset of free space in directory. */ +#ifdef DIAGNOSTIC + int i_lock_gen; + struct iown_tracker i_count_tracker; + struct iown_tracker i_endoff_tracker; + struct iown_tracker i_offset_tracker; +#endif int i_nextclustercg; /* last cg searched for cluster */ @@ -254,6 +272,35 @@ struct ufid { uint32_t ufid_ino; /* File number (ino). */ uint32_t ufid_gen; /* Generation number. */ }; + +#ifdef DIAGNOSTIC +void ufs_init_trackers(struct inode *ip); +void ufs_unlock_tracker(struct inode *ip); + +doff_t ufs_get_i_offset(struct inode *ip, const char *file, int line); +void ufs_set_i_offset(struct inode *ip, doff_t off, const char *file, int line); +#define I_OFFSET(ip) ufs_get_i_offset(ip, __FILE__, __LINE__) +#define SET_I_OFFSET(ip, off) ufs_set_i_offset(ip, off, __FILE__, __LINE__) + +int32_t ufs_get_i_count(struct inode *ip, const char *file, int line); +void ufs_set_i_count(struct inode *ip, int32_t cnt, const char *file, int line); +#define I_COUNT(ip) ufs_get_i_count(ip, __FILE__, __LINE__) +#define SET_I_COUNT(ip, cnt) ufs_set_i_count(ip, cnt, __FILE__, __LINE__) + +doff_t ufs_get_i_endoff(struct inode *ip, const char *file, int line); +void ufs_set_i_endoff(struct inode *ip, doff_t off, const char *file, int line); +#define I_ENDOFF(ip) ufs_get_i_endoff(ip, __FILE__, __LINE__) +#define SET_I_ENDOFF(ip, off) ufs_set_i_endoff(ip, off, __FILE__, __LINE__) + +#else +#define I_OFFSET(ip) ((ip)->i_offset) +#define SET_I_OFFSET(ip, off) ((ip)->i_offset = (off)) +#define I_COUNT(ip) ((ip)->i_count) +#define SET_I_COUNT(ip, cnt) ((ip)->i_count = cnt) +#define I_ENDOFF(ip) ((ip)->i_endoff) +#define SET_I_ENDOFF(ip, off) ((ip)->i_endoff = off) +#endif + #endif /* _KERNEL */ #endif /* !_UFS_UFS_INODE_H_ */ diff --git a/sys/ufs/ufs/ufs_lookup.c b/sys/ufs/ufs/ufs_lookup.c index ca151037a960..9566e436165c 100644 --- a/sys/ufs/ufs/ufs_lookup.c +++ b/sys/ufs/ufs/ufs_lookup.c @@ -66,6 +66,7 @@ __FBSDID("$FreeBSD$"); #endif #include #include +#include #ifdef DIAGNOSTIC static int dirchk = 1; @@ -504,22 +505,22 @@ ufs_lookup_ino(struct vnode *vdp, struct vnode **vpp, struct componentname *cnp, * dp->i_offset + dp->i_count. */ if (slotstatus == NONE) { - dp->i_offset = roundup2(dp->i_size, DIRBLKSIZ); - dp->i_count = 0; - enduseful = dp->i_offset; + SET_I_OFFSET(dp, roundup2(dp->i_size, DIRBLKSIZ)); + SET_I_COUNT(dp, 0); + enduseful = I_OFFSET(dp); } else if (nameiop == DELETE) { - dp->i_offset = slotoffset; - if ((dp->i_offset & (DIRBLKSIZ - 1)) == 0) - dp->i_count = 0; + SET_I_OFFSET(dp, slotoffset); + if ((I_OFFSET(dp) & (DIRBLKSIZ - 1)) == 0) + SET_I_COUNT(dp, 0); else - dp->i_count = dp->i_offset - prevoff; + SET_I_COUNT(dp, I_OFFSET(dp) - prevoff); } else { - dp->i_offset = slotoffset; - dp->i_count = slotsize; + SET_I_OFFSET(dp, slotoffset); + SET_I_COUNT(dp, slotsize); if (enduseful < slotoffset + slotsize) enduseful = slotoffset + slotsize; } - dp->i_endoff = roundup2(enduseful, DIRBLKSIZ); + SET_I_ENDOFF(dp, roundup2(enduseful, DIRBLKSIZ)); /* * We return with the directory locked, so that * the parameters we set up above will still be @@ -575,24 +576,32 @@ ufs_lookup_ino(struct vnode *vdp, struct vnode **vpp, struct componentname *cnp, if (nameiop == DELETE && (flags & ISLASTCN)) { if (flags & LOCKPARENT) ASSERT_VOP_ELOCKED(vdp, __FUNCTION__); - /* - * Return pointer to current entry in dp->i_offset, - * and distance past previous entry (if there - * is a previous entry in this block) in dp->i_count. - * Save directory inode pointer in ndp->ni_dvp for dirremove(). - * - * Technically we shouldn't be setting these in the - * WANTPARENT case (first lookup in rename()), but any - * lookups that will result in directory changes will - * overwrite these. - */ - dp->i_offset = i_offset; - if ((dp->i_offset & (DIRBLKSIZ - 1)) == 0) - dp->i_count = 0; - else - dp->i_count = dp->i_offset - prevoff; + + if (VOP_ISLOCKED(vdp) == LK_EXCLUSIVE) { + /* + * Return pointer to current entry in + * dp->i_offset, and distance past previous + * entry (if there is a previous entry in this + * block) in dp->i_count. + * + * We shouldn't be setting these in the + * WANTPARENT case (first lookup in rename()), but any + * lookups that will result in directory changes will + * overwrite these. + */ + SET_I_OFFSET(dp, i_offset); + if ((I_OFFSET(dp) & (DIRBLKSIZ - 1)) == 0) + SET_I_COUNT(dp, 0); + else + SET_I_COUNT(dp, I_OFFSET(dp) - prevoff); + } if (dd_ino != NULL) return (0); + + /* + * Save directory inode pointer in ndp->ni_dvp for + * dirremove(). + */ if ((error = VFS_VGET(vdp->v_mount, ino, LK_EXCLUSIVE, &tdp)) != 0) return (error); @@ -629,7 +638,7 @@ ufs_lookup_ino(struct vnode *vdp, struct vnode **vpp, struct componentname *cnp, * Careful about locking second inode. * This can only occur if the target is ".". */ - dp->i_offset = i_offset; + SET_I_OFFSET(dp, i_offset); if (dp->i_number == ino) return (EISDIR); if (dd_ino != NULL) @@ -887,14 +896,14 @@ ufs_direnter(dvp, tvp, dirp, cnp, newdirbp, isrename) dp = VTOI(dvp); newentrysize = DIRSIZ(OFSFMT(dvp), dirp); - if (dp->i_count == 0) { + if (I_COUNT(dp) == 0) { /* * If dp->i_count is 0, then namei could find no * space in the directory. Here, dp->i_offset will * be on a directory block boundary and we will write the * new entry into a fresh block. */ - if (dp->i_offset & (DIRBLKSIZ - 1)) + if (I_OFFSET(dp) & (DIRBLKSIZ - 1)) panic("ufs_direnter: newblk"); flags = BA_CLRBUF; if (!DOINGSOFTDEP(dvp) && !DOINGASYNC(dvp)) @@ -907,28 +916,28 @@ ufs_direnter(dvp, tvp, dirp, cnp, newdirbp, isrename) } #endif old_isize = dp->i_size; - vnode_pager_setsize(dvp, (u_long)dp->i_offset + DIRBLKSIZ); - if ((error = UFS_BALLOC(dvp, (off_t)dp->i_offset, DIRBLKSIZ, + vnode_pager_setsize(dvp, (u_long)I_OFFSET(dp) + DIRBLKSIZ); + if ((error = UFS_BALLOC(dvp, (off_t)I_OFFSET(dp), DIRBLKSIZ, cr, flags, &bp)) != 0) { if (DOINGSOFTDEP(dvp) && newdirbp != NULL) bdwrite(newdirbp); vnode_pager_setsize(dvp, (u_long)old_isize); return (error); } - dp->i_size = dp->i_offset + DIRBLKSIZ; + dp->i_size = I_OFFSET(dp) + DIRBLKSIZ; DIP_SET(dp, i_size, dp->i_size); - dp->i_endoff = dp->i_size; + SET_I_ENDOFF(dp, dp->i_size); UFS_INODE_SET_FLAG(dp, IN_SIZEMOD | IN_CHANGE | IN_UPDATE); dirp->d_reclen = DIRBLKSIZ; - blkoff = dp->i_offset & + blkoff = I_OFFSET(dp) & (VFSTOUFS(dvp->v_mount)->um_mountp->mnt_stat.f_iosize - 1); bcopy((caddr_t)dirp, (caddr_t)bp->b_data + blkoff,newentrysize); #ifdef UFS_DIRHASH if (dp->i_dirhash != NULL) { - ufsdirhash_newblk(dp, dp->i_offset); - ufsdirhash_add(dp, dirp, dp->i_offset); + ufsdirhash_newblk(dp, I_OFFSET(dp)); + ufsdirhash_add(dp, dirp, I_OFFSET(dp)); ufsdirhash_checkblock(dp, (char *)bp->b_data + blkoff, - dp->i_offset); + I_OFFSET(dp)); } #endif if (DOINGSOFTDEP(dvp)) { @@ -944,7 +953,7 @@ ufs_direnter(dvp, tvp, dirp, cnp, newdirbp, isrename) (bp->b_data + blkoff))->d_reclen = DIRBLKSIZ; blkoff += DIRBLKSIZ; } - if (softdep_setup_directory_add(bp, dp, dp->i_offset, + if (softdep_setup_directory_add(bp, dp, I_OFFSET(dp), dirp->d_ino, newdirbp, 1)) UFS_INODE_SET_FLAG(dp, IN_NEEDSYNC); if (newdirbp) @@ -1001,15 +1010,15 @@ ufs_direnter(dvp, tvp, dirp, cnp, newdirbp, isrename) * * N.B. - THIS IS AN ARTIFACT OF 4.2 AND SHOULD NEVER HAPPEN. */ - if (dp->i_offset + dp->i_count > dp->i_size) { - dp->i_size = dp->i_offset + dp->i_count; + if (I_OFFSET(dp) + I_COUNT(dp) > dp->i_size) { + dp->i_size = I_OFFSET(dp) + I_COUNT(dp); DIP_SET(dp, i_size, dp->i_size); UFS_INODE_SET_FLAG(dp, IN_SIZEMOD | IN_MODIFIED); } /* * Get the block containing the space for the new directory entry. */ - error = UFS_BLKATOFF(dvp, (off_t)dp->i_offset, &dirbuf, &bp); + error = UFS_BLKATOFF(dvp, (off_t)I_OFFSET(dp), &dirbuf, &bp); if (error) { if (DOINGSOFTDEP(dvp) && newdirbp != NULL) bdwrite(newdirbp); @@ -1024,7 +1033,7 @@ ufs_direnter(dvp, tvp, dirp, cnp, newdirbp, isrename) ep = (struct direct *)dirbuf; dsize = ep->d_ino ? DIRSIZ(OFSFMT(dvp), ep) : 0; spacefree = ep->d_reclen - dsize; - for (loc = ep->d_reclen; loc < dp->i_count; ) { + for (loc = ep->d_reclen; loc < I_COUNT(dp); ) { nep = (struct direct *)(dirbuf + loc); /* Trim the existing slot (NB: dsize may be zero). */ @@ -1052,8 +1061,8 @@ ufs_direnter(dvp, tvp, dirp, cnp, newdirbp, isrename) #ifdef UFS_DIRHASH if (dp->i_dirhash != NULL) ufsdirhash_move(dp, nep, - dp->i_offset + ((char *)nep - dirbuf), - dp->i_offset + ((char *)ep - dirbuf)); + I_OFFSET(dp) + ((char *)nep - dirbuf), + I_OFFSET(dp) + ((char *)ep - dirbuf)); #endif if (DOINGSOFTDEP(dvp)) softdep_change_directoryentry_offset(bp, dp, dirbuf, @@ -1094,19 +1103,19 @@ ufs_direnter(dvp, tvp, dirp, cnp, newdirbp, isrename) #ifdef UFS_DIRHASH if (dp->i_dirhash != NULL && (ep->d_ino == 0 || dirp->d_reclen == spacefree)) - ufsdirhash_add(dp, dirp, dp->i_offset + ((char *)ep - dirbuf)); + ufsdirhash_add(dp, dirp, I_OFFSET(dp) + ((char *)ep - dirbuf)); #endif bcopy((caddr_t)dirp, (caddr_t)ep, (u_int)newentrysize); #ifdef UFS_DIRHASH if (dp->i_dirhash != NULL) ufsdirhash_checkblock(dp, dirbuf - - (dp->i_offset & (DIRBLKSIZ - 1)), - rounddown2(dp->i_offset, DIRBLKSIZ)); + (I_OFFSET(dp) & (DIRBLKSIZ - 1)), + rounddown2(I_OFFSET(dp), DIRBLKSIZ)); #endif if (DOINGSOFTDEP(dvp)) { (void) softdep_setup_directory_add(bp, dp, - dp->i_offset + (caddr_t)ep - dirbuf, + I_OFFSET(dp) + (caddr_t)ep - dirbuf, dirp->d_ino, newdirbp, 0); if (newdirbp != NULL) bdwrite(newdirbp); @@ -1128,10 +1137,10 @@ ufs_direnter(dvp, tvp, dirp, cnp, newdirbp, isrename) * lock on the newly entered node. */ if (isrename == 0 && error == 0 && - dp->i_endoff && dp->i_endoff < dp->i_size) { + I_ENDOFF(dp) != 0 && I_ENDOFF(dp) < dp->i_size) { if (tvp != NULL) VOP_UNLOCK(tvp); - error = UFS_TRUNCATE(dvp, (off_t)dp->i_endoff, + error = UFS_TRUNCATE(dvp, (off_t)I_ENDOFF(dp), IO_NORMAL | (DOINGASYNC(dvp) ? 0 : IO_SYNC), cr); if (error != 0) vn_printf(dvp, @@ -1139,7 +1148,7 @@ ufs_direnter(dvp, tvp, dirp, cnp, newdirbp, isrename) error); #ifdef UFS_DIRHASH if (error == 0 && dp->i_dirhash != NULL) - ufsdirhash_dirtrunc(dp, dp->i_endoff); + ufsdirhash_dirtrunc(dp, I_ENDOFF(dp)); #endif error = 0; if (tvp != NULL) @@ -1190,9 +1199,9 @@ ufs_dirremove(dvp, ip, flags, isrmdir) } } if (flags & DOWHITEOUT) - offset = dp->i_offset; + offset = I_OFFSET(dp); else - offset = dp->i_offset - dp->i_count; + offset = I_OFFSET(dp) - I_COUNT(dp); if ((error = UFS_BLKATOFF(dvp, offset, (char **)&ep, &bp)) != 0) { if (ip) { ip->i_effnlink++; @@ -1216,7 +1225,7 @@ ufs_dirremove(dvp, ip, flags, isrmdir) goto out; } /* Set 'rep' to the entry being removed. */ - if (dp->i_count == 0) + if (I_COUNT(dp) == 0) rep = ep; else rep = (struct direct *)((char *)ep + ep->d_reclen); @@ -1226,7 +1235,7 @@ ufs_dirremove(dvp, ip, flags, isrmdir) * that `ep' is the previous entry when dp->i_count != 0. */ if (dp->i_dirhash != NULL) - ufsdirhash_remove(dp, rep, dp->i_offset); + ufsdirhash_remove(dp, rep, I_OFFSET(dp)); #endif if (ip && rep->d_ino != ip->i_number) panic("ufs_dirremove: ip %ju does not match dirent ino %ju\n", @@ -1240,7 +1249,7 @@ ufs_dirremove(dvp, ip, flags, isrmdir) rep->d_type = 0; rep->d_ino = 0; - if (dp->i_count != 0) { + if (I_COUNT(dp) != 0) { /* * Collapse new free space into previous entry. */ @@ -1250,8 +1259,8 @@ ufs_dirremove(dvp, ip, flags, isrmdir) #ifdef UFS_DIRHASH if (dp->i_dirhash != NULL) ufsdirhash_checkblock(dp, (char *)ep - - ((dp->i_offset - dp->i_count) & (DIRBLKSIZ - 1)), - rounddown2(dp->i_offset, DIRBLKSIZ)); + ((I_OFFSET(dp) - I_COUNT(dp)) & (DIRBLKSIZ - 1)), + rounddown2(I_OFFSET(dp), DIRBLKSIZ)); #endif out: error = 0; @@ -1313,7 +1322,7 @@ ufs_dirrewrite(dp, oip, newinum, newtype, isrmdir) UFS_INODE_SET_FLAG(oip, IN_CHANGE); } - error = UFS_BLKATOFF(vdp, (off_t)dp->i_offset, (char **)&ep, &bp); + error = UFS_BLKATOFF(vdp, (off_t)I_OFFSET(dp), (char **)&ep, &bp); if (error == 0 && ep->d_namlen == 2 && ep->d_name[1] == '.' && ep->d_name[0] == '.' && ep->d_ino != oip->i_number) { brelse(bp); @@ -1522,3 +1531,115 @@ ufs_checkpath(ino_t source_ino, ino_t parent_ino, struct inode *target, struct u vput(vp); return (error); } + +#ifdef DIAGNOSTIC +static void +ufs_assert_inode_offset_owner(struct inode *ip, struct iown_tracker *tr, + const char *name, const char *file, int line) +{ + char msg[128]; + + snprintf(msg, sizeof(msg), "at %s@%d", file, line); + ASSERT_VOP_ELOCKED(ITOV(ip), msg); + MPASS((ip->i_mode & IFMT) == IFDIR); + if (curthread == tr->tr_owner && ip->i_lock_gen == tr->tr_gen) + return; + printf("locked at\n"); + stack_print(&tr->tr_st); + printf("unlocked at\n"); + stack_print(&tr->tr_unlock); + panic("%s ip %p %jd offset owner %p %d gen %d " + "curthread %p %d gen %d at %s@%d\n", + name, ip, (uintmax_t)ip->i_number, tr->tr_owner, + tr->tr_owner->td_tid, tr->tr_gen, + curthread, curthread->td_tid, ip->i_lock_gen, + file, line); +} + +static void +ufs_set_inode_offset_owner(struct inode *ip, struct iown_tracker *tr, + const char *file, int line) +{ + char msg[128]; + + snprintf(msg, sizeof(msg), "at %s@%d", file, line); + ASSERT_VOP_ELOCKED(ITOV(ip), msg); + MPASS((ip->i_mode & IFMT) == IFDIR); + tr->tr_owner = curthread; + tr->tr_gen = ip->i_lock_gen; + stack_save(&tr->tr_st); +} + +static void +ufs_init_one_tracker(struct iown_tracker *tr) +{ + tr->tr_owner = NULL; + stack_zero(&tr->tr_st); +} + +void +ufs_init_trackers(struct inode *ip) +{ + ufs_init_one_tracker(&ip->i_offset_tracker); + ufs_init_one_tracker(&ip->i_count_tracker); + ufs_init_one_tracker(&ip->i_endoff_tracker); +} + +void +ufs_unlock_tracker(struct inode *ip) +{ + if (ip->i_count_tracker.tr_gen == ip->i_lock_gen) + stack_save(&ip->i_count_tracker.tr_unlock); + if (ip->i_offset_tracker.tr_gen == ip->i_lock_gen) + stack_save(&ip->i_offset_tracker.tr_unlock); + if (ip->i_endoff_tracker.tr_gen == ip->i_lock_gen) + stack_save(&ip->i_endoff_tracker.tr_unlock); + ip->i_lock_gen++; +} + +doff_t +ufs_get_i_offset(struct inode *ip, const char *file, int line) +{ + ufs_assert_inode_offset_owner(ip, &ip->i_offset_tracker, "i_offset", + file, line); + return (ip->i_offset); +} + +void +ufs_set_i_offset(struct inode *ip, doff_t off, const char *file, int line) +{ + ufs_set_inode_offset_owner(ip, &ip->i_offset_tracker, file, line); + ip->i_offset = off; +} + +int32_t +ufs_get_i_count(struct inode *ip, const char *file, int line) +{ + ufs_assert_inode_offset_owner(ip, &ip->i_count_tracker, "i_count", + file, line); + return (ip->i_count); +} + +void +ufs_set_i_count(struct inode *ip, int32_t cnt, const char *file, int line) +{ + ufs_set_inode_offset_owner(ip, &ip->i_count_tracker, file, line); + ip->i_count = cnt; +} + +doff_t +ufs_get_i_endoff(struct inode *ip, const char *file, int line) +{ + ufs_assert_inode_offset_owner(ip, &ip->i_endoff_tracker, "i_endoff", + file, line); + return (ip->i_endoff); +} + +void +ufs_set_i_endoff(struct inode *ip, doff_t off, const char *file, int line) +{ + ufs_set_inode_offset_owner(ip, &ip->i_endoff_tracker, file, line); + ip->i_endoff = off; +} + +#endif diff --git a/sys/ufs/ufs/ufs_vnops.c b/sys/ufs/ufs/ufs_vnops.c index 3034cbd24077..4d241bd53eea 100644 --- a/sys/ufs/ufs/ufs_vnops.c +++ b/sys/ufs/ufs/ufs_vnops.c @@ -1481,9 +1481,9 @@ ufs_rename(ap) if (error) goto bad; /* Setup tdvp for directory compaction if needed. */ - if (tdp->i_count && tdp->i_endoff && - tdp->i_endoff < tdp->i_size) - endoff = tdp->i_endoff; + if (I_COUNT(tdp) != 0 && I_ENDOFF(tdp) != 0 && + I_ENDOFF(tdp) < tdp->i_size) + endoff = I_ENDOFF(tdp); } else { if (ITODEV(tip) != ITODEV(tdp) || ITODEV(tip) != ITODEV(fip)) panic("ufs_rename: EXDEV"); @@ -1611,7 +1611,7 @@ ufs_rename(ap) } else if (DOINGSUJ(tdvp)) /* Journal must account for each new link. */ softdep_setup_dotdot_link(tdp, fip); - fip->i_offset = mastertemplate.dot_reclen; + SET_I_OFFSET(fip, mastertemplate.dot_reclen); ufs_dirrewrite(fip, fdp, newparent, DT_DIR, 0); cache_purge(fdvp); }