acquiring duplicate lock of same type: "vnode interlock"
Bruce Evans
bde at zeta.org.au
Thu Feb 12 11:16:26 PST 2004
On Thu, 12 Feb 2004, Jun Kuriyama wrote:
> Is this patch safe for locking? This may remove warnings below:
Perhaps, but it has some style bugs.
> Index: ffs_snapshot.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/ufs/ffs/ffs_snapshot.c,v
> retrieving revision 1.77
> diff -u -r1.77 ffs_snapshot.c
> --- ffs_snapshot.c 4 Jan 2004 04:08:34 -0000 1.77
> +++ ffs_snapshot.c 12 Feb 2004 01:08:31 -0000
> @@ -488,9 +488,10 @@
> VI_LOCK(devvp);
> snaphead = &devvp->v_rdev->si_snapshots;
> if ((xp = TAILQ_FIRST(snaphead)) != NULL) {
> - VI_LOCK(vp);
> - vp->v_vnlock = ITOV(xp)->v_vnlock;
> + struct lock *lkp = ITOV(xp)->v_vnlock;
(1) Nested declaration.
(2) Initialization in declaration.
(3) No blank line after declaration.
> VI_UNLOCK(devvp);
> + VI_LOCK(vp);
> + vp->v_vnlock = lkp;
> } else {
> struct lock *lkp;
>
However, (1) seems to be a normal style in this file. It is used here in
similar code. But (2) and (3) are not used here.
> @@ -1793,9 +1794,10 @@
> */
> VI_LOCK(devvp);
> if ((xp = TAILQ_FIRST(snaphead)) != NULL) {
> - VI_LOCK(vp);
> - vp->v_vnlock = ITOV(xp)->v_vnlock;
> + struct lock *lkp = ITOV(xp)->v_vnlock;
> VI_UNLOCK(devvp);
> + VI_LOCK(vp);
> + vp->v_vnlock = lkp;
As above.
> } else {
> struct lock *lkp;
>
As above.
The lkp local is now defined nested twice, so (1) is a larger style bug
than before; however, the functions are so large that the style bug is
more in the other direction -- they begin with a large list of declarations
and might benefit from more nested ones. Anyway, following ther nearby
style is never wrong.
Bruce
More information about the freebsd-current
mailing list