"Sleeping with non-sleepable lock" in NFS on recent -current

[Konstantin Belousov]

On Mon, Sep 16, 2019 at 05:44:28PM +1000, Peter Jeremy wrote:
> On 2019-Sep-16 09:32:52 +0300, Konstantin Belousov <kostikbel at gmail.com> wrote:
> >On Mon, Sep 16, 2019 at 04:12:05PM +1000, Peter Jeremy wrote:
> >> I'm consistently seeing panics in the NFS code on recent -current on aarm64.
> >> The panics are one of the following two:
> >> Sleeping on "vmopar" with the following non-sleepable locks held:
> >> exclusive sleep mutex NEWNFSnode lock (NEWNFSnode lock) r = 0 (0xfffffd0078b346f0) locked @ /usr/src/sys/fs/nfsclient/nfs_clport.c:432
> >> 
> >> Sleeping thread (tid 100077, pid 35) owns a non-sleepable lock
> >> 
> >> Both panics have nearly identical backtraces (see below).  I'm running
> >> diskless on a Rock64 with both filesystem and swap over NFS.  The panics
> >> can be fairly reliably triggered by any of:
> >> * "make -j4 buildworld"
> >> * linking the kernel (as part of buildkernel)
> >> * "make installworld"
> >> 
> >> Has anyone else seen this?
> ...
> 
> >Weird since this should have been fixed long time ago.  Anyway, please
> >try the following, it should fix the rest of cases.
> >
> >diff --git a/sys/fs/nfsclient/nfs_clport.c b/sys/fs/nfsclient/nfs_clport.c
> ...
> >@@ -540,7 +541,7 @@ nfscl_loadattrcache(struct vnode **vpp, struct nfsvattr *nap, void *nvaper,
> > 			} else {
> > 				np->n_size = vap->va_size;
> > 				np->n_flag |= NSIZECHANGED;
> >-				vnode_pager_setsize(vp, np->n_size);
> >+				setnsize = 1;
> 
> Should this else block include a "nsize = np->n_size;"?  Without it,
> nsize will remain set to 0, which looks wrong.

Yes, you are right.

diff --git a/sys/fs/nfsclient/nfs_clport.c b/sys/fs/nfsclient/nfs_clport.c
index 471e029a8b5..63ea4736707 100644
--- a/sys/fs/nfsclient/nfs_clport.c
+++ b/sys/fs/nfsclient/nfs_clport.c
@@ -511,10 +511,10 @@ nfscl_loadattrcache(struct vnode **vpp, struct nfsvattr *nap, void *nvaper,
 				 * zero np->n_attrstamp to indicate that
 				 * the attributes are stale.
 				 */
-				vap->va_size = np->n_size;
+				nsize = vap->va_size = np->n_size;
+				setnsize = 1;
 				np->n_attrstamp = 0;
 				KDTRACE_NFS_ATTRCACHE_FLUSH_DONE(vp);
-				vnode_pager_setsize(vp, np->n_size);
 			} else if (np->n_flag & NMODIFIED) {
 				/*
 				 * We've modified the file: Use the larger
@@ -526,7 +526,8 @@ nfscl_loadattrcache(struct vnode **vpp, struct nfsvattr *nap, void *nvaper,
 					np->n_size = vap->va_size;
 					np->n_flag |= NSIZECHANGED;
 				}
-				vnode_pager_setsize(vp, np->n_size);
+				nsize = np->n_size;
+				setnsize = 1;
 			} else if (vap->va_size < np->n_size) {
 				/*
 				 * When shrinking the size, the call to
@@ -538,9 +539,9 @@ nfscl_loadattrcache(struct vnode **vpp, struct nfsvattr *nap, void *nvaper,
 				np->n_flag |= NSIZECHANGED;
 				setnsize = 1;
 			} else {
-				np->n_size = vap->va_size;
+				nsize = np->n_size = vap->va_size;
 				np->n_flag |= NSIZECHANGED;
-				vnode_pager_setsize(vp, np->n_size);
+				setnsize = 1;
 			}
 		} else {
 			np->n_size = vap->va_size;