repeatable ZFS panic: share->excl
John Baldwin
jhb at FreeBSD.org
Mon Mar 30 08:35:02 PDT 2009
On Sunday 29 March 2009 12:29:13 pm Tim Kientzle wrote:
> Pawel Jakub Dawidek wrote:
> > On Fri, Mar 13, 2009 at 02:08:03PM -0400, John Baldwin wrote:
> >> John Baldwin wrote:
> >>> Yes, I think that is the real bug. Looking at this further I think
> >>> zfs_get_xattrdir() will return the vnode locked if it has to create a
> >>> new node via zfs_make_attrdir() but only returns it held and unlocked if
> >>> it finds an existing one. So my new patch is to just fix
> >>> zfs_get_xattrdir() to unlock the vnode if it creates a new one like so:
> >>>
> >>> (Sorry, TBird is probably going to butcher all the whitespace):
> >>>
> >>> ---
>
>>> //depot/user/jhb/lock/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_dir.c
> >>> +++
>
>>> /Users/jhb/work/p4/lock/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_dir.c
> >>>
> >>> @@ -940,6 +940,7 @@
> >>> /* NB: we already did dmu_tx_wait() if necessary */
> >>> goto top;
> >>> }
> >>> + VOP_UNLOCK(*xvpp, 0);
> >>>
> >>> return (error);
> >>> }
> >>>
> >>> A non-butchered version is at www.FreeBSD.org/~jhb/patches/zfs_ea.patch.
> >> So lulf@ reports success with this patch. Pawel, can you review it?
> >
> > Yes, it works for me too and looks good. The only thing we need to
> > change is to check for error beeing 0 before unlocking the vnode.
> > The zfs_make_xattrdir() function can still return with EIO, so I'd add
> > something like this:
> >
> > if (error == 0)
> > VOP_UNLOCK(*xvpp, 0);
> >
> > Thank you John for spending time on tracking this one down.
>
> Any estimate on when this can be committed?
>
> I'm waiting to re-enable the extended attribute
> archiving support in tar until this is fixed.
Err, it went into the tree a while ago:
Author: jhb
Date: Wed Mar 18 16:19:44 2009
New Revision: 189967
URL: http://svn.freebsd.org/changeset/base/189967
Log:
The zfs_get_xattrdir() function is used to find the extended attribute
directory for a znode. When the directory already exists, it returns a
referenced but unlocked vnode. When a directory does not yet exist, it
calls zfs_make_xattrdir() to create a new one. zfs_make_xattrdir() returns
the vnode both referenced and and locked and zfs_get_xattrdir() was leaking
this vnode lock to its callers. Fix this by dropping the vnode lock if
zfs_make_xattrdir() successfully creates a new extended attribute
directory.
Reviewed by: pjd
--
John Baldwin
More information about the freebsd-current
mailing list