Major issues with nfsv4

[Konstantin Belousov]

On Mon, Jan 04, 2021 at 01:11:26AM +0000, Rick Macklem wrote:
> Kostik wrote:
> >Rick Macklem wrote:
> [stuff snipped]
> >> I see vfs_deferred_inactive() in sys/kern/vfs_subr.c, but I do not
> >> know when/how it gets called?
> >Right, vfs_deferred_inactive() is one way which tries to handle missed
> >inactivations. If upon vput() the lock is only shared and upgrade
> >failed, vnode is marked as VI_OWEINACT and put onto 'lazy' list,
> >processed by vfs_sync(MNT_LAZY). It is typically called from syncer,
> >which means each 60 secs. There, if the vnode is still unreferenced, it
> >is inactivated.
> If I read the code correctly vfs_deferred_inactive() gets called with
> LK_EXCLUSIVE | LK_NOWAIT | LK_INTERLOCK.
> Then there is this code in nfs_lock():
>         vp = ap->a_vp;
> 332	        lktype = ap->a_flags & LK_TYPE_MASK;
> 333	        error = VOP_LOCK1_APV(&default_vnodeops, ap);
> 334	        if (error != 0 || vp->v_op != &newnfs_vnodeops)
> 335	                return (error);
> 336	        np = VTONFS(vp);
> 337	        if (np == NULL)
> 338	                return (0);
> 339	        NFSLOCKNODE(np);
> 340	        if ((np->n_flag & NVNSETSZSKIP) == 0 || (lktype != LK_SHARED &&
> 341	            lktype != LK_EXCLUSIVE && lktype != LK_UPGRADE &&
> 342	            lktype != LK_TRYUPGRADE)) {
> 343	                NFSUNLOCKNODE(np);
> 344	                return (0);
> 345	        }
> 346	        onfault = (ap->a_flags & LK_EATTR_MASK) == LK_NOWAIT &&
> 347	            (ap->a_flags & LK_INIT_MASK) == LK_CANRECURSE &&
> 348	            (lktype == LK_SHARED || lktype == LK_EXCLUSIVE);
> 349	        if (onfault && vp->v_vnlock->lk_recurse == 0) {
> 350	                /*
> 351	                 * Force retry in vm_fault(), to make the lock request
> 352	                 * sleepable, which allows us to piggy-back the
> 353	                 * sleepable call to vnode_pager_setsize().
> 354	                 */
> 355	                NFSUNLOCKNODE(np);
> 356	                VOP_UNLOCK(vp);
> 357	                return (EBUSY);
> 358	        }
> Would the above code result in an EBUSY reply when the vn_lock(LK_NOWAIT |
> LK_EXCLUSIVE) is made in vfs_deferred_inactive()?
I do not think so, there is no LK_CANRECURSE specified by vn_lock().

> 
> If so, it looks like vfs_periodic_inactive()->vfs_deferred_inactive() will just
> call vdefer_inactive_unlocked()->vdefer_inactive() and it will end up
> on the lazy list with VI_DEFINACT set again, if I am reading the
> code correctly?
If there is lock conflict then yes.  Otherwise, I do not think so.

> 
> >Another place where inactivation can occur is reclamation. There in
> >vgonel(), we call VOP_INACTIVE() if VI_OWEINACT is set. In principle,
> >this is redundand because correct filesystem must do the same cleanup
> >(and more) at reclamation as at the inactivation.  But we also call
> >VOP_CLOSE(FNONBLOCK) before VOP_RECLAIM().
> And this would make all the NFSv4 Opens go away (get closed)
> when the nullfs mounts are unmounted, as reported by J. David.
> 
> >Looking at this from another angle, if inactivation for NFSv4 vnodes
> >is not called longer than 2 minutes, perhaps there is a reference leak.
> >It is not due to VFS forgetting about due VOP_INACTIVE() call.
> Unless the nfs_lock(LK_NOWAIT) call keeps failing with EBUSY,
> I think?
> 
> rick
> ps: I need to find a way to reproduce this.
>