Date: Sat, 08 Aug 1998 11:52:23 -0700 From: Kirk McKusick <mckusick@McKusick.COM> To: Julian Elischer <julian@whistle.com> Cc: Luoqi Chen <luoqi@watermarkgroup.com>, kkennawa@physics.adelaide.edu.au, dg@root.com, current@FreeBSD.ORG Subject: Softupdates panic (fwd) Message-ID: <199808081852.LAA06087@flamingo.McKusick.COM>
next in thread | raw e-mail | index | archive | help
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 <luoqi@watermarkgroup.com>
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199808081852.LAA06087>