Re: Oops in nfs_scan_commit running xfstest generic/005 with NFSv4.2 and hammerspace flexfiles server

[Trond Myklebust <trondmy@hammerspace.com>]

On Sat, 2021-10-09 at 09:34 -0400, David Wysochanski wrote:
> Trond,
> 
> I wonder if you are aware of this or not.
> 
> This week I ran a lot of xfstests with hammerspace and other servers
> without any issues and just now seeing this oops (after rebuilding
> from a base of your testing branch at 0abb8895b065).  I then re-built
> with just your testing branch and got the same oops.  Same test passes
> on 5.14.0-rc4 (vanilla), as well as previous kernels I used at
> BakeAthon with the fscache and readahead patches only.  It reliably
> panics for me so let me know if you want any more info or a
> reproduction with tracepoints, etc.  FYI, I don't think the server
> matters because I can also reproduce with a rhel8 server
> (kernel-4.18.0-305.19.1.el8_4) and I can also just run 'generic/005'
> directly - previous tests don't matter.
> 
> [   15.767423] nfs4filelayout_init: NFSv4 File Layout Driver
> Registering...
> [   28.614447] nfs4flexfilelayout_init: NFSv4 Flexfile Layout Driver
> Registering...
> [   30.024616] run fstests generic/001 at 2021-10-09 09:01:14
> [   37.188167] run fstests generic/002 at 2021-10-09 09:01:21
> [   38.372767] run fstests generic/003 at 2021-10-09 09:01:23
> [   38.713218] run fstests generic/004 at 2021-10-09 09:01:23
> [   39.065705] run fstests generic/005 at 2021-10-09 09:01:23
> [   39.799076] general protection fault, probably for non-canonical
> address 0xffe826e8e8e7897c: 0000 [#1] SMP PTI
> [   39.805058] CPU: 0 PID: 6213 Comm: rm Kdump: loaded Not tainted
> 5.15.0-rc4-trond-testing+ #76
> [   39.808300] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009),
> BIOS 1.14.0-4.fc34 04/01/2014
> [   39.810819] RIP: 0010:__mutex_lock.constprop.0+0x97/0x3e0
> [   39.812438] Code: 65 d8 5b 41 5c 41 5d 41 5e 41 5f 5d c3 65 48 8b
> 04 25 c0 7b 01 00 48 8b 00 a8 08 75 1d 49 8b 06 48 83 e0 f8 0f 84 79
> 02 00 00 <8b> 50 34 85 d2 0f 85 5c 02 00 00 e8 59 8f 55 ff 65 48 8b 04
> 25 c0
> [   39.817418] RSP: 0018:ffffbb8e4558bcd0 EFLAGS: 00010286
> [   39.818546] RAX: ffe826e8e8e78948 RBX: ffffbb8e4558bd70 RCX:
> ffff932e89300000
> [   39.820087] RDX: ffff932e89300000 RSI: ffe826e8e8e78948 RDI:
> ffff932eae01edf0
> [   39.821583] RBP: ffffbb8e4558bd28 R08: 0000000000000001 R09:
> ffffbb8e4558bca0
> [   39.823091] R10: 000000000000001d R11: ffffffffffffcfcf R12:
> 0000000000000000
> [   39.824796] R13: 0000000000000000 R14: ffff932eae01edf0 R15:
> 0000000000000000
> [   39.826318] FS:  00007fac7df09740(0000) GS:ffff932ff7c00000(0000)
> knlGS:0000000000000000
> [   39.828037] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   39.829275] CR2: 0000555f9b7e2018 CR3: 000000011dc96005 CR4:
> 0000000000770ef0
> [   39.830787] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> 0000000000000000
> [   39.832275] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
> 0000000000000400
> [   39.833756] PKRU: 55555554
> [   39.834377] Call Trace:
> [   39.836219]  ? security_inode_permission+0x30/0x50
> [   39.837365]  nfs_scan_commit+0x36/0xa0 [nfs]
> [   39.838367]  __nfs_commit_inode+0xf8/0x160 [nfs]
> [   39.839417]  nfs_wb_all+0xa6/0xf0 [nfs]
> [   39.840309]  nfs4_inode_return_delegation+0x58/0x80 [nfsv4]
> [   39.841554]  nfs4_proc_remove+0xd1/0xe0 [nfsv4]
> [   39.842589]  nfs_unlink+0xec/0x2d0 [nfs]
> [   39.843461]  vfs_unlink+0x113/0x230
> [   39.844245]  do_unlinkat+0x170/0x280
> [   39.845040]  __x64_sys_unlinkat+0x33/0x60
> [   39.845922]  do_syscall_64+0x3b/0x90
> [   39.846726]  entry_SYSCALL_64_after_hwframe+0x44/0xae
> 

Whoops... I believe the following patch ought to fix it. Thanks for
reporting this!

8<---------------------------------------------------
From 64a082064b7b375263960c4f011bc9875ef50f6a Mon Sep 17 00:00:00 2001
From: Trond Myklebust <trond.myklebust@hammerspace.com>
Date: Sun, 10 Oct 2021 10:58:12 +0200
Subject: [PATCH] NFSv4: Fixes for nfs4_inode_return_delegation()

We mustn't call nfs_wb_all() on anything other than a regular file.
Furthermore, we can exit early when we don't hold a delegation.

Reported-by: David Wysochanski <dwysocha@redhat.com>
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/delegation.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 11118398f495..7c9eb679dbdb 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -755,11 +755,13 @@ int nfs4_inode_return_delegation(struct inode *inode)
 	struct nfs_delegation *delegation;
 
 	delegation = nfs_start_delegation_return(nfsi);
-	/* Synchronous recall of any application leases */
-	break_lease(inode, O_WRONLY | O_RDWR);
-	nfs_wb_all(inode);
-	if (delegation != NULL)
+	if (delegation != NULL) {
+		/* Synchronous recall of any application leases */
+		break_lease(inode, O_WRONLY | O_RDWR);
+		if (S_ISREG(inode->i_mode))
+			nfs_wb_all(inode);
 		return nfs_end_delegation_return(inode, delegation, 1);
+	}
 	return 0;
 }
 
-- 
2.31.1


-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com