Date: Wed, 28 Aug 2013 12:19:52 +0200 From: Davide Italiano <davide@freebsd.org> To: Xin LI <d@delphij.net> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, Andriy Gapon <avg@freebsd.org>, Konstantin Belousov <kib@freebsd.org>, svn-src-head@freebsd.org, Xin LI <delphij@freebsd.org> Subject: Re: svn commit: r254585 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs Message-ID: <CACYV=-HT_XfS9hOaPcJ-bv0-TkMNCXuQHWNk8MO7ZGQbkv%2BHeQ@mail.gmail.com> In-Reply-To: <521D4C41.3060208@delphij.net> References: <201308202231.r7KMVERi068300@svn.freebsd.org> <20130825221517.GM24767@caravan.chchile.org> <521B75CE.70103@FreeBSD.org> <521BDEAC.9080909@delphij.net> <521C5CAC.2060400@FreeBSD.org> <CACYV=-G0i3%2BU3ubWnPSGQpEvKsuJShYJZOMvKQnjBA1aLftJsA@mail.gmail.com> <521D4C41.3060208@delphij.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Aug 28, 2013 at 3:02 AM, Xin Li <delphij@delphij.net> wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA512 [snip] > > On 08/27/13 01:59, Davide Italiano wrote: >> I've written a patch that attempts to fix the second problem. >> http://people.freebsd.org/~davide/review/zfsrename_recycle.diff The >> culprit is that we're dereferencing sdvp without the vnode lock >> held, so data-stability is not guaranteed. tdvp, instead, is locked >> at the entry point of VOP_RENAME() (at least according to >> vop_rename_pre()) so it can be safely used. As pointed out by >> Andriy ZFS has some different mechanisms wrt other file systems >> that allow the vnode to not be recycled when it's referenced >> (ZFS_ENTER), so once we get zfsvfs_t * from tdzp we can call >> ZFS_ENTER and dereference sdvp in a safe manner. > > I'm not sure that this is right. Now we have: > > tdzp = VTOZ(tdvp); > ZFS_VERIFY_ZP(tdzp); > zfsvfs = tdzp->z_zfsvfs; > ZFS_ENTER(zfsvfs); // tdzp's z_zfsvfs entered > zilog = zfsvfs->z_log; > sdzp = VTOZ(sdvp); > ZFS_VERIFY_ZP(sdzp); // (*) Well, if your concern is that the running thread can exit from a different context than the one it entered, maybe partly inlining ZFS_VERIFY_ZP() might workaround the problem. if (sdzp->z_sa_hdl == NULL) { ZFS_EXIT(zfsvfs); /* this is the right context */ return (EIO); } Does this make sense for you? If not, can you propose an alternative? I'll be away for a couple of days but I will be happy to discuss this further when I'll come back. > > Note that in the (*) step, when sdzp is invalid and sdzp have > different z_zfsvfs than tdzp (for instance when the node is in the > snapshot directory; the code later would test this), we could end up > with ZFS_EXIT()'ing the wrong z_zfsvfs. > > Cheers, > - -- > Xin LI <delphij@delphij.net> https://www.delphij.net/ > FreeBSD - The Power to Serve! Live free or die > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v2.0.21 (FreeBSD) > > iQEcBAEBCgAGBQJSHUxBAAoJEG80Jeu8UPuzkzEIAKXlfGuTodrPcPkDkxBhhaOj > QoxoT06jFwMTplysICuCpslQNyyG2Jxq654u9nMh6q5dww370eTtm2FJ0n2QTxk4 > JeWLpZVUu7VHWNXYVJQqrmATaMFHO4wVf5AYpSHn+1iCWo0kQn1MPxPw/oSUt0yw > 1628jhWTs8n+rxQaYrYN6ewXYeylMwC50ZB0kE/gQpQZ+cKAGmrM/8tur24SQBEo > WwrHakrv1DGJ8rv3Q53FB2iUZ+zEAZs6MJ28w32lV3vOI20jfQbEK+RR7i7HvxdV > c/7M96wCd0X4iEqZb87VVfJFSRdQsXy/35scXhhh/BSviT7rervS8XxPGhfpXOs= > =MzwT > -----END PGP SIGNATURE----- Thanks, -- Davide "There are no solved problems; there are only problems that are more or less solved" -- Henri Poincare
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACYV=-HT_XfS9hOaPcJ-bv0-TkMNCXuQHWNk8MO7ZGQbkv%2BHeQ>