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

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

[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(-)

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index f30fa65..eb4a044 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -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;
 }
-- 
1.7.6.2

Re: [PATCH] SQUASHME: pnfsd: Don't call fs_layout_return on a none-file

[Trond Myklebust]

On Thu, 2011-11-10 at 14:50 -0800, Boaz Harrosh wrote: 
> 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...

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: [PATCH] SQUASHME: pnfsd: Don't call fs_layout_return on a none-file

[Boaz Harrosh]

On 11/10/2011 06:07 PM, Trond Myklebust wrote:
> On Thu, 2011-11-10 at 14:50 -0800, Boaz Harrosh wrote: 
>> 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

Re: [PATCH] SQUASHME: pnfsd: Don't call fs_layout_return on a none-file

[Benny Halevy]

On 2011-11-11 00:50, Boaz Harrosh wrote:
> 
> 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(-)
> 
> diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
> index f30fa65..eb4a044 100644
> --- a/fs/nfsd/nfs4pnfsd.c
> +++ b/fs/nfsd/nfs4pnfsd.c
> @@ -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/

> +			 * 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

> +			 */
> +			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;
>  }