Linux-NFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/2] NFS: Add FATTR4_WORD1_SPACE_USED to the cache_consistency_bitmask
@ 2019-11-08 21:02 schumaker.anna
  2019-11-08 21:02 ` [PATCH 2/2] NFS: Return -ETXTBSY when attempting to write to a swapfile schumaker.anna
  2019-11-08 21:22 ` [PATCH 1/2] NFS: Add FATTR4_WORD1_SPACE_USED to the cache_consistency_bitmask Trond Myklebust
  0 siblings, 2 replies; 4+ messages in thread
From: schumaker.anna @ 2019-11-08 21:02 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs; +Cc: Anna.Schumaker

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_SPACE_USED;
 		server->cache_consistency_bitmask[2] = 0;
 
 		/* Avoid a regression due to buggy server */
-- 
2.24.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] NFS: Return -ETXTBSY when attempting to write to a swapfile
  2019-11-08 21:02 [PATCH 1/2] NFS: Add FATTR4_WORD1_SPACE_USED to the cache_consistency_bitmask schumaker.anna
@ 2019-11-08 21:02 ` schumaker.anna
  2019-11-08 21:22 ` [PATCH 1/2] NFS: Add FATTR4_WORD1_SPACE_USED to the cache_consistency_bitmask Trond Myklebust
  1 sibling, 0 replies; 4+ messages in thread
From: schumaker.anna @ 2019-11-08 21:02 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs; +Cc: Anna.Schumaker

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

My understanding is that -EBUSY refers to the underlying device, and
that -ETXTBSY is used when attempting to access a file in use by the
kernel (like a swapfile). Changing this return code helps us pass
xfstests generic/569

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 95dc90570786..8eb731d9be3e 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -649,7 +649,7 @@ ssize_t nfs_file_write(struct kiocb *iocb, struct iov_iter *from)
 
 out_swapfile:
 	printk(KERN_INFO "NFS: attempt to write to active swap file!\n");
-	return -EBUSY;
+	return -ETXTBSY;
 }
 EXPORT_SYMBOL_GPL(nfs_file_write);
 
-- 
2.24.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] NFS: Add FATTR4_WORD1_SPACE_USED to the cache_consistency_bitmask
  2019-11-08 21:02 [PATCH 1/2] NFS: Add FATTR4_WORD1_SPACE_USED to the cache_consistency_bitmask schumaker.anna
  2019-11-08 21:02 ` [PATCH 2/2] NFS: Return -ETXTBSY when attempting to write to a swapfile schumaker.anna
@ 2019-11-08 21:22 ` Trond Myklebust
  2019-11-11 16:24   ` Anna Schumaker
  1 sibling, 1 reply; 4+ messages in thread
From: Trond Myklebust @ 2019-11-08 21:22 UTC (permalink / raw)
  To: linux-nfs, schumaker.anna; +Cc: Anna.Schumaker

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 */
-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] NFS: Add FATTR4_WORD1_SPACE_USED to the cache_consistency_bitmask
  2019-11-08 21:22 ` [PATCH 1/2] NFS: Add FATTR4_WORD1_SPACE_USED to the cache_consistency_bitmask Trond Myklebust
@ 2019-11-11 16:24   ` Anna Schumaker
  0 siblings, 0 replies; 4+ messages in thread
From: Anna Schumaker @ 2019-11-11 16:24 UTC (permalink / raw)
  To: Trond Myklebust, linux-nfs

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-08 21:02 [PATCH 1/2] NFS: Add FATTR4_WORD1_SPACE_USED to the cache_consistency_bitmask schumaker.anna
2019-11-08 21:02 ` [PATCH 2/2] NFS: Return -ETXTBSY when attempting to write to a swapfile schumaker.anna
2019-11-08 21:22 ` [PATCH 1/2] NFS: Add FATTR4_WORD1_SPACE_USED to the cache_consistency_bitmask Trond Myklebust
2019-11-11 16:24   ` Anna Schumaker

Linux-NFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nfs/0 linux-nfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nfs linux-nfs/ https://lore.kernel.org/linux-nfs \
		linux-nfs@vger.kernel.org
	public-inbox-index linux-nfs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-nfs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git