Re: panic: LK_RETRY set with incompatible flags

Konstantin Belousov wrote:
> On Sat, Feb 02, 2013 at 09:30:39PM -0500, Rick Macklem wrote:
> > Andriy Gapon wrote:
> > > on 31/01/2013 15:29 Sergey Kandaurov said the following:
> > > > Hi.
> > > >
> > > > Got this assertion on idle NFS server while `ls -la
> > > > /.zfs/shares/'
> > > > issued on NFS client.
Ok, here's a snippet of zfs_dirlook() from zfs_dir.c:
388 	} else if (name[0] == '.' && name[1] == '.' && name[2] == 0) {
389 	          zfsvfs_t *zfsvfs = dzp->z_zfsvfs;
390 	
391               /*
392 	           * If we are a snapshot mounted under .zfs, return
393 	           * the vp for the snapshot directory.
394 	           */
395 	if ((error = sa_lookup(dzp->z_sa_hdl,
396 	   SA_ZPL_PARENT(zfsvfs), &parent, sizeof (parent))) != 0)
397 	          return (error);
398 	if (parent == dzp->z_id && zfsvfs->z_parent != zfsvfs) {
399 	          error = zfsctl_root_lookup(zfsvfs->z_parent->z_ctldir,
400 	              "snapshot", vpp, NULL, 0, NULL, kcred,
401 	              NULL, NULL, NULL);
402 	          return (error);
403 	} 

Just reading the comment, I don't know what it is referring to by
"snapshot directory". Would that be "shares" for Sergey's case?

It seems too obvious, but maybe the lookup of ".." is returning the
vnode for /.zfs/shares for this case?

I don't know why this wouldn't have shown up before, but maybe it usually
replies from its cache (I think zfs_lookup() calls it a "fast path").

It might still be interesting to replace zfs_vnops.c line# 1451 with:
      if ((cnp->cn_flags & ISDOTDOT) && *vpp != dvp)
and see what happens?

rick

> > > > kern/vfs_vnops.c:_vn_lock()
> > > >                 KASSERT((flags & LK_RETRY) == 0 || error == 0,
> > > >                     ("LK_RETRY set with incompatible flags
> > > >                     (0x%x) or
> > > > an error occured (%d)",
> > > >
> > > > panic: LK_RETRY set with incompatible flags (0x200400) or an
> > > > error
> > > > occured (11)
> > > >
> > > > What does that mean and how is it possible? As you can see, both
> > > > parts
> > > > of assertion failed.
> > > > 11 is EDEADLK
> > > > 0x200400: LK_RETRY & LK_UPGRADE
> > >
> > > LK_SHARED, not LK_UPGRADE.
> > > Apparently the thread already holds an exlusive lock on the vnode,
> > > which you
> > > confirm below.
> > >
> > >
> > > > Tracing pid 2943 tid 101532 td 0xfffffe004f5b7000
> > > > kdb_enter() at kdb_enter+0x3e/frame 0xffffff848e45ef50
> > > > vpanic() at vpanic+0x147/frame 0xffffff848e45ef90
> > > > kassert_panic() at kassert_panic+0x136/frame 0xffffff848e45f000
> > > > _vn_lock() at _vn_lock+0x70/frame 0xffffff848e45f070
> > > > zfs_lookup() at zfs_lookup+0x392/frame 0xffffff848e45f100
> > > > zfs_freebsd_lookup() at zfs_freebsd_lookup+0x6d/frame
> > > > 0xffffff848e45f240
> > > > VOP_CACHEDLOOKUP_APV() at VOP_CACHEDLOOKUP_APV+0xc2/frame
> > > > 0xffffff848e45f260
> > > > vfs_cache_lookup() at vfs_cache_lookup+0xcf/frame
> > > > 0xffffff848e45f2b0
> > > > VOP_LOOKUP_APV() at VOP_LOOKUP_APV+0xc2/frame 0xffffff848e45f2d0
> > > > lookup() at lookup+0x548/frame 0xffffff848e45f350
> > > > nfsvno_namei() at nfsvno_namei+0x1a5/frame 0xffffff848e45f400
> > > > nfsrvd_lookup() at nfsrvd_lookup+0x13a/frame 0xffffff848e45f6b0
> > > > nfsrvd_dorpc() at nfsrvd_dorpc+0xca5/frame 0xffffff848e45f8a0
> > > > nfssvc_program() at nfssvc_program+0x482/frame
> > > > 0xffffff848e45fa00
> > > > svc_run_internal() at svc_run_internal+0x1e9/frame
> > > > 0xffffff848e45fba0
> > > > svc_thread_start() at svc_thread_start+0xb/frame
> > > > 0xffffff848e45fbb0
> > > > fork_exit() at fork_exit+0x84/frame 0xffffff848e45fbf0
> > > > fork_trampoline() at fork_trampoline+0xe/frame
> > > > 0xffffff848e45fbf0
> > > > --- trap 0xc, rip = 0x800883e9a, rsp = 0x7fffffffd488, rbp =
> > > > 0x7fffffffd730 ---
> > > >
> > > > db> show lockedvnods
> > > > Locked vnodes
> > > >
> > > > 0xfffffe02e21b11d8: tag zfs, type VDIR
> > > >     usecount 4, writecount 0, refcount 4 mountedhere 0
> > > >     flags (VI_ACTIVE)
> > > >     v_object 0xfffffe02d9f2eb40 ref 0 pages 0
> > > >     lock type zfs: EXCL by thread 0xfffffe004f5b7000 (pid 2943,
> > > >     nfsd,
> > > > tid 101532)
> > > >
> > > >
> > > >
> > I just took a look at zfs_vnops.c and I am probably missing
> > something,
> > but I can't see how this ever worked for a lookup of ".." when at
> > the
> > root (unless ZFS doesn't do the ".." is the current directory when
> > at
> > the root).
> >
> > Here's the code snippet:
> > 1442 if (error == 0 && (nm[0] != '.' || nm[1] != '\0')) {
> > 1443 int ltype = 0;
> > 1444
> > 1445 if (cnp->cn_flags & ISDOTDOT) {
> > 1446 ltype = VOP_ISLOCKED(dvp);
> > 1447 VOP_UNLOCK(dvp, 0);
> > 1448 }
> > 1449 ZFS_EXIT(zfsvfs);
> > 1450 error = zfs_vnode_lock(*vpp, cnp->cn_lkflags);
> > 1451 if (cnp->cn_flags & ISDOTDOT)
> > 1452 vn_lock(dvp, ltype | LK_RETRY);
> > 1453 if (error != 0) {
> > 1454 VN_RELE(*vpp);
> > 1455 *vpp = NULL;
> > 1456 return (error);
> > 1457 }
> >
> > Maybe line# 1451 should be changed to:
> >         if ((cnp->cn_flags & ISDOTDOT) && *vpp != dvp)
> >
> > I'm not at all familiar with ZFS, so I've probably way
> > off the mark on this, rick
> > ps: I hope kib and jhb don't mind being added as cc's, since
> >     they are familiar with this stuff, although maybe not ZFS
> >     specifics.
> 
> VFS (should) never call VOP_LOOKUP for the dotdot and root vnode.
> The logic in the lookup() prevents it.
> 
> Might it be a missing check in the nfs server code ? (I did not looked
> yet).