Re: UFS snapshot deadlocks

Good sleuthing job. You have correctly analyzed the problem and 
your fix is correct. You have my blessing to check it in. And thanks
for taking the time to figure this out when I did not have the time
to do so.

	Kirk McKusick

=-=-=-=-=-=

Date: Wed, 16 Jun 2004 12:38:18 +0900
From: Jun Kuriyama <kuriyama_at_imgsrc.co.jp>
To: Frode Nordahl <frode_at_nordahl.net>
Cc: Kirk McKusick <mckusick_at_mckusick.com>, freebsd-current_at_freebsd.org
Subject: Re: UFS snapshot deadlocks
X-ASK-Info: Whitelist match

At Fri, 4 Jun 2004 15:06:55 +0000 (UTC),
Frode Nordahl wrote:
> tty1:
> while (1)
> 	ls -la /usr/.snap
> end
> 
> tty2:
> dump -0af /some/where -C 32 -L /dev/ad0s1f

I did diagnose about this.

L1. ls(1) issues lstat(2) during walking inside directory.
L2. lstat(2) calls namei().
L3. namei() tries to lock VREG /usr/.snap/dump_snapshot after locking
    VDIR /usr/.snap.

D1. dump(8) calls mount(2) via mksnap_ffs(8).
D2. mount(2) calls ffs_snapshot().
D3. ffs_snapshot() locks /usr/.snap/dump_snapshot after VOP_CREATE().
D4. ffs_snapshot() searches active but unlinked files from
    mnt_nvnodelist.
D5. when it founds /usr/.snap vnode, tries to lock it.

My question is:

1. should ordering of such locks be parent dir vnode first, and file
   in that dir vnode second?
2. is comparing vnode pointer like this to skip /usr/.snap safe?

Index: ffs_snapshot.c
===================================================================
RCS file: /home/ncvs/src/sys/ufs/ffs/ffs_snapshot.c,v
retrieving revision 1.81
diff -u -r1.81 ffs_snapshot.c
--- ffs_snapshot.c	16 Jun 2004 00:26:30 -0000	1.81
+++ ffs_snapshot.c	16 Jun 2004 03:34:44 -0000
_at__at_ -424,6 +424,11 _at__at_
 			MNT_ILOCK(mp);
 			continue;
 		}
+		if (xvp == nd.ni_dvp) {
+			VI_UNLOCK(xvp);
+			MNT_ILOCK(mp);
+			continue;
+		}
 		if (vn_lock(xvp, LK_EXCLUSIVE | LK_INTERLOCK, td) != 0) {
 			MNT_ILOCK(mp);
 			goto loop;

I did repeated test like as Frode did, but I cannot see any deadlocks
after this patch.


-- 
Jun Kuriyama <kuriyama_at_imgsrc.co.jp> // IMG SRC, Inc.
             <kuriyama_at_FreeBSD.org> // FreeBSD Project