From: Olga Kornievskaia <[email protected]>
After client is done with the COPY operation, it needs to invalidate
its pagecache (as it did no reading or writing of the data locally)
and it needs to invalidate it's attributes just like it would have
for a read on the source file and write on the destination file.
Once the linux server started giving out read delegations to
read+write opens, the destination file of the copy_file range
started having delegations and not doing syncup on close of the
file leading to xfstest failures for generic/430,431,432,433,565.
Reported-by: Murphy Zhou <[email protected]>
Fixes: 2e72448b07dc ("NFS: Add COPY nfs operation")
Signed-off-by: Olga Kornievskaia <[email protected]>
---
fs/nfs/nfs42proc.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index 142225f0af59..a9074f3366fa 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -356,7 +356,11 @@ static ssize_t _nfs42_proc_copy(struct file *src,
truncate_pagecache_range(dst_inode, pos_dst,
pos_dst + res->write_res.count);
-
+ NFS_I(dst_inode)->cache_validity |= (NFS_INO_REVAL_PAGECACHE |
+ NFS_INO_REVAL_FORCED | NFS_INO_INVALID_SIZE |
+ NFS_INO_INVALID_ATTR | NFS_INO_INVALID_DATA);
+ NFS_I(src_inode)->cache_validity |= (NFS_INO_REVAL_PAGECACHE |
+ NFS_INO_REVAL_FORCED | NFS_INO_INVALID_ATIME);
status = res->write_res.count;
out:
if (args->sync)
--
2.18.1
On Mon, Sep 14, 2020 at 04:23:34PM -0400, Olga Kornievskaia wrote:
>
>
> From: Olga Kornievskaia <[email protected]>
>
> After client is done with the COPY operation, it needs to invalidate
> its pagecache (as it did no reading or writing of the data locally)
> and it needs to invalidate it's attributes just like it would have
> for a read on the source file and write on the destination file.
>
> Once the linux server started giving out read delegations to
> read+write opens, the destination file of the copy_file range
> started having delegations and not doing syncup on close of the
> file leading to xfstest failures for generic/430,431,432,433,565.
>
> Reported-by: Murphy Zhou <[email protected]>
> Fixes: 2e72448b07dc ("NFS: Add COPY nfs operation")
> Signed-off-by: Olga Kornievskaia <[email protected]>
> ---
> fs/nfs/nfs42proc.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> index 142225f0af59..a9074f3366fa 100644
> --- a/fs/nfs/nfs42proc.c
> +++ b/fs/nfs/nfs42proc.c
> @@ -356,7 +356,11 @@ static ssize_t _nfs42_proc_copy(struct file *src,
>
> truncate_pagecache_range(dst_inode, pos_dst,
> pos_dst + res->write_res.count);
> -
> + NFS_I(dst_inode)->cache_validity |= (NFS_INO_REVAL_PAGECACHE |
> + NFS_INO_REVAL_FORCED | NFS_INO_INVALID_SIZE |
> + NFS_INO_INVALID_ATTR | NFS_INO_INVALID_DATA);
> + NFS_I(src_inode)->cache_validity |= (NFS_INO_REVAL_PAGECACHE |
> + NFS_INO_REVAL_FORCED | NFS_INO_INVALID_ATIME);
> status = res->write_res.count;
> out:
> if (args->sync)
> --
> 2.18.1
Should this be copied to stable@ ?
- Frank
On Mon, Sep 14, 2020 at 04:23:34PM -0400, Olga Kornievskaia wrote:
> From: Olga Kornievskaia <[email protected]>
>
> After client is done with the COPY operation, it needs to invalidate
> its pagecache (as it did no reading or writing of the data locally)
> and it needs to invalidate it's attributes just like it would have
> for a read on the source file and write on the destination file.
>
> Once the linux server started giving out read delegations to
> read+write opens, the destination file of the copy_file range
> started having delegations and not doing syncup on close of the
> file leading to xfstest failures for generic/430,431,432,433,565.
Tested OK. ltp and xfstests on v3/v4.* looks fine.
The other issue generic/464 warning I reported before is still
there with Olga's patch. For the record.
Thanks!
>
> Reported-by: Murphy Zhou <[email protected]>
> Fixes: 2e72448b07dc ("NFS: Add COPY nfs operation")
> Signed-off-by: Olga Kornievskaia <[email protected]>
> ---
> fs/nfs/nfs42proc.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> index 142225f0af59..a9074f3366fa 100644
> --- a/fs/nfs/nfs42proc.c
> +++ b/fs/nfs/nfs42proc.c
> @@ -356,7 +356,11 @@ static ssize_t _nfs42_proc_copy(struct file *src,
>
> truncate_pagecache_range(dst_inode, pos_dst,
> pos_dst + res->write_res.count);
> -
> + NFS_I(dst_inode)->cache_validity |= (NFS_INO_REVAL_PAGECACHE |
> + NFS_INO_REVAL_FORCED | NFS_INO_INVALID_SIZE |
> + NFS_INO_INVALID_ATTR | NFS_INO_INVALID_DATA);
> + NFS_I(src_inode)->cache_validity |= (NFS_INO_REVAL_PAGECACHE |
> + NFS_INO_REVAL_FORCED | NFS_INO_INVALID_ATIME);
> status = res->write_res.count;
> out:
> if (args->sync)
> --
> 2.18.1
>
--
Murphy
On Tue, Sep 15, 2020 at 8:51 PM Murphy Zhou <[email protected]> wrote:
>
> On Mon, Sep 14, 2020 at 04:23:34PM -0400, Olga Kornievskaia wrote:
> > From: Olga Kornievskaia <[email protected]>
> >
> > After client is done with the COPY operation, it needs to invalidate
> > its pagecache (as it did no reading or writing of the data locally)
> > and it needs to invalidate it's attributes just like it would have
> > for a read on the source file and write on the destination file.
> >
> > Once the linux server started giving out read delegations to
> > read+write opens, the destination file of the copy_file range
> > started having delegations and not doing syncup on close of the
> > file leading to xfstest failures for generic/430,431,432,433,565.
>
> Tested OK. ltp and xfstests on v3/v4.* looks fine.
Thank you Murphy. I'm about to send a v2 as I didn't have appropriate
locking added.
> The other issue generic/464 warning I reported before is still
> there with Olga's patch. For the record.
Sorry I'm not seeing it but this doesn't look like copy_file_range
related and hopefully Bruce can take a look at this.
>
> Thanks!
> >
> > Reported-by: Murphy Zhou <[email protected]>
> > Fixes: 2e72448b07dc ("NFS: Add COPY nfs operation")
> > Signed-off-by: Olga Kornievskaia <[email protected]>
> > ---
> > fs/nfs/nfs42proc.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> > index 142225f0af59..a9074f3366fa 100644
> > --- a/fs/nfs/nfs42proc.c
> > +++ b/fs/nfs/nfs42proc.c
> > @@ -356,7 +356,11 @@ static ssize_t _nfs42_proc_copy(struct file *src,
> >
> > truncate_pagecache_range(dst_inode, pos_dst,
> > pos_dst + res->write_res.count);
> > -
> > + NFS_I(dst_inode)->cache_validity |= (NFS_INO_REVAL_PAGECACHE |
> > + NFS_INO_REVAL_FORCED | NFS_INO_INVALID_SIZE |
> > + NFS_INO_INVALID_ATTR | NFS_INO_INVALID_DATA);
> > + NFS_I(src_inode)->cache_validity |= (NFS_INO_REVAL_PAGECACHE |
> > + NFS_INO_REVAL_FORCED | NFS_INO_INVALID_ATIME);
> > status = res->write_res.count;
> > out:
> > if (args->sync)
> > --
> > 2.18.1
> >
>
> --
> Murphy