| Message ID | 1596031949-26793-14-git-send-email-dwysocha@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Convert NFS to new FS-Cache iter API | expand |
On Wed, 2020-07-29 at 10:12 -0400, Dave Wysochanski wrote: > Handle truncate / setattr when fscache is enabled by calling > fscache_resize_cookie(). > > Signed-off-by: Dave Wysochanski <dwysocha@redhat.com> > --- > fs/nfs/inode.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c > index 45067303348c..6b814246d07d 100644 > --- a/fs/nfs/inode.c > +++ b/fs/nfs/inode.c > @@ -667,6 +667,7 @@ static int nfs_vmtruncate(struct inode * inode, loff_t offset) > spin_unlock(&inode->i_lock); > truncate_pagecache(inode, offset); > spin_lock(&inode->i_lock); > + fscache_resize_cookie(nfs_i_fscache(inode), i_size_read(inode)); > out: > return err; > } truncate can happen even when you have no open file descriptors on the file and therefore w/o the cookie being "used". In the ceph vmtruncate handling code, I do an explicit use/unuse around this call. Do you need to do the same here?
On Thu, Jul 30, 2020 at 2:39 PM Jeff Layton <jlayton@redhat.com> wrote: > > On Wed, 2020-07-29 at 10:12 -0400, Dave Wysochanski wrote: > > Handle truncate / setattr when fscache is enabled by calling > > fscache_resize_cookie(). > > > > Signed-off-by: Dave Wysochanski <dwysocha@redhat.com> > > --- > > fs/nfs/inode.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c > > index 45067303348c..6b814246d07d 100644 > > --- a/fs/nfs/inode.c > > +++ b/fs/nfs/inode.c > > @@ -667,6 +667,7 @@ static int nfs_vmtruncate(struct inode * inode, loff_t offset) > > spin_unlock(&inode->i_lock); > > truncate_pagecache(inode, offset); > > spin_lock(&inode->i_lock); > > + fscache_resize_cookie(nfs_i_fscache(inode), i_size_read(inode)); > > out: > > return err; > > } > > truncate can happen even when you have no open file descriptors on the > file and therefore w/o the cookie being "used". In the ceph vmtruncate > handling code, I do an explicit use/unuse around this call. Do you need > to do the same here? > -- > Jeff Layton <jlayton@redhat.com> > Actually I think the case you mention is covered by a patch that I've just added today on top of my v2 posting. This was the result of looking deeper into a few xfstest failures with NFSv4.2. I think this covers the truncate without a file open: commit 91d6922df9390ca1c090911be6e5c5ab1a79ea83 Author: Dave Wysochanski <dwysocha@redhat.com> Date: Thu Jul 30 12:33:40 2020 -0400 NFS: Call fscache_invalidate() from nfs_invalidate_mapping() Be sure to invalidate fscache cookie for any call to nfs_invalidate_mapping(). This patch fixes the following xfstests on NFS4.x: generic/240 as well as fixes the following xfstests on NFSv4.2: generic/029 generic/030 Signed-off-by: Dave Wysochanski <dwysocha@redhat.com> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index 6b814246d07d..62243ec05917 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -1233,6 +1233,7 @@ static int nfs_invalidate_mapping(struct inode *inode, struct address_space *map struct nfs_inode *nfsi = NFS_I(inode); int ret; + nfs_fscache_invalidate(inode, 0); if (mapping->nrpages != 0) { if (S_ISREG(inode->i_mode)) { ret = nfs_sync_mapping(mapping);
On Thu, Jul 30, 2020 at 3:23 PM David Wysochanski <dwysocha@redhat.com> wrote: > > On Thu, Jul 30, 2020 at 2:39 PM Jeff Layton <jlayton@redhat.com> wrote: > > > > On Wed, 2020-07-29 at 10:12 -0400, Dave Wysochanski wrote: > > > Handle truncate / setattr when fscache is enabled by calling > > > fscache_resize_cookie(). > > > > > > Signed-off-by: Dave Wysochanski <dwysocha@redhat.com> > > > --- > > > fs/nfs/inode.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c > > > index 45067303348c..6b814246d07d 100644 > > > --- a/fs/nfs/inode.c > > > +++ b/fs/nfs/inode.c > > > @@ -667,6 +667,7 @@ static int nfs_vmtruncate(struct inode * inode, loff_t offset) > > > spin_unlock(&inode->i_lock); > > > truncate_pagecache(inode, offset); > > > spin_lock(&inode->i_lock); > > > + fscache_resize_cookie(nfs_i_fscache(inode), i_size_read(inode)); > > > out: > > > return err; > > > } > > > > truncate can happen even when you have no open file descriptors on the > > file and therefore w/o the cookie being "used". In the ceph vmtruncate > > handling code, I do an explicit use/unuse around this call. Do you need > > to do the same here? > > -- > > Jeff Layton <jlayton@redhat.com> > > > > Actually I think the case you mention is covered by a patch that I've just > added today on top of my v2 posting. > This was the result of looking deeper into a few xfstest failures with > NFSv4.2. I think this covers the truncate without a file open: > > commit 91d6922df9390ca1c090911be6e5c5ab1a79ea83 > Author: Dave Wysochanski <dwysocha@redhat.com> > Date: Thu Jul 30 12:33:40 2020 -0400 > > NFS: Call fscache_invalidate() from nfs_invalidate_mapping() > > Be sure to invalidate fscache cookie for any call to > nfs_invalidate_mapping(). > > This patch fixes the following xfstests on NFS4.x: > generic/240 > as well as fixes the following xfstests on NFSv4.2: > generic/029 generic/030 > > Signed-off-by: Dave Wysochanski <dwysocha@redhat.com> > > diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c > index 6b814246d07d..62243ec05917 100644 > --- a/fs/nfs/inode.c > +++ b/fs/nfs/inode.c > @@ -1233,6 +1233,7 @@ static int nfs_invalidate_mapping(struct inode > *inode, struct address_space *map > struct nfs_inode *nfsi = NFS_I(inode); > int ret; > > + nfs_fscache_invalidate(inode, 0); > if (mapping->nrpages != 0) { > if (S_ISREG(inode->i_mode)) { > ret = nfs_sync_mapping(mapping); Actually the above patch fixes truncates when a file is open, not the case that Jeff mentions. To be honest I'm not sure about needing a call to fscache_use/unuse_cookie() around the call to fscache_resize_cookie(). If the cookie has a refcount of 1 when it is created, and a file is never opened, so we never call fscache_use_cookie(), what might happen inside fscache_resize_cookie()? The header on use_cookie() says /* * Start using the cookie for I/O. This prevents the backing object from being * reaped by VM pressure. */ But we're not using it for I/O in this case. I will have to dig deeper to be sure, or maybe David H will elaborate further.
David Wysochanski <dwysocha@redhat.com> wrote: > To be honest I'm not sure about needing a call to fscache_use/unuse_cookie() > around the call to fscache_resize_cookie(). If the cookie has a > refcount of 1 when it is created, and a file is never opened, so > we never call fscache_use_cookie(), what might happen inside > fscache_resize_cookie()? The header on use_cookie() says I've have afs_setattr() doing use/unuse on the cookie around resize. David
On Thu, Jul 30, 2020 at 4:03 PM David Howells <dhowells@redhat.com> wrote: > > David Wysochanski <dwysocha@redhat.com> wrote: > > > To be honest I'm not sure about needing a call to fscache_use/unuse_cookie() > > around the call to fscache_resize_cookie(). If the cookie has a > > refcount of 1 when it is created, and a file is never opened, so > > we never call fscache_use_cookie(), what might happen inside > > fscache_resize_cookie()? The header on use_cookie() says > > I've have afs_setattr() doing use/unuse on the cookie around resize. > > David > Got it and will be fixed in next series. Thanks!
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index 45067303348c..6b814246d07d 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -667,6 +667,7 @@ static int nfs_vmtruncate(struct inode * inode, loff_t offset) spin_unlock(&inode->i_lock); truncate_pagecache(inode, offset); spin_lock(&inode->i_lock); + fscache_resize_cookie(nfs_i_fscache(inode), i_size_read(inode)); out: return err; }
Handle truncate / setattr when fscache is enabled by calling fscache_resize_cookie(). Signed-off-by: Dave Wysochanski <dwysocha@redhat.com> --- fs/nfs/inode.c | 1 + 1 file changed, 1 insertion(+)