Re: panic: condition seqc_in_modify(_vp->v_seqc) not met at zfs_acl.c:1147 (zfs_acl_chown_setattr)

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
> _at__at_ -2756,7 +2756,9 _at__at_ 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 <avg_at_freebsd.org> 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_at_entry=0xfffffe029a36c870, flags=flags_at_entry=0,
>>>>     cr=<optimized out>, cr_at_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