From owner-svn-src-head@FreeBSD.ORG Wed Aug 28 10:19:54 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id D74F1E99; Wed, 28 Aug 2013 10:19:53 +0000 (UTC) (envelope-from davide.italiano@gmail.com) Received: from mail-vb0-x22e.google.com (mail-vb0-x22e.google.com [IPv6:2607:f8b0:400c:c02::22e]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 2427D2B1F; Wed, 28 Aug 2013 10:19:53 +0000 (UTC) Received: by mail-vb0-f46.google.com with SMTP id p13so3826305vbe.5 for ; Wed, 28 Aug 2013 03:19:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=EaaZQmuqCUbXO31DMYhiovyEjIUkNwZ7B5p9jdUPgUA=; b=xnZZPOZ1BIq172SZh+5Q5fa4lEJ5DT2T61Vdbj5b2vzJNf8YV+Ut52ehOSTPWlKrq6 qSMi6Txp0ox27jHQp9xl7axgRgQMDNIBFlw0vmeOIf4rkc6LeQONca1ft64AY444LgOY jftNQdOD4x1Gnrt/W+JB9IqjlaWIgfwF55cywLgRGwaPY1TWEhZQ39wsNz8m8h1TRl0D WTQJ/oj1EO6GSquJWOj7V8kwK4wEAbdPX1jcsmdWVZVVutcqDlHS88sbdxNXzo1uYSsu fsBbTgThCGcfOG7kYwPCvzjAUzlQEfYlvD/nuvKCxGSPucC4dEf1yqhQXCJKp7z0p46v IEOQ== MIME-Version: 1.0 X-Received: by 10.52.103.35 with SMTP id ft3mr21340731vdb.5.1377685192111; Wed, 28 Aug 2013 03:19:52 -0700 (PDT) Sender: davide.italiano@gmail.com Received: by 10.220.65.132 with HTTP; Wed, 28 Aug 2013 03:19:52 -0700 (PDT) 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> <521D4C41.3060208@delphij.net> Date: Wed, 28 Aug 2013 12:19:52 +0200 X-Google-Sender-Auth: 6kOTWUp--gDN3dKr-LAaFHjdFNo Message-ID: Subject: Re: svn commit: r254585 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs From: Davide Italiano To: Xin LI Content-Type: text/plain; charset=ISO-8859-1 Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, Andriy Gapon , Konstantin Belousov , svn-src-head@freebsd.org, Xin LI X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 28 Aug 2013 10:19:54 -0000 On Wed, Aug 28, 2013 at 3:02 AM, Xin Li 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 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