2011-11-10 22:50:43

by Boaz Harrosh

[permalink] [raw]
Subject: [PATCH] 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(-)

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



2011-11-11 20:09:46

by Boaz Harrosh

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

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

2011-11-11 02:07:51

by Myklebust, Trond

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

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
[email protected]
http://www.netapp.com


2011-11-14 11:15:03

by Benny Halevy

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

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;
> }