| Message ID | 20191108210224.33645-1-Anna.Schumaker@Netapp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] NFS: Add FATTR4_WORD1_SPACE_USED to the cache_consistency_bitmask | expand |
On Fri, 2019-11-08 at 16:02 -0500, schumaker.anna@gmail.com wrote: > From: Anna Schumaker <Anna.Schumaker@Netapp.com> > > Changing a sparse file could have an effect not only on the file > size, > but also on the number of blocks used by the file in the underlying > filesystem. Let's update the SPACE_USED attribute whenever we update > SIZE to be as accurate as possible. > > This patch fixes xfstests generic/568, which tests that fallocating > an > unaligned range allocates all blocks touched by that range. Without > this > patch, `stat` reports 0 bytes used immediately after the fallocate. > Adding a `sleep 5` to the test also catches the update, but it's > better > to just do it when we know something has changed. > > Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com> > --- > fs/nfs/nfs4proc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c > index ac9063c06205..00a1f3ec7f22 100644 > --- a/fs/nfs/nfs4proc.c > +++ b/fs/nfs/nfs4proc.c > @@ -3775,7 +3775,7 @@ static int _nfs4_server_capabilities(struct > nfs_server *server, struct nfs_fh *f > > memcpy(server->cache_consistency_bitmask, > res.attr_bitmask, sizeof(server->cache_consistency_bitmask)); > server->cache_consistency_bitmask[0] &= > FATTR4_WORD0_CHANGE|FATTR4_WORD0_SIZE; > - server->cache_consistency_bitmask[1] &= > FATTR4_WORD1_TIME_METADATA|FATTR4_WORD1_TIME_MODIFY; > + server->cache_consistency_bitmask[1] &= > FATTR4_WORD1_TIME_METADATA|FATTR4_WORD1_TIME_MODIFY|FATTR4_WORD1_SPAC > E_USED; I'd rather not do this. Space used is not a cache consistency attribute , as we do not use it to revalidate the cache and it can be rather expensive to retrieve on some platforms. I'd therefore prefer that we just make sure we mark the cache validity with NFS_INO_INVALID_OTHER when we have a write succeed on a sparse file. > server->cache_consistency_bitmask[2] = 0; > > /* Avoid a regression due to buggy server */
Hi Trond, On Fri, 2019-11-08 at 21:22 +0000, Trond Myklebust wrote: > On Fri, 2019-11-08 at 16:02 -0500, schumaker.anna@gmail.com wrote: > > From: Anna Schumaker <Anna.Schumaker@Netapp.com> > > > > Changing a sparse file could have an effect not only on the file > > size, > > but also on the number of blocks used by the file in the underlying > > filesystem. Let's update the SPACE_USED attribute whenever we update > > SIZE to be as accurate as possible. > > > > This patch fixes xfstests generic/568, which tests that fallocating > > an > > unaligned range allocates all blocks touched by that range. Without > > this > > patch, `stat` reports 0 bytes used immediately after the fallocate. > > Adding a `sleep 5` to the test also catches the update, but it's > > better > > to just do it when we know something has changed. > > > > Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com> > > --- > > fs/nfs/nfs4proc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c > > index ac9063c06205..00a1f3ec7f22 100644 > > --- a/fs/nfs/nfs4proc.c > > +++ b/fs/nfs/nfs4proc.c > > @@ -3775,7 +3775,7 @@ static int _nfs4_server_capabilities(struct > > nfs_server *server, struct nfs_fh *f > > > > memcpy(server->cache_consistency_bitmask, > > res.attr_bitmask, sizeof(server->cache_consistency_bitmask)); > > server->cache_consistency_bitmask[0] &= > > FATTR4_WORD0_CHANGE|FATTR4_WORD0_SIZE; > > - server->cache_consistency_bitmask[1] &= > > FATTR4_WORD1_TIME_METADATA|FATTR4_WORD1_TIME_MODIFY; > > + server->cache_consistency_bitmask[1] &= > > FATTR4_WORD1_TIME_METADATA|FATTR4_WORD1_TIME_MODIFY|FATTR4_WORD1_SPAC > > E_USED; > > I'd rather not do this. Space used is not a cache consistency attribute > , as we do not use it to revalidate the cache and it can be rather > expensive to retrieve on some platforms. Okay, that makes sense. > > I'd therefore prefer that we just make sure we mark the cache validity > with NFS_INO_INVALID_OTHER when we have a write succeed on a sparse > file. It looks like setting falloc_bitmask to use the standard nfs4_fattr_bitmap instead of the cache_consistency_bitmask will also update the SPACE_USED attribute after a fallocate call. I'll play around with NFS_INO_INVALID_OTHER next, but making use of the trailing getattr that ALLOCATE and DEALLOCATE already have seems more straightforward to me. Anna > > > server->cache_consistency_bitmask[2] = 0; > > > > /* Avoid a regression due to buggy server */ > -- > Trond Myklebust > Linux NFS client maintainer, Hammerspace > trond.myklebust@hammerspace.com > >
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index ac9063c06205..00a1f3ec7f22 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -3775,7 +3775,7 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f memcpy(server->cache_consistency_bitmask, res.attr_bitmask, sizeof(server->cache_consistency_bitmask)); server->cache_consistency_bitmask[0] &= FATTR4_WORD0_CHANGE|FATTR4_WORD0_SIZE; - server->cache_consistency_bitmask[1] &= FATTR4_WORD1_TIME_METADATA|FATTR4_WORD1_TIME_MODIFY; + server->cache_consistency_bitmask[1] &= FATTR4_WORD1_TIME_METADATA|FATTR4_WORD1_TIME_MODIFY|FATTR4_WORD1_SPACE_USED; server->cache_consistency_bitmask[2] = 0; /* Avoid a regression due to buggy server */