Date: Mon, 4 Jan 2021 10:56:46 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: Rick Macklem <rmacklem@uoguelph.ca> Cc: J David <j.david.lists@gmail.com>, "freebsd-fs@freebsd.org" <freebsd-fs@freebsd.org> Subject: Re: Major issues with nfsv4 Message-ID: <X/LYTkkfOK2VaNkW@kib.kiev.ua> In-Reply-To: <YQXPR0101MB0968836D68FBBB0C19ACC2B4DDD20@YQXPR0101MB0968.CANPRD01.PROD.OUTLOOK.COM> References: <YQXPR0101MB0968C7629D57CA21319E50C2DDC90@YQXPR0101MB0968.CANPRD01.PROD.OUTLOOK.COM> <X9UDArKjUqJVS035@kib.kiev.ua> <CABXB=RRNnW9nNhFCJS1evNUTEX9LNnzyf2gOmZHHGkzAoQxbPw@mail.gmail.com> <YQXPR0101MB0968B120A417AF69CEBB6A12DDC80@YQXPR0101MB0968.CANPRD01.PROD.OUTLOOK.COM> <X9aGwshgh7Cwiv8p@kib.kiev.ua> <CABXB=RTFSAEZvp%2BmoiF%2BrE9vpEjJVacLYa6G=yP641f9oHJ1zw@mail.gmail.com> <YQXPR0101MB09681D2CB8FBD5DDE907D5A5DDC40@YQXPR0101MB0968.CANPRD01.PROD.OUTLOOK.COM> <YQXPR0101MB096897CA4344DFDC8D22DFE9DDDB0@YQXPR0101MB0968.CANPRD01.PROD.OUTLOOK.COM> <X%2BitPGhDz6Vldqh%2B@kib.kiev.ua> <YQXPR0101MB0968836D68FBBB0C19ACC2B4DDD20@YQXPR0101MB0968.CANPRD01.PROD.OUTLOOK.COM>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?X/LYTkkfOK2VaNkW>