diff mbox

nfs: Set s_time_gran consistently on NFSv2

Message ID 1473678968-13124-1-git-send-email-agruenba@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andreas Gruenbacher Sept. 12, 2016, 11:16 a.m. UTC 
Hi Trond and Anna,

is it true that nfs_clone_super accidentally sets s_time_gran to 1 on NFSv2?
If so, could you please merge the following patch?

Thanks,
Andreas

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/nfs/super.c | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

Comments

Trond Myklebust Sept. 12, 2016, 12:18 p.m. UTC | #1 
Hi Andreas,

> On Sep 12, 2016, at 07:16, Andreas Gruenbacher <agruenba@redhat.com> wrote:
> 
> Hi Trond and Anna,
> 
> is it true that nfs_clone_super accidentally sets s_time_gran to 1 on NFSv2?
> If so, could you please merge the following patch?
> 
> Thanks,
> Andreas
> 
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
> fs/nfs/super.c | 25 +++++++++----------------
> 1 file changed, 9 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index d396013..4c72fec 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2312,6 +2312,15 @@ inline void nfs_initialise_sb(struct super_block *sb)
> {
> 	struct nfs_server *server = NFS_SB(sb);
> 
> +	if (server->nfs_client->rpc_ops->version != 2) {
> +		sb->s_time_gran = 1;
> +		/*
> +		 * The VFS shouldn't apply the umask to mode bits. We will do
> +		 * so ourselves when necessary.
> +		 */
> +		sb->s_flags |= MS_POSIXACL;
> +	}
> +
> 	sb->s_magic = NFS_SUPER_MAGIC;
> 
> 	/* We probably want something more informative here */
> @@ -2342,14 +2351,6 @@ void nfs_fill_super(struct super_block *sb, struct nfs_mount_info *mount_info)
> 	if (data && data->bsize)
> 		sb->s_blocksize = nfs_block_size(data->bsize, &sb->s_blocksize_bits);
> 
> -	if (server->nfs_client->rpc_ops->version != 2) {
> -		/* The VFS shouldn't apply the umask to mode bits. We will do
> -		 * so ourselves when necessary.
> -		 */
> -		sb->s_flags |= MS_POSIXACL;
> -		sb->s_time_gran = 1;
> -	}
> -
>  	nfs_initialise_sb(sb);
> }
> EXPORT_SYMBOL_GPL(nfs_fill_super);
> @@ -2367,14 +2368,6 @@ void nfs_clone_super(struct super_block *sb, struct nfs_mount_info *mount_info)
> 	sb->s_maxbytes = old_sb->s_maxbytes;
> 	sb->s_xattr = old_sb->s_xattr;
> 	sb->s_op = old_sb->s_op;
> -	sb->s_time_gran = 1;
> -
> -	if (server->nfs_client->rpc_ops->version != 2) {
> -		/* The VFS shouldn't apply the umask to mode bits. We will do
> -		 * so ourselves when necessary.
> -		 */
> -		sb->s_flags |= MS_POSIXACL;
> -	}
> 
>  	nfs_initialise_sb(sb);
> }
> — 

Actually, we should be using the fsinfo->time_delta in nfs_server_set_fsinfo() to set sb->s_time_gran to the precise value supported by the filesystem.
We can then fake up a value for NFSv2, since its ‘statfs’ implementation doesn’t return one.

Cheers
  Trond

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
kernel test robot Sept. 12, 2016, 12:32 p.m. UTC | #2 
Hi Andreas,

[auto build test WARNING on nfs/linux-next]
[also build test WARNING on v4.8-rc6 next-20160912]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:    https://github.com/0day-ci/linux/commits/Andreas-Gruenbacher/nfs-Set-s_time_gran-consistently-on-NFSv2/20160912-200059
base:   git://git.linux-nfs.org/projects/trondmy/linux-nfs.git linux-next
config: x86_64-randconfig-x001-201637 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   fs/nfs/super.c: In function 'nfs_clone_super':
>> fs/nfs/super.c:2364:21: warning: unused variable 'server' [-Wunused-variable]
     struct nfs_server *server = NFS_SB(sb);
                        ^~~~~~

vim +/server +2364 fs/nfs/super.c

