[PATCH v2 0/1] NFSv4.2: Fix NFS4ERR_STALE with inter server copy

[PATCH v2 0/1] NFSv4.2: Fix NFS4ERR_STALE with inter server copy

[Dai Ngo]

This cover email is intended for including my test results.

This patch adds the ops table in nfs_common for knfsd to access
NFS client modules without calling these functions directly.

The client module registers their functions and deregisters them
when the module is loaded and unloaded respectively.

 fs/nfs/nfs4file.c       |  44 ++++++++++++--
 fs/nfs/nfs4super.c      |   6 ++
 fs/nfs/super.c          |  20 +++++++
 fs/nfs_common/Makefile  |   1 +
 fs/nfs_common/nfs_ssc.c | 136 +++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/Kconfig         |   2 +-
 fs/nfsd/nfs4proc.c      |   3 +-
 include/linux/nfs_ssc.h |  77 ++++++++++++++++++++++++
 8 files changed, 281 insertions(+), 8 deletions(-)

Test Results:

Upstream version used for testing:  5.9-rc5

|----------------------------------------------------------|
|  NFSD  |  NFS_FS  |  NFS_V4  |       RESULTS             |
|----------------------------------------------------------|
|   m    |    y     |    m     | inter server copy OK      |
|----------------------------------------------------------|
|   m    |    m     |    m     | inter server copy OK      |
|----------------------------------------------------------|
|   m    |    m     |   y (m)  | inter server copy OK      |
|----------------------------------------------------------|
|   m    |    y     |    y     | inter server copy OK      |
|----------------------------------------------------------|
|   m    |    n     |    n     | NFS4ERR_STALE error       |
|----------------------------------------------------------|


|----------------------------------------------------------|
|  NFSD  |  NFS_FS  |  NFS_V4  |        RESULTS            |
|----------------------------------------------------------|
|   y    |    y     |    m     | inter server copy OK      |
|----------------------------------------------------------|
|   y    |    m     |    m     | inter server copy OK      |
|----------------------------------------------------------|
|   y    |    m     |   y (m)  | inter server copy OK      |
|----------------------------------------------------------|
|   y    |    y     |    y     | inter server copy OK      |
|----------------------------------------------------------|
|   y    |    n     |    n     | NFS4ERR_STALE error       |
|----------------------------------------------------------|

NOTE:
When NFS_V4=y and NFS_FS=m, the build process automatically builds
with NFS_V4=m and ignores the setting NFS_V4=y in the config file. 

This probably due to NFS_V4 in fs/nfs/Kconfig is configured to
depend on NFS_FS.

[PATCH v2 1/1] NFSv4.2: Fix NFS4ERR_STALE error when doing inter server copy

[Dai Ngo]

NFS_FS=y as dependency of CONFIG_NFSD_V4_2_INTER_SSC still have
build errors and some configs with NFSD=m to get NFS4ERR_STALE
error when doing inter server copy.

Added ops table in nfs_common for knfsd to access NFS client modules.

Fixes: 3ac3711adb88 ("NFSD: Fix NFS server build errors")
Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
 fs/nfs/nfs4file.c       |  44 +++++++++++++---
 fs/nfs/nfs4super.c      |   6 +++
 fs/nfs/super.c          |  20 +++++++
 fs/nfs_common/Makefile  |   1 +
 fs/nfs_common/nfs_ssc.c | 136 ++++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/Kconfig         |   2 +-
 fs/nfsd/nfs4proc.c      |   3 +-
 include/linux/nfs_ssc.h |  77 +++++++++++++++++++++++++++
 8 files changed, 281 insertions(+), 8 deletions(-)
 create mode 100644 fs/nfs_common/nfs_ssc.c
 create mode 100644 include/linux/nfs_ssc.h

diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
index fdfc77486ace..ea42c39bc2ff 100644
--- a/fs/nfs/nfs4file.c
+++ b/fs/nfs/nfs4file.c
@@ -9,6 +9,7 @@
 #include <linux/falloc.h>
 #include <linux/mount.h>
 #include <linux/nfs_fs.h>
+#include <linux/nfs_ssc.h>
 #include "delegation.h"
 #include "internal.h"
 #include "iostat.h"
@@ -23,6 +24,16 @@
 
 #define NFSDBG_FACILITY		NFSDBG_FILE
 
