From owner-freebsd-current@freebsd.org Sat Feb 27 16:01:16 2021 Return-Path: Delivered-To: freebsd-current@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 3805954B1E8; Sat, 27 Feb 2021 16:01:16 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4DnrqH4XgQz3HvL; Sat, 27 Feb 2021 16:01:15 +0000 (UTC) (envelope-from avg@FreeBSD.org) X-Originating-IP: 195.64.148.76 Received: from [192.168.0.88] (unknown [195.64.148.76]) (Authenticated sender: andriy.gapon@uabsd.com) by relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 5AE60FF804; Sat, 27 Feb 2021 16:01:12 +0000 (UTC) Subject: Re: panic: condition seqc_in_modify(_vp->v_seqc) not met at zfs_acl.c:1147 (zfs_acl_chown_setattr) To: Mateusz Guzik Cc: FreeBSD Current , freebsd-fs@freebsd.org References: <1b261679-3492-dda8-614e-21150a2375d8@FreeBSD.org> <038304d0-2cd0-f089-56ee-c094cdc4cc21@FreeBSD.org> <8f944450-734d-e70b-c1fb-10d2239576e2@FreeBSD.org> From: Andriy Gapon Message-ID: <7f377610-c1fe-1658-4cb5-72b8f60cea36@FreeBSD.org> Date: Sat, 27 Feb 2021 18:01:11 +0200 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:78.0) Gecko/20100101 Firefox/78.0 Thunderbird/78.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 4DnrqH4XgQz3HvL X-Spamd-Bar: / Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [0.00 / 15.00]; ASN(0.00)[asn:29169, ipnet:217.70.176.0/20, country:FR]; local_wl_from(0.00)[FreeBSD.org] X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 27 Feb 2021 16:01:16 -0000 On 16/02/2021 22:38, Mateusz Guzik wrote: > I think for future proofing it would be best if all vnodes going there > had seqc marked, thus I think this should do the trick: > > diff --git a/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_vnops_os.c > b/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_vnops_os.c > index d5f0da9ecd4b..8172916c4329 100644 > --- a/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_vnops_os.c > +++ b/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_vnops_os.c > @@ -2756,7 +2756,9 @@ zfs_setattr(znode_t *zp, vattr_t *vap, int > flags, cred_t *cr) > err = zfs_acl_chown_setattr(zp); > ASSERT(err == 0); > if (attrzp) { > + vn_seqc_write_begin(ZTOV(attrzp)); > err = zfs_acl_chown_setattr(attrzp); > + vn_seqc_write_end(ZTOV(attrzp)); > ASSERT(err == 0); > } > } > > I don't see other calls to the routine. This patch works perfectly for me. Thank you! > On 2/16/21, Andriy Gapon wrote: >> On 15/02/2021 11:45, Andriy Gapon wrote: >>> On 15/02/2021 10:22, Andriy Gapon wrote: >>>> >>>> I've got this panic once when copying a couple of files. >>>> The system is stable/13 as of 1996360d7338d, a custom kernel >>>> configuration, but >>>> no local source code modifications. >>>> >>>> Unread portion of the kernel message buffer: >>>> VNASSERT failed: ({ seqc_t __seqc = (_vp->v_seqc); >>>> __builtin_expect((__seqc & >>>> 1), 0); }) not true at >>>> /usr/devel/git/trant/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_acl.c:1147 >>>> (zfs_acl_chown_setattr) >>>> 0xfffff8013e4e85b8: type VDIR >>>> usecount 1, writecount 0, refcount 1 seqc users 0 mountedhere 0 >>>> hold count flags () >>>> flags () >>>> lock type zfs: EXCL by thread 0xfffffe01dd1cd560 (pid 30747, >>>> kdeinit5, tid >>>> 159911) >>>> panic: condition seqc_in_modify(_vp->v_seqc) not met at >>>> /usr/devel/git/trant/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_acl.c:1147 >>>> (zfs_acl_chown_setattr) >>>> >>>> Any ideas, suggestions, hints? >>>> Thanks! >>>> >>> ... >>>> #4 0xffffffff8036fd21 in zfs_acl_chown_setattr (zp=0xfffff801ccd203b0) >>>> at >>>> /usr/devel/git/trant/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_acl.c:1147 >>>> #5 0xffffffff8037e52d in zfs_setattr (zp=0xfffff8024b04f760, >>>> vap=vap@entry=0xfffffe029a36c870, flags=flags@entry=0, >>>> cr=, cr@entry=0xfffff8003ecedc00) >>>> at >>>> /usr/devel/git/trant/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_vnops_os.c:2758 >>> >>> So, this is actually the second zfs_acl_chown_setattr call here: >>> err = zfs_acl_chown_setattr(zp); >>> ASSERT(err == 0); >>> if (attrzp) { >>> err = zfs_acl_chown_setattr(attrzp); >>> ASSERT(err == 0); >>> } >>> >>> I am not sure if the assertion is actually applicable to attrzp (extended >>> attributes "directory"). >>> At least I do not see any seq calls for it. >>> >> >> So, I think that the problem should be reproducible by simply chown-ing a >> file >> with an extended attribute. The kernel should be compiled with both >> DEBUG_VFS_LOCKS and INVARIANTS. >> >> -- >> Andriy Gapon >> > > -- Andriy Gapon