f7b422b1 David Howells   2006-06-09  2348  	sb->s_blocksize = 0;
6a74490d Bryan Schumaker 2012-07-30  2349  	sb->s_xattr = server->nfs_client->cl_nfs_mod->xattr;
6a74490d Bryan Schumaker 2012-07-30  2350  	sb->s_op = server->nfs_client->cl_nfs_mod->sops;
6a74490d Bryan Schumaker 2012-07-30  2351  	if (data && data->bsize)
f7b422b1 David Howells   2006-06-09  2352  		sb->s_blocksize = nfs_block_size(data->bsize, &sb->s_blocksize_bits);
f7b422b1 David Howells   2006-06-09  2353  
54ceac45 David Howells   2006-08-22  2354   	nfs_initialise_sb(sb);
54ceac45 David Howells   2006-08-22  2355  }
89d77c8f Bryan Schumaker 2012-07-30  2356  EXPORT_SYMBOL_GPL(nfs_fill_super);
8fa5c000 David Howells   2006-08-22  2357  
f7b422b1 David Howells   2006-06-09  2358  /*
3cadf4b8 Bryan Schumaker 2012-07-16  2359   * Finish setting up a cloned NFS2/3/4 superblock
54ceac45 David Howells   2006-08-22  2360   */
fbdefd64 Bryan Schumaker 2012-07-16  2361  void nfs_clone_super(struct super_block *sb, struct nfs_mount_info *mount_info)
54ceac45 David Howells   2006-08-22  2362  {
8c958e0c Bryan Schumaker 2012-05-10  2363  	const struct super_block *old_sb = mount_info->cloned->sb;
54ceac45 David Howells   2006-08-22 @2364  	struct nfs_server *server = NFS_SB(sb);
54ceac45 David Howells   2006-08-22  2365  
54ceac45 David Howells   2006-08-22  2366  	sb->s_blocksize_bits = old_sb->s_blocksize_bits;
54ceac45 David Howells   2006-08-22  2367  	sb->s_blocksize = old_sb->s_blocksize;
54ceac45 David Howells   2006-08-22  2368  	sb->s_maxbytes = old_sb->s_maxbytes;
3cadf4b8 Bryan Schumaker 2012-07-16  2369  	sb->s_xattr = old_sb->s_xattr;
3cadf4b8 Bryan Schumaker 2012-07-16  2370  	sb->s_op = old_sb->s_op;
f7b422b1 David Howells   2006-06-09  2371  
54ceac45 David Howells   2006-08-22  2372   	nfs_initialise_sb(sb);

:::::: The code at line 2364 was first introduced by commit
:::::: 54ceac4515986030c2502960be620198dd8fe25b NFS: Share NFS superblocks per-protocol per-server per-FSID

:::::: TO: David Howells <dhowells@redhat.com>
:::::: CC: Trond Myklebust <Trond.Myklebust@netapp.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index d396013..4c72fec 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2312,6 +2312,15 @@  inline void nfs_initialise_sb(struct super_block *sb)
 {
 	struct nfs_server *server = NFS_SB(sb);
 
+	if (server->nfs_client->rpc_ops->version != 2) {
+		sb->s_time_gran = 1;
+		/*
+		 * The VFS shouldn't apply the umask to mode bits. We will do
+		 * so ourselves when necessary.
+		 */
+		sb->s_flags |= MS_POSIXACL;
+	}
+
 	sb->s_magic = NFS_SUPER_MAGIC;
 
 	/* We probably want something more informative here */
@@ -2342,14 +2351,6 @@  void nfs_fill_super(struct super_block *sb, struct nfs_mount_info *mount_info)
 	if (data && data->bsize)
 		sb->s_blocksize = nfs_block_size(data->bsize, &sb->s_blocksize_bits);
 
-	if (server->nfs_client->rpc_ops->version != 2) {
-		/* The VFS shouldn't apply the umask to mode bits. We will do
-		 * so ourselves when necessary.
-		 */
-		sb->s_flags |= MS_POSIXACL;
-		sb->s_time_gran = 1;
-	}
-
  	nfs_initialise_sb(sb);
 }
 EXPORT_SYMBOL_GPL(nfs_fill_super);
@@ -2367,14 +2368,6 @@  void nfs_clone_super(struct super_block *sb, struct nfs_mount_info *mount_info)
 	sb->s_maxbytes = old_sb->s_maxbytes;
 	sb->s_xattr = old_sb->s_xattr;
 	sb->s_op = old_sb->s_op;
-	sb->s_time_gran = 1;
-
-	if (server->nfs_client->rpc_ops->version != 2) {
-		/* The VFS shouldn't apply the umask to mode bits. We will do
-		 * so ourselves when necessary.
-		 */
-		sb->s_flags |= MS_POSIXACL;
-	}
 
  	nfs_initialise_sb(sb);
 }