+static struct file *__nfs42_ssc_open(struct vfsmount *ss_mnt,
+		struct nfs_fh *src_fh, nfs4_stateid *stateid);
+static void __nfs42_ssc_close(struct file *filep);
+
+const struct nfs4_ssc_client_ops nfs4_ssc_clnt_ops_tbl = {
+	.sco_owner = THIS_MODULE,
+	.sco_open = __nfs42_ssc_open,
+	.sco_close = __nfs42_ssc_close,
+};
+
 static int
 nfs4_file_open(struct inode *inode, struct file *filp)
 {
@@ -314,9 +325,8 @@ static loff_t nfs42_remap_file_range(struct file *src_file, loff_t src_off,
 static int read_name_gen = 1;
 #define SSC_READ_NAME_BODY "ssc_read_%d"
 
-struct file *
-nfs42_ssc_open(struct vfsmount *ss_mnt, struct nfs_fh *src_fh,
-		nfs4_stateid *stateid)
+static struct file *__nfs42_ssc_open(struct vfsmount *ss_mnt,
+		struct nfs_fh *src_fh, nfs4_stateid *stateid)
 {
 	struct nfs_fattr fattr;
 	struct file *filep, *res;
@@ -398,14 +408,36 @@ struct file *
 	fput(filep);
 	goto out_free_name;
 }
-EXPORT_SYMBOL_GPL(nfs42_ssc_open);
-void nfs42_ssc_close(struct file *filep)
+
+static void __nfs42_ssc_close(struct file *filep)
 {
 	struct nfs_open_context *ctx = nfs_file_open_context(filep);
 
 	ctx->state->flags = 0;
 }
-EXPORT_SYMBOL_GPL(nfs42_ssc_close);
+
+/**
+ * nfs42_ssc_register_ops - Wrapper to register NFS_V4 ops in nfs_common
+ *
+ * Return values:
+ *   On success, returns 0
+ *   %-EINVAL if validation check fails
+ */
+int nfs42_ssc_register_ops(void)
+{
+	return nfs42_ssc_register(&nfs4_ssc_clnt_ops_tbl);
+}
+
+/**
+ * nfs42_ssc_unregister_ops - wrapper to un-register NFS_V4 ops in nfs_common
+ *
+ * Return values:
+ *   None.
+ */
+void nfs42_ssc_unregister_ops(void)
+{
+	nfs42_ssc_unregister(&nfs4_ssc_clnt_ops_tbl);
+}
 #endif /* CONFIG_NFS_V4_2 */
 
 const struct file_operations nfs4_file_operations = {
diff --git a/fs/nfs/nfs4super.c b/fs/nfs/nfs4super.c
index 0c1ab846b83d..ed0c1f9fc890 100644
--- a/fs/nfs/nfs4super.c
+++ b/fs/nfs/nfs4super.c
@@ -7,6 +7,7 @@
 #include <linux/mount.h>
 #include <linux/nfs4_mount.h>
 #include <linux/nfs_fs.h>
+#include <linux/nfs_ssc.h>
 #include "delegation.h"
 #include "internal.h"
 #include "nfs4_fs.h"
@@ -273,6 +274,10 @@ static int __init init_nfs_v4(void)
 	err = nfs4_xattr_cache_init();
 	if (err)
 		goto out2;
+
+	err = nfs42_ssc_register_ops();
+	if (err)
+		goto out2;
 #endif
 
 	err = nfs4_register_sysctl();
@@ -297,6 +302,7 @@ static void __exit exit_nfs_v4(void)
 	unregister_nfs_version(&nfs_v4);
 #ifdef CONFIG_NFS_V4_2
 	nfs4_xattr_cache_exit();
+	nfs42_ssc_unregister_ops();
 #endif
 	nfs4_unregister_sysctl();
 	nfs_idmap_quit();
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 7a70287f21a2..c6324e2de1ae 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -57,6 +57,7 @@
 #include <linux/rcupdate.h>
 
 #include <linux/uaccess.h>
+#include <linux/nfs_ssc.h>
 
 #include "nfs4_fs.h"
 #include "callback.h"
@@ -85,6 +86,11 @@
 };
 EXPORT_SYMBOL_GPL(nfs_sops);
 
+const struct nfs_ssc_client_ops nfs_ssc_clnt_ops_tbl = {
+	.sco_owner = THIS_MODULE,
+	.sco_sb_deactive = nfs_sb_deactive,
+};
+
 #if IS_ENABLED(CONFIG_NFS_V4)
 static int __init register_nfs4_fs(void)
 {
@@ -106,6 +112,16 @@ static void unregister_nfs4_fs(void)
 }
 #endif
 
+static int nfs_ssc_register_ops(void)
+{
+	return nfs_ssc_register(&nfs_ssc_clnt_ops_tbl);
+}
+
+static void nfs_ssc_unregister_ops(void)
+{
+	nfs_ssc_unregister(&nfs_ssc_clnt_ops_tbl);
+}
+
 static struct shrinker acl_shrinker = {
 	.count_objects	= nfs_access_cache_count,
 	.scan_objects	= nfs_access_cache_scan,
@@ -133,6 +149,9 @@ int __init register_nfs_fs(void)
 	ret = register_shrinker(&acl_shrinker);
 	if (ret < 0)
 		goto error_3;
+	ret = nfs_ssc_register_ops();
+	if (ret < 0)
+		goto error_3;
 	return 0;
 error_3:
 	nfs_unregister_sysctl();
@@ -152,6 +171,7 @@ void __exit unregister_nfs_fs(void)
 	unregister_shrinker(&acl_shrinker);
 	nfs_unregister_sysctl();
 	unregister_nfs4_fs();
+	nfs_ssc_unregister_ops();
 	unregister_filesystem(&nfs_fs_type);
 }
 
diff --git a/fs/nfs_common/Makefile b/fs/nfs_common/Makefile
index 4bebe834c009..fa82f5aaa6d9 100644
--- a/fs/nfs_common/Makefile
+++ b/fs/nfs_common/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_NFS_ACL_SUPPORT) += nfs_acl.o
 nfs_acl-objs := nfsacl.o
 
 obj-$(CONFIG_GRACE_PERIOD) += grace.o
+obj-$(CONFIG_GRACE_PERIOD) += nfs_ssc.o
diff --git a/fs/nfs_common/nfs_ssc.c b/fs/nfs_common/nfs_ssc.c
new file mode 100644
index 000000000000..60b875c4fb03
--- /dev/null
+++ b/fs/nfs_common/nfs_ssc.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * fs/nfs_common/nfs_ssc_comm.c
+ *
+ * Helper for knfsd's SSC to access ops in NFS client modules
+ *
+ * Author: Dai Ngo <dai.ngo@oracle.com>
+ *
+ * Copyright (c) 2020, Oracle and/or its affiliates.
+ */
+
+#include <linux/module.h>
+#include <linux/fs.h>
+#include <linux/nfs_ssc.h>
+#include "../nfs/nfs4_fs.h"
+
+MODULE_LICENSE("GPL");
+
+/*
+ * NFS_FS
+ */
+static void nfs_sb_deactive_def(struct super_block *sb);
+
+static struct nfs_ssc_client_ops nfs_ssc_clnt_ops_def = {
+	.sco_owner = THIS_MODULE,
+	.sco_sb_deactive = nfs_sb_deactive_def,
+};
+
+/*
+ * NFS_V4
+ */
+static struct file *nfs42_ssc_open_def(struct vfsmount *ss_mnt,
+		struct nfs_fh *src_fh, nfs4_stateid *stateid);
+static void nfs42_ssc_close_def(struct file *filep);
+
+static struct nfs4_ssc_client_ops nfs4_ssc_clnt_ops_def = {
+	.sco_owner = THIS_MODULE,
+	.sco_open = nfs42_ssc_open_def,
+	.sco_close = nfs42_ssc_close_def,
+};
+
+
+struct nfs_ssc_client_ops_tbl nfs_ssc_client_tbl = {
+	.ssc_nfs4_ops	= &nfs4_ssc_clnt_ops_def,
+	.ssc_nfs_ops = &nfs_ssc_clnt_ops_def
+};
+EXPORT_SYMBOL_GPL(nfs_ssc_client_tbl);
+
+
+static struct file *nfs42_ssc_open_def(struct vfsmount *ss_mnt,
+		struct nfs_fh *src_fh, nfs4_stateid *stateid)
+{
+	return ERR_PTR(-EIO);
+}
+
+static void nfs42_ssc_close_def(struct file *filep)
+{
+}
+
+#ifdef CONFIG_NFS_V4_2
+/**
+ * nfs42_ssc_register - install the NFS_V4 client ops in the nfs_ssc_client_tbl
+ * @ops: NFS_V4 ops to be installed
+ *
+ * Return values:
+ *   On success, return 0
+ *   %-EINVAL  if validation check fails
+ */
+int nfs42_ssc_register(const struct nfs4_ssc_client_ops *ops)
+{
+	if (ops == NULL || ops->sco_open == NULL || ops->sco_close == NULL)
+		return -EINVAL;
+	nfs_ssc_client_tbl.ssc_nfs4_ops = ops;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(nfs42_ssc_register);
+
+/**
+ * nfs42_ssc_unregister - uninstall the NFS_V4 client ops from
+ *				the nfs_ssc_client_tbl
+ * @ops: ops to be uninstalled
+ *
+ * Return values:
+ *   None
+ */
+void nfs42_ssc_unregister(const struct nfs4_ssc_client_ops *ops)
+{
+	if (nfs_ssc_client_tbl.ssc_nfs4_ops != ops)
+		return;
+
+	nfs_ssc_client_tbl.ssc_nfs4_ops = &nfs4_ssc_clnt_ops_def;
+}
+EXPORT_SYMBOL_GPL(nfs42_ssc_unregister);
+#endif /* CONFIG_NFS_V4_2 */
+
+/*
+ * NFS_FS
+ */
+static void nfs_sb_deactive_def(struct super_block *sb)
+{
+}
+
+#ifdef CONFIG_NFS_V4_2
+/**
+ * nfs_ssc_register - install the NFS_FS client ops in the nfs_ssc_client_tbl
+ * @ops: NFS_FS ops to be installed
+ *
+ * Return values:
+ *   On success, return 0
+ *   %-EINVAL  if validation check fails
+ */
+int nfs_ssc_register(const struct nfs_ssc_client_ops *ops)
+{
+	if (ops == NULL || ops->sco_sb_deactive == NULL)
+		return -EINVAL;
+	nfs_ssc_client_tbl.ssc_nfs_ops = ops;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(nfs_ssc_register);
+
+/**
+ * nfs_ssc_unregister - uninstall the NFS_FS client ops from
+ *				the nfs_ssc_client_tbl
+ * @ops: ops to be uninstalled
+ *
+ * Return values:
+ *   None
+ */
+void nfs_ssc_unregister(const struct nfs_ssc_client_ops *ops)
+{
+	if (nfs_ssc_client_tbl.ssc_nfs_ops != ops)
+		return;
+	nfs_ssc_client_tbl.ssc_nfs_ops = &nfs_ssc_clnt_ops_def;
+}
+EXPORT_SYMBOL_GPL(nfs_ssc_unregister);
+#endif /* CONFIG_NFS_V4_2 */
diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
index 99d2cae91bd6..f368f3215f88 100644
--- a/fs/nfsd/Kconfig
+++ b/fs/nfsd/Kconfig
@@ -136,7 +136,7 @@ config NFSD_FLEXFILELAYOUT
 
 config NFSD_V4_2_INTER_SSC
 	bool "NFSv4.2 inter server to server COPY"
-	depends on NFSD_V4 && NFS_V4_1 && NFS_V4_2 && NFS_FS=y
+	depends on NFSD_V4 && NFS_V4_1 && NFS_V4_2
 	help
 	  This option enables support for NFSv4.2 inter server to
 	  server copy where the destination server calls the NFSv4.2
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index eaf50eafa935..84e10aef1417 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -38,6 +38,7 @@
 #include <linux/slab.h>
 #include <linux/kthread.h>
 #include <linux/sunrpc/addr.h>
+#include <linux/nfs_ssc.h>
 
 #include "idmap.h"
 #include "cache.h"
@@ -1247,7 +1248,7 @@ extern struct file *nfs42_ssc_open(struct vfsmount *ss_mnt,
 static void
 nfsd4_interssc_disconnect(struct vfsmount *ss_mnt)
 {
-	nfs_sb_deactive(ss_mnt->mnt_sb);
+	nfs_do_sb_deactive(ss_mnt->mnt_sb);
 	mntput(ss_mnt);
 }
 
diff --git a/include/linux/nfs_ssc.h b/include/linux/nfs_ssc.h
new file mode 100644
index 000000000000..45a763bd6b0b
--- /dev/null
+++ b/include/linux/nfs_ssc.h
@@ -0,0 +1,77 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * include/linux/nfs_ssc.h
+ *
+ * Author: Dai Ngo <dai.ngo@oracle.com>
+ *
+ * Copyright (c) 2020, Oracle and/or its affiliates.
+ */
+
+#include <linux/nfs_fs.h>
+
+extern struct nfs_ssc_client_ops_tbl nfs_ssc_client_tbl;
+
+/*
+ * NFS_V4
+ */
+struct nfs4_ssc_client_ops {
+	struct module *sco_owner;
+	struct file *(*sco_open)(struct vfsmount *ss_mnt,
+		struct nfs_fh *src_fh, nfs4_stateid *stateid);
+	void (*sco_close)(struct file *filep);
+};
+
+/*
+ * NFS_FS
+ */
+struct nfs_ssc_client_ops {
+	struct module *sco_owner;
+	void (*sco_sb_deactive)(struct super_block *sb);
+};
+
+struct nfs_ssc_client_ops_tbl {
+	const struct nfs4_ssc_client_ops *ssc_nfs4_ops;
+	const struct nfs_ssc_client_ops *ssc_nfs_ops;
+};
+
+extern int nfs42_ssc_register_ops(void);
+extern void nfs42_ssc_unregister_ops(void);
+
+extern int nfs42_ssc_register(const struct nfs4_ssc_client_ops *ops);
+extern void nfs42_ssc_unregister(const struct nfs4_ssc_client_ops *ops);
+
+#ifdef CONFIG_NFSD_V4_2_INTER_SSC
+static inline struct file *nfs42_ssc_open(struct vfsmount *ss_mnt,
+		struct nfs_fh *src_fh, nfs4_stateid *stateid)
+{
+	struct file *file;
+
+	if (!try_module_get(nfs_ssc_client_tbl.ssc_nfs4_ops->sco_owner))
+		return ERR_PTR(-EIO);
+	file = (*nfs_ssc_client_tbl.ssc_nfs4_ops->sco_open)(ss_mnt, src_fh, stateid);
+	module_put(nfs_ssc_client_tbl.ssc_nfs4_ops->sco_owner);
+	return file;
+}
+
+static inline void nfs42_ssc_close(struct file *filep)
+{
+	if (!try_module_get(nfs_ssc_client_tbl.ssc_nfs4_ops->sco_owner))
+		return;
+	(*nfs_ssc_client_tbl.ssc_nfs4_ops->sco_close)(filep);
+	module_put(nfs_ssc_client_tbl.ssc_nfs4_ops->sco_owner);
+}
+#endif
+
+/*
+ * NFS_FS
+ */
+extern int nfs_ssc_register(const struct nfs_ssc_client_ops *ops);
+extern void nfs_ssc_unregister(const struct nfs_ssc_client_ops *ops);
+
+static inline void nfs_do_sb_deactive(struct super_block *sb)
+{
+	if (!try_module_get(nfs_ssc_client_tbl.ssc_nfs_ops->sco_owner))
+		return;
+	(*nfs_ssc_client_tbl.ssc_nfs_ops->sco_sb_deactive)(sb);
+	module_put(nfs_ssc_client_tbl.ssc_nfs_ops->sco_owner);
+}
-- 
1.8.3.1

Re: [PATCH v2 1/1] NFSv4.2: Fix NFS4ERR_STALE error when doing inter server copy

[kernel test robot]

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

Hi Dai,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on nfs/linux-next]
[also build test WARNING on nfsd/nfsd-next v5.9-rc8 next-20201007]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Dai-Ngo/NFSv4-2-Fix-NFS4ERR_STALE-with-inter-server-copy/20201008-092931
base:   git://git.linux-nfs.org/projects/trondmy/linux-nfs.git linux-next
config: sparc64-randconfig-s031-20201008 (attached as .config)
compiler: sparc64-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.2-218-gc0e96d6d-dirty
        # https://github.com/0day-ci/linux/commit/589c5501bac22055fafe42825de27f3cc1c82626
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dai-Ngo/NFSv4-2-Fix-NFS4ERR_STALE-with-inter-server-copy/20201008-092931
        git checkout 589c5501bac22055fafe42825de27f3cc1c82626
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=sparc64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> fs/nfs/nfs4file.c:27:21: warning: '__nfs42_ssc_open' used but never defined
      27 | static struct file *__nfs42_ssc_open(struct vfsmount *ss_mnt,
         |                     ^~~~~~~~~~~~~~~~
>> fs/nfs/nfs4file.c:29:13: warning: '__nfs42_ssc_close' used but never defined
      29 | static void __nfs42_ssc_close(struct file *filep);
         |             ^~~~~~~~~~~~~~~~~

vim +/__nfs42_ssc_open +27 fs/nfs/nfs4file.c

    26	
  > 27	static struct file *__nfs42_ssc_open(struct vfsmount *ss_mnt,
    28			struct nfs_fh *src_fh, nfs4_stateid *stateid);
  > 29	static void __nfs42_ssc_close(struct file *filep);
    30	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28470 bytes --]

Re: [PATCH v2 1/1] NFSv4.2: Fix NFS4ERR_STALE error when doing inter server copy

[kernel test robot]

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

Hi Dai,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on nfs/linux-next]
[also build test WARNING on nfsd/nfsd-next v5.9-rc8 next-20201007]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Dai-Ngo/NFSv4-2-Fix-NFS4ERR_STALE-with-inter-server-copy/20201008-092931
base:   git://git.linux-nfs.org/projects/trondmy/linux-nfs.git linux-next
config: i386-randconfig-s001-20201008 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.2-218-gc0e96d6d-dirty
        # https://github.com/0day-ci/linux/commit/589c5501bac22055fafe42825de27f3cc1c82626
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dai-Ngo/NFSv4-2-Fix-NFS4ERR_STALE-with-inter-server-copy/20201008-092931
        git checkout 589c5501bac22055fafe42825de27f3cc1c82626
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

echo
echo "sparse warnings: (new ones prefixed by >>)"
echo
>> fs/nfs/super.c:89:33: sparse: sparse: symbol 'nfs_ssc_clnt_ops_tbl' was not declared. Should it be static?
   fs/nfs/super.c:1114:49: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct rpc_xprt *xprt1 @@     got struct rpc_xprt [noderef] __rcu *cl_xprt @@
   fs/nfs/super.c:1114:49: sparse:     expected struct rpc_xprt *xprt1
   fs/nfs/super.c:1114:49: sparse:     got struct rpc_xprt [noderef] __rcu *cl_xprt
   fs/nfs/super.c:1115:49: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct rpc_xprt *xprt2 @@     got struct rpc_xprt [noderef] __rcu *cl_xprt @@
   fs/nfs/super.c:1115:49: sparse:     expected struct rpc_xprt *xprt2
   fs/nfs/super.c:1115:49: sparse:     got struct rpc_xprt [noderef] __rcu *cl_xprt
   fs/nfs/super.c:1335:14: sparse: sparse: symbol 'nfs_idmap_cache_timeout' was not declared. Should it be static?
--
>> fs/nfs/nfs4file.c:31:34: sparse: sparse: symbol 'nfs4_ssc_clnt_ops_tbl' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34975 bytes --]

[RFC PATCH] NFSv4.2: nfs_ssc_clnt_ops_tbl can be static

[kernel test robot]

Signed-off-by: kernel test robot <lkp@intel.com>
---
 nfs4file.c |    2 +-
 super.c    |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
index ea42c39bc2ffd..124876dfd6447 100644
--- a/fs/nfs/nfs4file.c
+++ b/fs/nfs/nfs4file.c
@@ -28,7 +28,7 @@ static struct file *__nfs42_ssc_open(struct vfsmount *ss_mnt,
 		struct nfs_fh *src_fh, nfs4_stateid *stateid);
 static void __nfs42_ssc_close(struct file *filep);
 
-const struct nfs4_ssc_client_ops nfs4_ssc_clnt_ops_tbl = {
+static const struct nfs4_ssc_client_ops nfs4_ssc_clnt_ops_tbl = {
 	.sco_owner = THIS_MODULE,
 	.sco_open = __nfs42_ssc_open,
 	.sco_close = __nfs42_ssc_close,
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index c6324e2de1ae5..65636fef6a006 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -86,7 +86,7 @@ const struct super_operations nfs_sops = {
 };
 EXPORT_SYMBOL_GPL(nfs_sops);
 
-const struct nfs_ssc_client_ops nfs_ssc_clnt_ops_tbl = {
+static const struct nfs_ssc_client_ops nfs_ssc_clnt_ops_tbl = {
 	.sco_owner = THIS_MODULE,
 	.sco_sb_deactive = nfs_sb_deactive,
 };

Re: [PATCH v2 1/1] NFSv4.2: Fix NFS4ERR_STALE error when doing inter server copy

[kernel test robot]

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

Hi Dai,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on nfs/linux-next]
[also build test ERROR on nfsd/nfsd-next v5.9-rc8 next-20201007]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Dai-Ngo/NFSv4-2-Fix-NFS4ERR_STALE-with-inter-server-copy/20201008-092931
base:   git://git.linux-nfs.org/projects/trondmy/linux-nfs.git linux-next
config: nds32-defconfig (attached as .config)
compiler: nds32le-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/589c5501bac22055fafe42825de27f3cc1c82626
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dai-Ngo/NFSv4-2-Fix-NFS4ERR_STALE-with-inter-server-copy/20201008-092931
        git checkout 589c5501bac22055fafe42825de27f3cc1c82626
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=nds32 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   nds32le-linux-ld: fs/nfs/super.o: in function `register_nfs_fs':
   super.c:(.init.text+0x72): undefined reference to `nfs_ssc_register'
   nds32le-linux-ld: super.c:(.init.text+0x76): undefined reference to `nfs_ssc_register'
>> nds32le-linux-ld: fs/nfs/nfs4file.o:(.rodata+0x84): undefined reference to `__nfs42_ssc_open'
>> nds32le-linux-ld: fs/nfs/nfs4file.o:(.rodata+0x88): undefined reference to `__nfs42_ssc_close'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 10917 bytes --]

Re: [PATCH v2 0/1] NFSv4.2: Fix NFS4ERR_STALE with inter server copy

[J. Bruce Fields]

On Wed, Oct 07, 2020 at 09:25:12PM -0400, Dai Ngo wrote:
> This cover email is intended for including my test results.
> 
> This patch adds the ops table in nfs_common for knfsd to access
> NFS client modules without calling these functions directly.
> 
> The client module registers their functions and deregisters them
> when the module is loaded and unloaded respectively.
> 
>  fs/nfs/nfs4file.c       |  44 ++++++++++++--
>  fs/nfs/nfs4super.c      |   6 ++
>  fs/nfs/super.c          |  20 +++++++
>  fs/nfs_common/Makefile  |   1 +
>  fs/nfs_common/nfs_ssc.c | 136 +++++++++++++++++++++++++++++++++++++++++++
>  fs/nfsd/Kconfig         |   2 +-
>  fs/nfsd/nfs4proc.c      |   3 +-
>  include/linux/nfs_ssc.h |  77 ++++++++++++++++++++++++
>  8 files changed, 281 insertions(+), 8 deletions(-)
> 
> Test Results:
> 
> Upstream version used for testing:  5.9-rc5
> 
> |----------------------------------------------------------|
> |  NFSD  |  NFS_FS  |  NFS_V4  |       RESULTS             |
> |----------------------------------------------------------|
> |   m    |    y     |    m     | inter server copy OK      |
> |----------------------------------------------------------|
> |   m    |    m     |    m     | inter server copy OK      |
> |----------------------------------------------------------|
> |   m    |    m     |   y (m)  | inter server copy OK      |
> |----------------------------------------------------------|
> |   m    |    y     |    y     | inter server copy OK      |
> |----------------------------------------------------------|
> |   m    |    n     |    n     | NFS4ERR_STALE error       |
> |----------------------------------------------------------|

Why are there two?  And how are you getting that NFS4ERR_STALE case?
NFSD_V4_2_INTER_SSC depends on NFS_FS, so it shouldn't be possible to
build server-to-server-copy support without building the client.  And if
you don't build NFSD_V4_2_INTER_SSC at all, then I think it should be
returning NOTSUPP instead of STALE.

--b.

> |----------------------------------------------------------|
> |  NFSD  |  NFS_FS  |  NFS_V4  |        RESULTS            |
> |----------------------------------------------------------|
> |   y    |    y     |    m     | inter server copy OK      |
> |----------------------------------------------------------|
> |   y    |    m     |    m     | inter server copy OK      |
> |----------------------------------------------------------|
> |   y    |    m     |   y (m)  | inter server copy OK      |
> |----------------------------------------------------------|
> |   y    |    y     |    y     | inter server copy OK      |
> |----------------------------------------------------------|
> |   y    |    n     |    n     | NFS4ERR_STALE error       |
> |----------------------------------------------------------|
> 
> NOTE:
> When NFS_V4=y and NFS_FS=m, the build process automatically builds
> with NFS_V4=m and ignores the setting NFS_V4=y in the config file. 
> 
> This probably due to NFS_V4 in fs/nfs/Kconfig is configured to
> depend on NFS_FS.

Re: [PATCH v2 0/1] NFSv4.2: Fix NFS4ERR_STALE with inter server copy

[Dai Ngo]

On 10/8/20 10:58 AM, J. Bruce Fields wrote:
> On Wed, Oct 07, 2020 at 09:25:12PM -0400, Dai Ngo wrote:
>> This cover email is intended for including my test results.
>>
>> This patch adds the ops table in nfs_common for knfsd to access
>> NFS client modules without calling these functions directly.
>>
>> The client module registers their functions and deregisters them
>> when the module is loaded and unloaded respectively.
>>
>>   fs/nfs/nfs4file.c       |  44 ++++++++++++--
>>   fs/nfs/nfs4super.c      |   6 ++
>>   fs/nfs/super.c          |  20 +++++++
>>   fs/nfs_common/Makefile  |   1 +
>>   fs/nfs_common/nfs_ssc.c | 136 +++++++++++++++++++++++++++++++++++++++++++
>>   fs/nfsd/Kconfig         |   2 +-
>>   fs/nfsd/nfs4proc.c      |   3 +-
>>   include/linux/nfs_ssc.h |  77 ++++++++++++++++++++++++
>>   8 files changed, 281 insertions(+), 8 deletions(-)
>>
>> Test Results:
>>
>> Upstream version used for testing:  5.9-rc5
>>
>> |----------------------------------------------------------|
>> |  NFSD  |  NFS_FS  |  NFS_V4  |       RESULTS             |
>> |----------------------------------------------------------|
>> |   m    |    y     |    m     | inter server copy OK      |
>> |----------------------------------------------------------|
>> |   m    |    m     |    m     | inter server copy OK      |
>> |----------------------------------------------------------|
>> |   m    |    m     |   y (m)  | inter server copy OK      |
>> |----------------------------------------------------------|
>> |   m    |    y     |    y     | inter server copy OK      |
>> |----------------------------------------------------------|
>> |   m    |    n     |    n     | NFS4ERR_STALE error       |
>> |----------------------------------------------------------|
> Why are there two?

Can you clarify this question?

>   And how are you getting that NFS4ERR_STALE case?
> NFSD_V4_2_INTER_SSC depends on NFS_FS, so it shouldn't be possible to
> build server-to-server-copy support without building the client.  And if
> you don't build NFSD_V4_2_INTER_SSC at all, then I think it should be
> returning NOTSUPP instead of STALE.

In the case where CONFIG_NFSD_V4_2_INTER_SSC is not set, when the inter
server copy fails in nfsd4_putfh, before nfsd4_copy, with nfserr_stale
returned from fs_verify. There is no code to handle this error and it is
returned to the client. This is the existing behavior, the patch does not
attempt to make any change in this area since there are more to fixes in
this area and it can be done in separate patches.

For example, when NFS4ERR_STALE happens, the file was left created with
size 0.

and the 'refcount_t: underflow; use-after-free' problem:

Oct  4 20:21:31 nfsvmf24 kernel: refcount_t: underflow; use-after-free.
Oct  4 20:21:31 nfsvmf24 kernel: WARNING: CPU: 0 PID: 7 at lib/refcount.c:28 refcount_warn_saturate+0xae/0xf0
Oct  4 20:21:31 nfsvmf24 kernel: Modules linked in: rpcsec_gss_krb5 nfsv4 dns_resolver xt_REDIRECT xt_nat ip6table_nat ip6_tables iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 rfkill btrfs blake2b_generic xor zstd_compress raid6_pq sb_edac intel_powerclamp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel crypto_simd cryptd glue_helper pcspkr sg video i2c_piix4 nfsd auth_rpcgss ip_tables xfs libcrc32c sd_mod t10_pi ahci libahci libata e1000 crc32c_intel serio_raw dm_mirror dm_region_hash dm_log dm_mod
Oct  4 20:21:31 nfsvmf24 kernel: CPU: 0 PID: 7 Comm: kworker/u2:0 Not tainted 5.9.0-rc5+ #4
Oct  4 20:21:31 nfsvmf24 kernel: Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
Oct  4 20:21:31 nfsvmf24 kernel: Workqueue: rpciod rpc_async_schedule
Oct  4 20:21:31 nfsvmf24 kernel: RIP: 0010:refcount_warn_saturate+0xae/0xf0
Oct  4 20:21:31 nfsvmf24 kernel: Code: d4 dc 16 01 01 e8 07 7b bf ff 0f 0b 5d c3 80 3d c1 dc 16 01 00 75 91 48 c7 c7 70 0b 17 8d c6 05 b1 dc 16 01 01 e8 e7 7a bf ff <0f> 0b 5d c3 80 3d 9f dc 16 01 00 0f 85 6d ff ff ff 48 c7 c7 c8 0b
Oct  4 20:21:31 nfsvmf24 kernel: RSP: 0018:ffffa589c0043d68 EFLAGS: 00010286
Oct  4 20:21:31 nfsvmf24 kernel: RAX: 0000000000000000 RBX: 0000000000002a81 RCX: 0000000000000027
Oct  4 20:21:31 nfsvmf24 kernel: RDX: 0000000000000027 RSI: 0000000000000086 RDI: ffff996c17c18c48
Oct  4 20:21:31 nfsvmf24 kernel: RBP: ffffa589c0043d68 R08: ffff996c17c18c40 R09: 0000000000000004
Oct  4 20:21:31 nfsvmf24 kernel: R10: 0000000000000000 R11: 0000000000000001 R12: ffff996c14c7e470
Oct  4 20:21:31 nfsvmf24 kernel: R13: ffff996c14ca4510 R14: ffff996c0eef6130 R15: 0000000000000000
Oct  4 20:21:31 nfsvmf24 kernel: FS:  0000000000000000(0000) GS:ffff996c17c00000(0000) knlGS:0000000000000000
Oct  4 20:21:31 nfsvmf24 kernel: CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
Oct  4 20:21:31 nfsvmf24 kernel: CR2: 00007fbb6fada000 CR3: 000000020d3fe000 CR4: 00000000000406f0
Oct  4 20:21:31 nfsvmf24 kernel: Call Trace:
Oct  4 20:21:31 nfsvmf24 kernel: nfs4_put_copy+0x3c/0x40 [nfsd]
Oct  4 20:21:31 nfsvmf24 kernel: nfsd4_cb_offload_release+0x15/0x20 [nfsd]
Oct  4 20:21:31 nfsvmf24 kernel: nfsd41_destroy_cb+0x3a/0x50 [nfsd]
Oct  4 20:21:31 nfsvmf24 kernel: nfsd4_cb_release+0x2b/0x30 [nfsd]
Oct  4 20:21:31 nfsvmf24 kernel: rpc_free_task+0x40/0x70
Oct  4 20:21:31 nfsvmf24 kernel: __rpc_execute+0x3c9/0x3e0
Oct  4 20:21:31 nfsvmf24 kernel: ? __switch_to_asm+0x36/0x70
Oct  4 20:21:31 nfsvmf24 kernel: rpc_async_schedule+0x30/0x50
Oct  4 20:21:31 nfsvmf24 kernel: process_one_work+0x1b4/0x380
Oct  4 20:21:31 nfsvmf24 kernel: worker_thread+0x50/0x3d0
Oct  4 20:21:31 nfsvmf24 kernel: kthread+0x114/0x150

Thanks,
-Dai

>
> --b.
>
>> |----------------------------------------------------------|
>> |  NFSD  |  NFS_FS  |  NFS_V4  |        RESULTS            |
>> |----------------------------------------------------------|
>> |   y    |    y     |    m     | inter server copy OK      |
>> |----------------------------------------------------------|
>> |   y    |    m     |    m     | inter server copy OK      |
>> |----------------------------------------------------------|
>> |   y    |    m     |   y (m)  | inter server copy OK      |
>> |----------------------------------------------------------|
>> |   y    |    y     |    y     | inter server copy OK      |
>> |----------------------------------------------------------|
>> |   y    |    n     |    n     | NFS4ERR_STALE error       |
>> |----------------------------------------------------------|
>>
>> NOTE:
>> When NFS_V4=y and NFS_FS=m, the build process automatically builds
>> with NFS_V4=m and ignores the setting NFS_V4=y in the config file.
>>
>> This probably due to NFS_V4 in fs/nfs/Kconfig is configured to
>> depend on NFS_FS.

Re: [PATCH v2 0/1] NFSv4.2: Fix NFS4ERR_STALE with inter server copy

[J. Bruce Fields]

On Thu, Oct 08, 2020 at 11:29:45AM -0700, Dai Ngo wrote:
> 
> On 10/8/20 10:58 AM, J. Bruce Fields wrote:
> >On Wed, Oct 07, 2020 at 09:25:12PM -0400, Dai Ngo wrote:
> >>This cover email is intended for including my test results.
> >>
> >>This patch adds the ops table in nfs_common for knfsd to access
> >>NFS client modules without calling these functions directly.
> >>
> >>The client module registers their functions and deregisters them
> >>when the module is loaded and unloaded respectively.
> >>
> >>  fs/nfs/nfs4file.c       |  44 ++++++++++++--
> >>  fs/nfs/nfs4super.c      |   6 ++
> >>  fs/nfs/super.c          |  20 +++++++
> >>  fs/nfs_common/Makefile  |   1 +
> >>  fs/nfs_common/nfs_ssc.c | 136 +++++++++++++++++++++++++++++++++++++++++++
> >>  fs/nfsd/Kconfig         |   2 +-
> >>  fs/nfsd/nfs4proc.c      |   3 +-
> >>  include/linux/nfs_ssc.h |  77 ++++++++++++++++++++++++
> >>  8 files changed, 281 insertions(+), 8 deletions(-)
> >>
> >>Test Results:
> >>
> >>Upstream version used for testing:  5.9-rc5
> >>
> >>|----------------------------------------------------------|
> >>|  NFSD  |  NFS_FS  |  NFS_V4  |       RESULTS             |
> >>|----------------------------------------------------------|
> >>|   m    |    y     |    m     | inter server copy OK      |
> >>|----------------------------------------------------------|
> >>|   m    |    m     |    m     | inter server copy OK      |
> >>|----------------------------------------------------------|
> >>|   m    |    m     |   y (m)  | inter server copy OK      |
> >>|----------------------------------------------------------|
> >>|   m    |    y     |    y     | inter server copy OK      |
> >>|----------------------------------------------------------|
> >>|   m    |    n     |    n     | NFS4ERR_STALE error       |
> >>|----------------------------------------------------------|
> >Why are there two?
> 
> Can you clarify this question?

Sorry, I meant: why are there two copies of this table?

OK, I see now, the first is for the NFSD=m case, the second is for
NFSD=y.

> >  And how are you getting that NFS4ERR_STALE case?
> >NFSD_V4_2_INTER_SSC depends on NFS_FS, so it shouldn't be possible to
> >build server-to-server-copy support without building the client.  And if
> >you don't build NFSD_V4_2_INTER_SSC at all, then I think it should be
> >returning NOTSUPP instead of STALE.
> 
> In the case where CONFIG_NFSD_V4_2_INTER_SSC is not set, when the inter
> server copy fails in nfsd4_putfh, before nfsd4_copy, with nfserr_stale
> returned from fs_verify. There is no code to handle this error and it is
> returned to the client. This is the existing behavior, the patch does not
> attempt to make any change in this area since there are more to fixes in
> this area and it can be done in separate patches.

OK.

> For example, when NFS4ERR_STALE happens, the file was left created with
> size 0.

Well, that's bad.  Is this the case where CONFIG_NFSD_V4_2_INTER_SSC is
set, or is this some other failure?

(Also, are you checking this on the server side?  There's a known
problem where the copy completes succesfully but the client assumes its
cache is still valid and (incorrectly) reports the destination file is
still empty.)

--b.

> and the 'refcount_t: underflow; use-after-free' problem:
> 
> Oct  4 20:21:31 nfsvmf24 kernel: refcount_t: underflow; use-after-free.
> Oct  4 20:21:31 nfsvmf24 kernel: WARNING: CPU: 0 PID: 7 at lib/refcount.c:28 refcount_warn_saturate+0xae/0xf0
> Oct  4 20:21:31 nfsvmf24 kernel: Modules linked in: rpcsec_gss_krb5 nfsv4 dns_resolver xt_REDIRECT xt_nat ip6table_nat ip6_tables iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 rfkill btrfs blake2b_generic xor zstd_compress raid6_pq sb_edac intel_powerclamp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel crypto_simd cryptd glue_helper pcspkr sg video i2c_piix4 nfsd auth_rpcgss ip_tables xfs libcrc32c sd_mod t10_pi ahci libahci libata e1000 crc32c_intel serio_raw dm_mirror dm_region_hash dm_log dm_mod
> Oct  4 20:21:31 nfsvmf24 kernel: CPU: 0 PID: 7 Comm: kworker/u2:0 Not tainted 5.9.0-rc5+ #4
> Oct  4 20:21:31 nfsvmf24 kernel: Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
> Oct  4 20:21:31 nfsvmf24 kernel: Workqueue: rpciod rpc_async_schedule
> Oct  4 20:21:31 nfsvmf24 kernel: RIP: 0010:refcount_warn_saturate+0xae/0xf0
> Oct  4 20:21:31 nfsvmf24 kernel: Code: d4 dc 16 01 01 e8 07 7b bf ff 0f 0b 5d c3 80 3d c1 dc 16 01 00 75 91 48 c7 c7 70 0b 17 8d c6 05 b1 dc 16 01 01 e8 e7 7a bf ff <0f> 0b 5d c3 80 3d 9f dc 16 01 00 0f 85 6d ff ff ff 48 c7 c7 c8 0b
> Oct  4 20:21:31 nfsvmf24 kernel: RSP: 0018:ffffa589c0043d68 EFLAGS: 00010286
> Oct  4 20:21:31 nfsvmf24 kernel: RAX: 0000000000000000 RBX: 0000000000002a81 RCX: 0000000000000027
> Oct  4 20:21:31 nfsvmf24 kernel: RDX: 0000000000000027 RSI: 0000000000000086 RDI: ffff996c17c18c48
> Oct  4 20:21:31 nfsvmf24 kernel: RBP: ffffa589c0043d68 R08: ffff996c17c18c40 R09: 0000000000000004
> Oct  4 20:21:31 nfsvmf24 kernel: R10: 0000000000000000 R11: 0000000000000001 R12: ffff996c14c7e470
> Oct  4 20:21:31 nfsvmf24 kernel: R13: ffff996c14ca4510 R14: ffff996c0eef6130 R15: 0000000000000000
> Oct  4 20:21:31 nfsvmf24 kernel: FS:  0000000000000000(0000) GS:ffff996c17c00000(0000) knlGS:0000000000000000
> Oct  4 20:21:31 nfsvmf24 kernel: CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> Oct  4 20:21:31 nfsvmf24 kernel: CR2: 00007fbb6fada000 CR3: 000000020d3fe000 CR4: 00000000000406f0
> Oct  4 20:21:31 nfsvmf24 kernel: Call Trace:
> Oct  4 20:21:31 nfsvmf24 kernel: nfs4_put_copy+0x3c/0x40 [nfsd]
> Oct  4 20:21:31 nfsvmf24 kernel: nfsd4_cb_offload_release+0x15/0x20 [nfsd]
> Oct  4 20:21:31 nfsvmf24 kernel: nfsd41_destroy_cb+0x3a/0x50 [nfsd]
> Oct  4 20:21:31 nfsvmf24 kernel: nfsd4_cb_release+0x2b/0x30 [nfsd]
> Oct  4 20:21:31 nfsvmf24 kernel: rpc_free_task+0x40/0x70
> Oct  4 20:21:31 nfsvmf24 kernel: __rpc_execute+0x3c9/0x3e0
> Oct  4 20:21:31 nfsvmf24 kernel: ? __switch_to_asm+0x36/0x70
> Oct  4 20:21:31 nfsvmf24 kernel: rpc_async_schedule+0x30/0x50
> Oct  4 20:21:31 nfsvmf24 kernel: process_one_work+0x1b4/0x380
> Oct  4 20:21:31 nfsvmf24 kernel: worker_thread+0x50/0x3d0
> Oct  4 20:21:31 nfsvmf24 kernel: kthread+0x114/0x150
> 
> Thanks,
> -Dai
> 
> >
> >--b.
> >
> >>|----------------------------------------------------------|
> >>|  NFSD  |  NFS_FS  |  NFS_V4  |        RESULTS            |
> >>|----------------------------------------------------------|
> >>|   y    |    y     |    m     | inter server copy OK      |
> >>|----------------------------------------------------------|
> >>|   y    |    m     |    m     | inter server copy OK      |
> >>|----------------------------------------------------------|
> >>|   y    |    m     |   y (m)  | inter server copy OK      |
> >>|----------------------------------------------------------|
> >>|   y    |    y     |    y     | inter server copy OK      |
> >>|----------------------------------------------------------|
> >>|   y    |    n     |    n     | NFS4ERR_STALE error       |
> >>|----------------------------------------------------------|
> >>
> >>NOTE:
> >>When NFS_V4=y and NFS_FS=m, the build process automatically builds
> >>with NFS_V4=m and ignores the setting NFS_V4=y in the config file.
> >>
> >>This probably due to NFS_V4 in fs/nfs/Kconfig is configured to
> >>depend on NFS_FS.

Re: [PATCH v2 0/1] NFSv4.2: Fix NFS4ERR_STALE with inter server copy

[Dai Ngo]

On 10/8/20 12:47 PM, J. Bruce Fields wrote:
> On Thu, Oct 08, 2020 at 11:29:45AM -0700, Dai Ngo wrote:
>> On 10/8/20 10:58 AM, J. Bruce Fields wrote:
>>> On Wed, Oct 07, 2020 at 09:25:12PM -0400, Dai Ngo wrote:
>>>> This cover email is intended for including my test results.
>>>>
>>>> This patch adds the ops table in nfs_common for knfsd to access
>>>> NFS client modules without calling these functions directly.
>>>>
>>>> The client module registers their functions and deregisters them
>>>> when the module is loaded and unloaded respectively.
>>>>
>>>>   fs/nfs/nfs4file.c       |  44 ++++++++++++--
>>>>   fs/nfs/nfs4super.c      |   6 ++
>>>>   fs/nfs/super.c          |  20 +++++++
>>>>   fs/nfs_common/Makefile  |   1 +
>>>>   fs/nfs_common/nfs_ssc.c | 136 +++++++++++++++++++++++++++++++++++++++++++
>>>>   fs/nfsd/Kconfig         |   2 +-
>>>>   fs/nfsd/nfs4proc.c      |   3 +-
>>>>   include/linux/nfs_ssc.h |  77 ++++++++++++++++++++++++
>>>>   8 files changed, 281 insertions(+), 8 deletions(-)
>>>>
>>>> Test Results:
>>>>
>>>> Upstream version used for testing:  5.9-rc5
>>>>
>>>> |----------------------------------------------------------|
>>>> |  NFSD  |  NFS_FS  |  NFS_V4  |       RESULTS             |
>>>> |----------------------------------------------------------|
>>>> |   m    |    y     |    m     | inter server copy OK      |
>>>> |----------------------------------------------------------|
>>>> |   m    |    m     |    m     | inter server copy OK      |
>>>> |----------------------------------------------------------|
>>>> |   m    |    m     |   y (m)  | inter server copy OK      |
>>>> |----------------------------------------------------------|
>>>> |   m    |    y     |    y     | inter server copy OK      |
>>>> |----------------------------------------------------------|
>>>> |   m    |    n     |    n     | NFS4ERR_STALE error       |
>>>> |----------------------------------------------------------|
>>> Why are there two?
>> Can you clarify this question?
> Sorry, I meant: why are there two copies of this table?
>
> OK, I see now, the first is for the NFSD=m case, the second is for
> NFSD=y.
>
>>>   And how are you getting that NFS4ERR_STALE case?
>>> NFSD_V4_2_INTER_SSC depends on NFS_FS, so it shouldn't be possible to
>>> build server-to-server-copy support without building the client.  And if
>>> you don't build NFSD_V4_2_INTER_SSC at all, then I think it should be
>>> returning NOTSUPP instead of STALE.
>> In the case where CONFIG_NFSD_V4_2_INTER_SSC is not set, when the inter
>> server copy fails in nfsd4_putfh, before nfsd4_copy, with nfserr_stale
>> returned from fs_verify. There is no code to handle this error and it is
>> returned to the client. This is the existing behavior, the patch does not
>> attempt to make any change in this area since there are more to fixes in
>> this area and it can be done in separate patches.
> OK.
>
>> For example, when NFS4ERR_STALE happens, the file was left created with
>> size 0.
> Well, that's bad.  Is this the case where CONFIG_NFSD_V4_2_INTER_SSC is
> set, or is this some other failure?

fs_verify always return nfserr_stale even when CONFIG_NFSD_V4_2_INTER_SSC
is set, that is the way the existing code works. With CONFIG_NFSD_V4_2_INTER_SSC
set, the code in nfsd4_putfh handles the error and ignores it if no_verify
is set. With CONFIG_NFSD_V4_2_INTER_SSC not set, the error is returned to
the client. So I think what should happens is if CONFIG_NFSD_V4_2_INTER_SSC
is not set, the operation should fail with appropriate error code and removes
the file.

>
> (Also, are you checking this on the server side?  There's a known
> problem where the copy completes succesfully but the client assumes its
> cache is still valid and (incorrectly) reports the destination file is
> still empty.)

I have not run into this case when CONFIG_NFSD_V4_2_INTER_SSC is set.

Thanks,
-Dai

>
> --b.
>
>> and the 'refcount_t: underflow; use-after-free' problem:
>>
>> Oct  4 20:21:31 nfsvmf24 kernel: refcount_t: underflow; use-after-free.
>> Oct  4 20:21:31 nfsvmf24 kernel: WARNING: CPU: 0 PID: 7 at lib/refcount.c:28 refcount_warn_saturate+0xae/0xf0
>> Oct  4 20:21:31 nfsvmf24 kernel: Modules linked in: rpcsec_gss_krb5 nfsv4 dns_resolver xt_REDIRECT xt_nat ip6table_nat ip6_tables iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 rfkill btrfs blake2b_generic xor zstd_compress raid6_pq sb_edac intel_powerclamp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel crypto_simd cryptd glue_helper pcspkr sg video i2c_piix4 nfsd auth_rpcgss ip_tables xfs libcrc32c sd_mod t10_pi ahci libahci libata e1000 crc32c_intel serio_raw dm_mirror dm_region_hash dm_log dm_mod
>> Oct  4 20:21:31 nfsvmf24 kernel: CPU: 0 PID: 7 Comm: kworker/u2:0 Not tainted 5.9.0-rc5+ #4
>> Oct  4 20:21:31 nfsvmf24 kernel: Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
>> Oct  4 20:21:31 nfsvmf24 kernel: Workqueue: rpciod rpc_async_schedule
>> Oct  4 20:21:31 nfsvmf24 kernel: RIP: 0010:refcount_warn_saturate+0xae/0xf0
>> Oct  4 20:21:31 nfsvmf24 kernel: Code: d4 dc 16 01 01 e8 07 7b bf ff 0f 0b 5d c3 80 3d c1 dc 16 01 00 75 91 48 c7 c7 70 0b 17 8d c6 05 b1 dc 16 01 01 e8 e7 7a bf ff <0f> 0b 5d c3 80 3d 9f dc 16 01 00 0f 85 6d ff ff ff 48 c7 c7 c8 0b
>> Oct  4 20:21:31 nfsvmf24 kernel: RSP: 0018:ffffa589c0043d68 EFLAGS: 00010286
>> Oct  4 20:21:31 nfsvmf24 kernel: RAX: 0000000000000000 RBX: 0000000000002a81 RCX: 0000000000000027
>> Oct  4 20:21:31 nfsvmf24 kernel: RDX: 0000000000000027 RSI: 0000000000000086 RDI: ffff996c17c18c48
>> Oct  4 20:21:31 nfsvmf24 kernel: RBP: ffffa589c0043d68 R08: ffff996c17c18c40 R09: 0000000000000004
>> Oct  4 20:21:31 nfsvmf24 kernel: R10: 0000000000000000 R11: 0000000000000001 R12: ffff996c14c7e470
>> Oct  4 20:21:31 nfsvmf24 kernel: R13: ffff996c14ca4510 R14: ffff996c0eef6130 R15: 0000000000000000
>> Oct  4 20:21:31 nfsvmf24 kernel: FS:  0000000000000000(0000) GS:ffff996c17c00000(0000) knlGS:0000000000000000
>> Oct  4 20:21:31 nfsvmf24 kernel: CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> Oct  4 20:21:31 nfsvmf24 kernel: CR2: 00007fbb6fada000 CR3: 000000020d3fe000 CR4: 00000000000406f0
>> Oct  4 20:21:31 nfsvmf24 kernel: Call Trace:
>> Oct  4 20:21:31 nfsvmf24 kernel: nfs4_put_copy+0x3c/0x40 [nfsd]
>> Oct  4 20:21:31 nfsvmf24 kernel: nfsd4_cb_offload_release+0x15/0x20 [nfsd]
>> Oct  4 20:21:31 nfsvmf24 kernel: nfsd41_destroy_cb+0x3a/0x50 [nfsd]
>> Oct  4 20:21:31 nfsvmf24 kernel: nfsd4_cb_release+0x2b/0x30 [nfsd]
>> Oct  4 20:21:31 nfsvmf24 kernel: rpc_free_task+0x40/0x70
>> Oct  4 20:21:31 nfsvmf24 kernel: __rpc_execute+0x3c9/0x3e0
>> Oct  4 20:21:31 nfsvmf24 kernel: ? __switch_to_asm+0x36/0x70
>> Oct  4 20:21:31 nfsvmf24 kernel: rpc_async_schedule+0x30/0x50
>> Oct  4 20:21:31 nfsvmf24 kernel: process_one_work+0x1b4/0x380
>> Oct  4 20:21:31 nfsvmf24 kernel: worker_thread+0x50/0x3d0
>> Oct  4 20:21:31 nfsvmf24 kernel: kthread+0x114/0x150
>>
>> Thanks,
>> -Dai
>>
>>> --b.
>>>
>>>> |----------------------------------------------------------|
>>>> |  NFSD  |  NFS_FS  |  NFS_V4  |        RESULTS            |
>>>> |----------------------------------------------------------|
>>>> |   y    |    y     |    m     | inter server copy OK      |
>>>> |----------------------------------------------------------|
>>>> |   y    |    m     |    m     | inter server copy OK      |
>>>> |----------------------------------------------------------|
>>>> |   y    |    m     |   y (m)  | inter server copy OK      |
>>>> |----------------------------------------------------------|
>>>> |   y    |    y     |    y     | inter server copy OK      |
>>>> |----------------------------------------------------------|
>>>> |   y    |    n     |    n     | NFS4ERR_STALE error       |
>>>> |----------------------------------------------------------|
>>>>
>>>> NOTE:
>>>> When NFS_V4=y and NFS_FS=m, the build process automatically builds
>>>> with NFS_V4=m and ignores the setting NFS_V4=y in the config file.
>>>>
>>>> This probably due to NFS_V4 in fs/nfs/Kconfig is configured to
>>>> depend on NFS_FS.

Re: [PATCH v2 0/1] NFSv4.2: Fix NFS4ERR_STALE with inter server copy

[Dai Ngo]

On 10/8/20 2:15 PM, Dai Ngo wrote:
>
> On 10/8/20 12:47 PM, J. Bruce Fields wrote:
>> On Thu, Oct 08, 2020 at 11:29:45AM -0700, Dai Ngo wrote:
>>> On 10/8/20 10:58 AM, J. Bruce Fields wrote:
>>>> On Wed, Oct 07, 2020 at 09:25:12PM -0400, Dai Ngo wrote:
>>>>> This cover email is intended for including my test results.
>>>>>
>>>>> This patch adds the ops table in nfs_common for knfsd to access
>>>>> NFS client modules without calling these functions directly.
>>>>>
>>>>> The client module registers their functions and deregisters them
>>>>> when the module is loaded and unloaded respectively.
>>>>>
>>>>>   fs/nfs/nfs4file.c       |  44 ++++++++++++--
>>>>>   fs/nfs/nfs4super.c      |   6 ++
>>>>>   fs/nfs/super.c          |  20 +++++++
>>>>>   fs/nfs_common/Makefile  |   1 +
>>>>>   fs/nfs_common/nfs_ssc.c | 136 
>>>>> +++++++++++++++++++++++++++++++++++++++++++
>>>>>   fs/nfsd/Kconfig         |   2 +-
>>>>>   fs/nfsd/nfs4proc.c      |   3 +-
>>>>>   include/linux/nfs_ssc.h |  77 ++++++++++++++++++++++++
>>>>>   8 files changed, 281 insertions(+), 8 deletions(-)
>>>>>
>>>>> Test Results:
>>>>>
>>>>> Upstream version used for testing:  5.9-rc5
>>>>>
>>>>> |----------------------------------------------------------|
>>>>> |  NFSD  |  NFS_FS  |  NFS_V4  |       RESULTS |
>>>>> |----------------------------------------------------------|
>>>>> |   m    |    y     |    m     | inter server copy OK |
>>>>> |----------------------------------------------------------|
>>>>> |   m    |    m     |    m     | inter server copy OK |
>>>>> |----------------------------------------------------------|
>>>>> |   m    |    m     |   y (m)  | inter server copy OK |
>>>>> |----------------------------------------------------------|
>>>>> |   m    |    y     |    y     | inter server copy OK |
>>>>> |----------------------------------------------------------|
>>>>> |   m    |    n     |    n     | NFS4ERR_STALE error |
>>>>> |----------------------------------------------------------|
>>>> Why are there two?
>>> Can you clarify this question?
>> Sorry, I meant: why are there two copies of this table?
>>
>> OK, I see now, the first is for the NFSD=m case, the second is for
>> NFSD=y.
>>
>>>>   And how are you getting that NFS4ERR_STALE case?
>>>> NFSD_V4_2_INTER_SSC depends on NFS_FS, so it shouldn't be possible to
>>>> build server-to-server-copy support without building the client.  
>>>> And if
>>>> you don't build NFSD_V4_2_INTER_SSC at all, then I think it should be
>>>> returning NOTSUPP instead of STALE.
>>> In the case where CONFIG_NFSD_V4_2_INTER_SSC is not set, when the inter
>>> server copy fails in nfsd4_putfh, before nfsd4_copy, with nfserr_stale
>>> returned from fs_verify. There is no code to handle this error and 
>>> it is
>>> returned to the client. This is the existing behavior, the patch 
>>> does not
>>> attempt to make any change in this area since there are more to 
>>> fixes in
>>> this area and it can be done in separate patches.
>> OK.
>>
>>> For example, when NFS4ERR_STALE happens, the file was left created with
>>> size 0.
>> Well, that's bad.  Is this the case where CONFIG_NFSD_V4_2_INTER_SSC is
>> set, or is this some other failure?
>
> fs_verify always return nfserr_stale even when CONFIG_NFSD_V4_2_INTER_SSC
> is set, that is the way the existing code works.

I think this happens for the source file handle only.

-Dai

> With CONFIG_NFSD_V4_2_INTER_SSC
> set, the code in nfsd4_putfh handles the error and ignores it if 
> no_verify
> is set. With CONFIG_NFSD_V4_2_INTER_SSC not set, the error is returned to
> the client. So I think what should happens is if 
> CONFIG_NFSD_V4_2_INTER_SSC
> is not set, the operation should fail with appropriate error code and 
> removes
> the file.
>
>>
>> (Also, are you checking this on the server side?  There's a known
>> problem where the copy completes succesfully but the client assumes its
>> cache is still valid and (incorrectly) reports the destination file is
>> still empty.)
>
> I have not run into this case when CONFIG_NFSD_V4_2_INTER_SSC is set.
>
> Thanks,
> -Dai
>
>>
>> --b.
>>
>>> and the 'refcount_t: underflow; use-after-free' problem:
>>>
>>> Oct  4 20:21:31 nfsvmf24 kernel: refcount_t: underflow; use-after-free.
>>> Oct  4 20:21:31 nfsvmf24 kernel: WARNING: CPU: 0 PID: 7 at 
>>> lib/refcount.c:28 refcount_warn_saturate+0xae/0xf0
>>> Oct  4 20:21:31 nfsvmf24 kernel: Modules linked in: rpcsec_gss_krb5 
>>> nfsv4 dns_resolver xt_REDIRECT xt_nat ip6table_nat ip6_tables 
>>> iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 rfkill 
>>> btrfs blake2b_generic xor zstd_compress raid6_pq sb_edac 
>>> intel_powerclamp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel 
>>> aesni_intel crypto_simd cryptd glue_helper pcspkr sg video i2c_piix4 
>>> nfsd auth_rpcgss ip_tables xfs libcrc32c sd_mod t10_pi ahci libahci 
>>> libata e1000 crc32c_intel serio_raw dm_mirror dm_region_hash dm_log 
>>> dm_mod
>>> Oct  4 20:21:31 nfsvmf24 kernel: CPU: 0 PID: 7 Comm: kworker/u2:0 
>>> Not tainted 5.9.0-rc5+ #4
>>> Oct  4 20:21:31 nfsvmf24 kernel: Hardware name: innotek GmbH 
>>> VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
>>> Oct  4 20:21:31 nfsvmf24 kernel: Workqueue: rpciod rpc_async_schedule
>>> Oct  4 20:21:31 nfsvmf24 kernel: RIP: 
>>> 0010:refcount_warn_saturate+0xae/0xf0
>>> Oct  4 20:21:31 nfsvmf24 kernel: Code: d4 dc 16 01 01 e8 07 7b bf ff 
>>> 0f 0b 5d c3 80 3d c1 dc 16 01 00 75 91 48 c7 c7 70 0b 17 8d c6 05 b1 
>>> dc 16 01 01 e8 e7 7a bf ff <0f> 0b 5d c3 80 3d 9f dc 16 01 00 0f 85 
>>> 6d ff ff ff 48 c7 c7 c8 0b
>>> Oct  4 20:21:31 nfsvmf24 kernel: RSP: 0018:ffffa589c0043d68 EFLAGS: 
>>> 00010286
>>> Oct  4 20:21:31 nfsvmf24 kernel: RAX: 0000000000000000 RBX: 
>>> 0000000000002a81 RCX: 0000000000000027
>>> Oct  4 20:21:31 nfsvmf24 kernel: RDX: 0000000000000027 RSI: 
>>> 0000000000000086 RDI: ffff996c17c18c48
>>> Oct  4 20:21:31 nfsvmf24 kernel: RBP: ffffa589c0043d68 R08: 
>>> ffff996c17c18c40 R09: 0000000000000004
>>> Oct  4 20:21:31 nfsvmf24 kernel: R10: 0000000000000000 R11: 
>>> 0000000000000001 R12: ffff996c14c7e470
>>> Oct  4 20:21:31 nfsvmf24 kernel: R13: ffff996c14ca4510 R14: 
>>> ffff996c0eef6130 R15: 0000000000000000
>>> Oct  4 20:21:31 nfsvmf24 kernel: FS:  0000000000000000(0000) 
>>> GS:ffff996c17c00000(0000) knlGS:0000000000000000
>>> Oct  4 20:21:31 nfsvmf24 kernel: CS:  0010 DS: 0000 ES: 0000 CR0: 
>>> 0000000080050033
>>> Oct  4 20:21:31 nfsvmf24 kernel: CR2: 00007fbb6fada000 CR3: 
>>> 000000020d3fe000 CR4: 00000000000406f0
>>> Oct  4 20:21:31 nfsvmf24 kernel: Call Trace:
>>> Oct  4 20:21:31 nfsvmf24 kernel: nfs4_put_copy+0x3c/0x40 [nfsd]
>>> Oct  4 20:21:31 nfsvmf24 kernel: nfsd4_cb_offload_release+0x15/0x20 
>>> [nfsd]
>>> Oct  4 20:21:31 nfsvmf24 kernel: nfsd41_destroy_cb+0x3a/0x50 [nfsd]
>>> Oct  4 20:21:31 nfsvmf24 kernel: nfsd4_cb_release+0x2b/0x30 [nfsd]
>>> Oct  4 20:21:31 nfsvmf24 kernel: rpc_free_task+0x40/0x70
>>> Oct  4 20:21:31 nfsvmf24 kernel: __rpc_execute+0x3c9/0x3e0
>>> Oct  4 20:21:31 nfsvmf24 kernel: ? __switch_to_asm+0x36/0x70
>>> Oct  4 20:21:31 nfsvmf24 kernel: rpc_async_schedule+0x30/0x50
>>> Oct  4 20:21:31 nfsvmf24 kernel: process_one_work+0x1b4/0x380
>>> Oct  4 20:21:31 nfsvmf24 kernel: worker_thread+0x50/0x3d0
>>> Oct  4 20:21:31 nfsvmf24 kernel: kthread+0x114/0x150
>>>
>>> Thanks,
>>> -Dai
>>>
>>>> --b.
>>>>
>>>>> |----------------------------------------------------------|
>>>>> |  NFSD  |  NFS_FS  |  NFS_V4  |        RESULTS |
>>>>> |----------------------------------------------------------|
>>>>> |   y    |    y     |    m     | inter server copy OK |
>>>>> |----------------------------------------------------------|
>>>>> |   y    |    m     |    m     | inter server copy OK |
>>>>> |----------------------------------------------------------|
>>>>> |   y    |    m     |   y (m)  | inter server copy OK |
>>>>> |----------------------------------------------------------|
>>>>> |   y    |    y     |    y     | inter server copy OK |
>>>>> |----------------------------------------------------------|
>>>>> |   y    |    n     |    n     | NFS4ERR_STALE error |
>>>>> |----------------------------------------------------------|
>>>>>
>>>>> NOTE:
>>>>> When NFS_V4=y and NFS_FS=m, the build process automatically builds
>>>>> with NFS_V4=m and ignores the setting NFS_V4=y in the config file.
>>>>>
>>>>> This probably due to NFS_V4 in fs/nfs/Kconfig is configured to
>>>>> depend on NFS_FS.