[PATCH] nfs: Set s_time_gran consistently on NFSv2

[PATCH] nfs: Set s_time_gran consistently on NFSv2

[Andreas Gruenbacher]

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);
 }
-- 
2.7.4

Re: [PATCH] nfs: Set s_time_gran consistently on NFSv2

[Trond Myklebust]

Hi Andreas,

> On Sep 12, 2016, at 07:16, Andreas Gruenbacher <agruenba@redhat.com> wrot=
e:
>=20
> Hi Trond and Anna,
>=20
> is it true that nfs_clone_super accidentally sets s_time_gran to 1 on NFS=
v2?
> If so, could you please merge the following patch?
>=20
> Thanks,
> Andreas
>=20
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
> fs/nfs/super.c | 25 +++++++++----------------
> 1 file changed, 9 insertions(+), 16 deletions(-)
>=20
> 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)
> {
> =09struct nfs_server *server =3D NFS_SB(sb);
>=20
> +=09if (server->nfs_client->rpc_ops->version !=3D 2) {
> +=09=09sb->s_time_gran =3D 1;
> +=09=09/*
> +=09=09 * The VFS shouldn't apply the umask to mode bits. We will do
> +=09=09 * so ourselves when necessary.
> +=09=09 */
> +=09=09sb->s_flags |=3D MS_POSIXACL;
> +=09}
> +
> =09sb->s_magic =3D NFS_SUPER_MAGIC;
>=20
> =09/* 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)
> =09if (data && data->bsize)
> =09=09sb->s_blocksize =3D nfs_block_size(data->bsize, &sb->s_blocksize_bi=
ts);
>=20
> -=09if (server->nfs_client->rpc_ops->version !=3D 2) {
> -=09=09/* The VFS shouldn't apply the umask to mode bits. We will do
> -=09=09 * so ourselves when necessary.
> -=09=09 */
> -=09=09sb->s_flags |=3D MS_POSIXACL;
> -=09=09sb->s_time_gran =3D 1;
> -=09}
> -
>  =09nfs_initialise_sb(sb);
> }
> EXPORT_SYMBOL_GPL(nfs_fill_super);
> @@ -2367,14 +2368,6 @@ void nfs_clone_super(struct super_block *sb, struc=
t nfs_mount_info *mount_info)
> =09sb->s_maxbytes =3D old_sb->s_maxbytes;
> =09sb->s_xattr =3D old_sb->s_xattr;
> =09sb->s_op =3D old_sb->s_op;
> -=09sb->s_time_gran =3D 1;
> -
> -=09if (server->nfs_client->rpc_ops->version !=3D 2) {
> -=09=09/* The VFS shouldn't apply the umask to mode bits. We will do
> -=09=09 * so ourselves when necessary.
> -=09=09 */
> -=09=09sb->s_flags |=3D MS_POSIXACL;
> -=09}
>=20
>  =09nfs_initialise_sb(sb);
> }
> =97=20

Actually, we should be using the fsinfo->time_delta in nfs_server_set_fsinf=
o() 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 =91statfs=92 implementatio=
n doesn=92t return one.

Cheers
  Trond

Re: [PATCH] nfs: Set s_time_gran consistently on NFSv2

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3383 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 29486 bytes --]