From owner-freebsd-current Sat Aug 8 12:09:34 1998 Return-Path: Received: (from majordom@localhost) by hub.freebsd.org (8.8.8/8.8.8) id MAA14146 for freebsd-current-outgoing; Sat, 8 Aug 1998 12:09:34 -0700 (PDT) (envelope-from owner-freebsd-current@FreeBSD.ORG) Received: from flamingo.McKusick.COM (flamingo.mckusick.com [209.31.233.178]) by hub.freebsd.org (8.8.8/8.8.8) with ESMTP id MAA14128 for ; Sat, 8 Aug 1998 12:09:21 -0700 (PDT) (envelope-from mckusick@flamingo.McKusick.COM) Received: from flamingo.McKusick.COM (mckusick@localhost [127.0.0.1]) by flamingo.McKusick.COM (8.8.5/8.8.5) with ESMTP id LAA06087; Sat, 8 Aug 1998 11:52:34 -0700 (PDT) Message-Id: <199808081852.LAA06087@flamingo.McKusick.COM> To: Julian Elischer Subject: Softupdates panic (fwd) cc: Luoqi Chen , kkennawa@physics.adelaide.edu.au, dg@root.com, current@FreeBSD.ORG Date: Sat, 08 Aug 1998 11:52:23 -0700 From: Kirk McKusick Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG The problem occured when renaming a directory to a new parent, but not when renaming a directory within the same parent (write a one line C program that does ``rename("d1", "d2");''). Luoqi Chen's fix (forwarded message below) for the broken case, breaks the same directory rename case. Thus, the soft update code needs to be aware of which case it is handling. I now pass this information through and do the right thing for both cases. Affected files are /sys/ufs/ufs/ufs_vnops.c and /sys/ufs/ffs/ffs_softdep.c. Note there is a related one line fix if ffs_softdep.c which sometimes caused the rename to fail with an EINVAL error (compounding the difficulty of trying to track down this bug). The diffs follow. Kirk McKusick Index: ffs_softdep.c =================================================================== RCS file: /master/sys/ufs/ffs/ffs_softdep.c,v retrieving revision 2.21 diff -c -r2.21 ffs_softdep.c *** ffs_softdep.c 1998/06/12 17:54:33 2.21 --- ffs_softdep.c 1998/08/08 18:32:36 *************** *** 52,58 **** * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * ! * @(#)ffs_softdep.c 9.27 (McKusick) 6/12/98 */ /* --- 52,58 ---- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * ! * @(#)ffs_softdep.c 9.28 (McKusick) 8/8/98 */ /* *************** *** 2205,2210 **** --- 2205,2211 ---- } else { dirrem = dap->da_previous; pagedep = dirrem->dm_pagedep; + dirrem->dm_dirinum = pagedep->pd_ino; add_to_worklist(&dirrem->dm_list); } if (inodedep_lookup(VFSTOUFS(pagedep->pd_mnt)->um_fs, dap->da_newinum, *************** *** 2384,2389 **** --- 2385,2404 ---- */ dirrem = newdirrem(bp, dp, ip, isrmdir); pagedep = dirrem->dm_pagedep; + /* + * The possible values for isrmdir: + * 0 - non-directory file rename + * 1 - directory rename within same directory + * inum - directory rename to new directory of given inode number + * When renaming to a new directory, we are both deleting and + * creating a new directory entry, so the link count on the new + * directory should not change. Thus we do not need the followup + * dirrem which is usually done in handle_workitem_remove. We set + * the DIRCHG flag to tell handle_workitem_remove to skip the + * followup dirrem. + */ + if (isrmdir > 1) + dirrem->dm_state |= DIRCHG; /* * Whiteouts have no additional dependencies, *************** *** 2493,2498 **** --- 2508,2523 ---- ip->i_flag |= IN_CHANGE; if ((error = VOP_TRUNCATE(vp, (off_t)0, 0, p->p_ucred, p)) != 0) softdep_error("handle_workitem_remove: truncate", error); + /* + * Rename a directory to a new parent. Since, we are both deleting + * and creating a new directory entry, the link count on the new + * directory should not change. Thus we skip the followup dirrem. + */ + if (dirrem->dm_state & DIRCHG) { + vput(vp); + WORKITEM_FREE(dirrem, D_DIRREM); + return; + } ACQUIRE_LOCK(&lk); (void) inodedep_lookup(ip->i_fs, dirrem->dm_oldinum, DEPALLOC, &inodedep); Index: ufs_vnops.c =================================================================== RCS file: /master/sys/ufs/ufs/ufs_vnops.c,v retrieving revision 2.29 diff -c -r2.29 ufs_vnops.c *** ufs_vnops.c 1998/06/08 18:52:11 2.29 --- ufs_vnops.c 1998/08/08 18:20:28 *************** *** 930,936 **** } ip->i_flag |= IN_RENAME; oldparent = dp->i_number; ! doingdirectory++; } /* --- 930,936 ---- } ip->i_flag |= IN_RENAME; oldparent = dp->i_number; ! doingdirectory = 1; } /* *************** *** 1073,1079 **** goto bad; } if ((error = ufs_dirrewrite(dp, xp, ip->i_number, ! IFTODT(ip->i_mode), doingdirectory)) != 0) goto bad; if (doingdirectory) { if (!newparent) { --- 1073,1080 ---- goto bad; } if ((error = ufs_dirrewrite(dp, xp, ip->i_number, ! IFTODT(ip->i_mode), ! newparent ? newparent : doingdirectory)) != 0) goto bad; if (doingdirectory) { if (!newparent) { ---------- Forwarded message ---------- Date: Fri, 7 Aug 1998 12:09:03 -0400 (EDT) From: Luoqi Chen To: julian@whistle.com, kkennawa@physics.adelaide.edu.au Cc: current@FreeBSD.ORG Subject: Re: Softupdates panic > Well, here's the problem simplified: > > cd /tmp (assuming mounted soft-updates) > mkdir d1 > mkdir d1/d2 > mkdir d2 > mv d2 d1 > rmdir d1/d2 > rmdir d1 > [system will panic in 15 seconds at 'sync' of that data.] > > fix to follow. (and checked in I guess) > > julian > I looked at the problem and figured out what went wrong. During the rename step (mv d2 d1), the target directory d1/d2 is deleted and a dirrem dependency is generated. Normally, a dirrem dependency would decrease the parent's link count by 1 and subdirectory's link count by 2. But for this particular dirrem, we don't want to decrement the parent's link count, otherwise we would the panic above. The solution would be mark this dirrem as such, and don't decrement the parent's link count when it's handled. Attached is a fix. -lq Index: ffs_softdep.c =================================================================== RCS file: /fun/cvs/src/contrib/sys/softupdates/ffs_softdep.c,v retrieving revision 1.12 diff -u -r1.12 ffs_softdep.c --- ffs_softdep.c 1998/06/12 21:21:26 1.12 +++ ffs_softdep.c 1998/08/07 15:37:57 @@ -2436,6 +2436,7 @@ * Allocate a new dirrem and ACQUIRE_LOCK. */ dirrem = newdirrem(bp, dp, ip, isrmdir); + dirrem->dm_state |= DIRCHG; pagedep = dirrem->dm_pagedep; /* @@ -2546,6 +2547,15 @@ ip->i_flag |= IN_CHANGE; if ((error = UFS_TRUNCATE(vp, (off_t)0, 0, p->p_ucred, p)) != 0) softdep_error("handle_workitem_remove: truncate", error); + /* + * Target directory deletion during a directory rename. The + * parent directory's link count doesn't need to be decremented. + */ + if (dirrem->dm_state & DIRCHG) { + vput(vp); + WORKITEM_FREE(dirrem, D_DIRREM); + return; + } ACQUIRE_LOCK(&lk); (void) inodedep_lookup(ip->i_fs, dirrem->dm_oldinum, DEPALLOC, &inodedep); -------- End Forwarded message -------- To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message