Skip to contentSkip to conversation

SQUASHME: pnfsd: Don't call fs_layout_return on a none-file

PATCH In review Boaz Harrosh 3 participants 4 messages Nov 10, 2011 – Nov 14, 2011

Subsystems (2)
NFS client NFS server
lore
Share & tools
lore

Boaz Harrosh

author
#0 lore raw

PATCH 1/1 SQUASHME: pnfsd: Don't call fs_layout_return on a none-file

If we had a layout on the file the nfs4_file would be referenced and we should have found it. Since we don't then it means all layouts where ROC and at this point we returned all of them on file close.

So why does the Linux wonderful forgetfull client returnes it's ROC layouts on inode evict() (I hit this on umount at client). Actually this is on me, because it is a bug in Generic client code, but can only manifest with pnf-obj after an IO error.

Sign-off-by: Boaz Harrosh

---
 fs/nfsd/nfs4pnfsd.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)
fs/nfsd/nfs4pnfsd.c +10 -1
index f30fa65..eb4a044 100644
@@ -1109,7 +1109,15 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
 			printk(KERN_ERR "%s: RETURN_FILE: no nfs4_file for "
 				"ino %p:%lu\n",
 				__func__, ino, ino ? ino->i_ino : 0L);
-			goto out;
+			/* If we had a layout on the file the nfs4_file would
+			 * be referenced and we should have found it. Since we
+			 * don't then it means all layouts where ROC and at this
+			 * point we returned all of them on file close.
+			 * TODO:
+			 * Benny this is what Bruce calls a client spam, we might
+			 * want to degrade the print level to dprintk.
+			 */
+			goto out_no_fs_call;
 		}
 
 		/* Check the stateid */

@@ -1163,6 +1173,7 @@ out:
 	/* call exported filesystem layout_return (ignore return-code) */
 	fs_layout_return(sb, ino, lrp, 0, recall_cookie);
 
+out_no_fs_call:
 	dprintk("pNFS %s: exit status %d \n", __func__, status);
 	return status;
 }
Signature · 1 line
1.7.6.2
#1 lore raw
Quoted 4 lines · Boaz Harrosh

So why does the Linux wonderful forgetfull client returnes it's ROC layouts on inode evict() (I hit this on umount at client). Actually this is on me, because it is a bug in Generic client code, but can only manifest with pnf-obj after an IO error.

Client patches are still welcome...

Signature · 6 lines
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
www.netapp.com
author
#2 lore raw
Quoted 8 lines · Trond Myklebust
Quoted 4 lines · Boaz Harrosh

So why does the Linux wonderful forgetfull client returnes it's ROC layouts on inode evict() (I hit this on umount at client). Actually this is on me, because it is a bug in Generic client code, but can only manifest with pnf-obj after an IO error.

Client patches are still welcome...

I'm on it. There is still a pnfs-objects-std-violation that I need to fix in regards to IO errors and layout-returns. So the above will be a part of that as well.

Thanks
Boaz

#3 lore raw
In reply to Boaz Harrosh
Quoted 20 lines · Boaz Harrosh

If we had a layout on the file the nfs4_file would be referenced and we should have found it. Since we don't then it means all layouts where ROC and at this point we returned all of them on file close.

So why does the Linux wonderful forgetfull client returnes it's ROC layouts on inode evict() (I hit this on umount at client). Actually this is on me, because it is a bug in Generic client code, but can only manifest with pnf-obj after an IO error.

Sign-off-by: Boaz Harrosh

---
 fs/nfsd/nfs4pnfsd.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)
fs/nfsd/nfs4pnfsd.c +0 -0
index f30fa65..eb4a044 100644

fs/nfsd/nfs4pnfsd.c int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh, · line 1114

Quoted 7 lines
fs/nfsd/nfs4pnfsd.c +3 -1
@@ -1109,7 +1109,15 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
 			printk(KERN_ERR "%s: RETURN_FILE: no nfs4_file for "
 				"ino %p:%lu\n",
 				__func__, ino, ino ? ino->i_ino : 0L);
-			goto out;
+			/* If we had a layout on the file the nfs4_file would
+			 * be referenced and we should have found it. Since we
+			 * don't then it means all layouts where ROC and at this

I'll s/where/were/

fs/nfsd/nfs4pnfsd.c int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh, · line 1118

Quoted 4 lines
fs/nfsd/nfs4pnfsd.c +4 -0
+			 * point we returned all of them on file close.
+			 * TODO:
+			 * Benny this is what Bruce calls a client spam, we might
+			 * want to degrade the print level to dprintk.

Agreed, if this case is not an exception no need to shout about it. I'll make this change.

Benny

Quoted 13 lines
fs/nfsd/nfs4pnfsd.c +3 -0
+			 */
+			goto out_no_fs_call;
 		}
 
 		/* Check the stateid */

@@ -1163,6 +1173,7 @@ out:
 	/* call exported filesystem layout_return (ignore return-code) */
 	fs_layout_return(sb, ino, lrp, 0, recall_cookie);
 
+out_no_fs_call:
 	dprintk("pNFS %s: exit status %d \n", __func__, status);
 	return status;
 }