Date: Thu, 16 May 2002 03:06:57 +0700 (NOVST) From: "Semen A. Ustimenko" <semenu@FreeBSD.org> To: Boris Popov <bp@butya.kz> Cc: freebsd-fs@FreeBSD.org, "Flood, Jim" <Jim.Flood@acirro.com> Subject: Re: NULLFS-related possible deadlock + fix proposal Message-ID: <20020516023242.L1992-200000@def.the.net> In-Reply-To: <Pine.BSF.4.21.0205131728290.16895-100000@lion.butya.kz>
index | next in thread | previous in thread | raw e-mail
[-- Attachment #1 --]
Hi!
On Mon, 13 May 2002, Boris Popov wrote:
> On Sat, 11 May 2002, Semen A. Ustimenko wrote:
>
> > DEADLOCK...
>
> Indeed, weird situation. Nice analysis, btw :)
>
It would be weird, if it didn't happen in practice :)
Thanks are going to Jim, not me...
If you like, i can provide a small patch to help you simulate the problem,
but it's kind of hack as NULLFS looks seriously broken in -current.
> > Make vn_lock() in vrele() lock vnode only LK_THISLAYER. Obviously, the
> > NULLFS and other stacking FSes will have to deal with this in their
> > VOP_INACTIVE() handlers. This changes won't touch real FSes as they ignore
> > the LK_THISLAYER, don't they?
>
> Yes, you're correct in that LK_THISLAYER currently used only by
> "stacked" filesystem(s) and it used exactly for such situations to avoid
> deadlocks. The proposed solution may even work without any additional
> code because null_inactive() performs its own management on the lower
> vnode locking.
>
Looks like we can't do this that easily, because VOP_INACTIVE is called
from different places. Most noticeable from vput(), and it's obvious that
vput() caller is responsible for vnode lock...
Any ideas?
BYe!
[-- Attachment #2 --]
Index: null_vnops.c
===================================================================
RCS file: /home/ncvs/src/sys/fs/nullfs/null_vnops.c,v
retrieving revision 1.51
diff -c -r1.51 null_vnops.c
*** null_vnops.c 18 Mar 2002 05:39:04 -0000 1.51
--- null_vnops.c 15 May 2002 19:41:26 -0000
***************
*** 405,410 ****
--- 405,425 ----
*ap->a_vpp = vp;
}
}
+
+ printf("null_lookup %*s error %d\n", (int)cnp->cn_namelen, cnp->cn_nameptr, error);
+ if (error == 0 &&
+ cnp->cn_namelen == 7 && !strncmp("foofile", cnp->cn_nameptr, 7)) {
+ int i;
+
+ printf("Delay...");
+ for (i=5; i; i--) {
+ printf(" %d", i);
+ tsleep(cnp, PPAUSE, "nullbug", hz);
+ }
+ printf("\n");
+ } else
+ printf("No delay\n");
+
return (error);
}
***************
*** 712,717 ****
--- 727,734 ----
{
struct vnode *vp = ap->a_vp;
+ VOP_UNLOCK(vp, 0, ap->a_td);
+
/*
* If this is the last reference, then free up the vnode
* so as not to tie up the lower vnodes.
***************
*** 742,759 ****
LIST_REMOVE(xp, null_hash);
lockmgr(&null_hashlock, LK_RELEASE, NULL, td);
- xp->null_lowervp = NULLVP;
- if (vp->v_vnlock != NULL) {
- vp->v_vnlock = &vp->v_lock; /* we no longer share the lock */
- } else
- VOP_UNLOCK(vp, LK_THISLAYER, td);
-
/*
* Now it is safe to drop references to the lower vnode.
* VOP_INACTIVE() will be called by vrele() if necessary.
*/
! vput(lowervp);
! vrele (lowervp);
vdata = vp->v_data;
vp->v_data = NULL;
--- 759,770 ----
LIST_REMOVE(xp, null_hash);
lockmgr(&null_hashlock, LK_RELEASE, NULL, td);
/*
* Now it is safe to drop references to the lower vnode.
* VOP_INACTIVE() will be called by vrele() if necessary.
*/
! vrele(lowervp);
! vrele(lowervp);
vdata = vp->v_data;
vp->v_data = NULL;
home |
help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020516023242.L1992-200000>