On Tue, May 6, 2014 at 5:37 PM, Jorgen Lundman <[email protected]> wrote:
> > Hello masters, > > > So I have a small question about zfs_ctldir.c's holds being leaked in error > cases. > > Basically, in this case here: > > > https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/fs/zfs/zfs_ctldir.c#L813 > > if (gfs_lookup_dot(vpp, dvp, zfsvfs->z_ctldir, nm) == 0) { > ZFS_EXIT(zfsvfs); > return (0); > } > > > I believe gfs_lookup_dot() will call VN_HOLD on one of the cases, and > return the vnode in "vpp". (If it doesn't call VN_HOLD it returns error, > and we bail here). > You may be misreading this code. If gfs_lookup_dot() is successful, the vnode is held and we return immediately from the code you quoted. If there was an error (e.g. the name is not "." or ".."), we continue in this funciton, and *vpp is not a held vnode. > > Then further down, we have four cases which can exit; > > > https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/fs/zfs/zfs_ctldir.c#L826 no vnode is held here > > > https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/fs/zfs/zfs_ctldir.c#L860 This should do the right thing; if err==0 then the vnode is held, otherwise it is not (based on traverse()). > > > https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/fs/zfs/zfs_ctldir.c#L870 no vnode is held here > > > https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/fs/zfs/zfs_ctldir.c#L880 no vnode is held here. > > > For example, the last case which OsX branch hits quite often. > > if (dmu_objset_hold(snapname, FTAG, &snap) != 0) { > mutex_exit(&sdp->sd_lock); > ZFS_EXIT(zfsvfs); > + VN_RELE(*vpp); > return (SET_ERROR(ENOENT)); > } > > This happens because if you have ".zfs/snapshot/send" then Darwin kernel > will look for "._send" as well. (apple double file, for xattr etc). > > I need to call VN_HOLD(*vpp) Do you mean VN_RELE()? > here to let go of the "gfs_lookup_dot()" hold. > There is no hold from gfs_lookup_dot() here. If it put a hold, we returned on line 815. > But why do upstream get away with it? Or am I missing something, or a > difference between the platforms? Darwin is very keen to panic if the > iocounts are off. > At the very end of the function, if there was an error, you do release it > at; > > https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/fs/zfs/zfs_ctldir.c#L942 That's releasing the hold from zfsctl_snapshot_mknode() if domount() fails. --matt > > > Lund > > -- > Jorgen Lundman | <[email protected]> > Unix Administrator | +81 (0)3 -5456-2687 ext 1017 (work) > Shibuya-ku, Tokyo | +81 (0)90-5578-8500 (cell) > Japan | +81 (0)3 -3375-1767 (home) > _______________________________________________ > developer mailing list > [email protected] > http://lists.open-zfs.org/mailman/listinfo/developer >
_______________________________________________ developer mailing list [email protected] http://lists.open-zfs.org/mailman/listinfo/developer
