Re: linux-next: nfs tree build failures

Re: linux-next: nfs tree build failures

[Benny Halevy]

On Apr. 06, 2009, 9:29 +0300, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> Hi Trond,
> 
> Today's linux-next build (powerpc ppc44x_config) failed like this:
> 
> fs/nfs/client.c: In function 'nfs_match_client':
> fs/nfs/client.c:444: error: 'struct nfs_client' has no member named 'cl_minorversion'
> 
> Caused by commit 10d5a514705f0687cfbb6a080c3562b857e340fe ("nfs41: Use
> mount minorversion option").  The existence of cl_minorversion is
> controlled by CONFIG_NFS_V4 but the reference is not.


Stephan,

First, thanks! I apologize for introducing this.
This was caused by a late fix to Trond's review comment which
apparently was under-tested, as you mentioned.

> 
> I added the small patch below for today.

Based on Trond's review comment:
http://linux-nfs.org/pipermail/pnfs/2009-March/006938.html

We'd actually want to keep cl_minorversion's usage in the clear
and rather move its definition out of the #ifdef.

Please see PATCH 1/3 in reply to this message.

> 
> The same build gets this warning as well:
> 
> fs/nfs/client.c:554: warning: 'nfs4_session_set_rwsize' defined but not used

This one needs to go under CONFIG_NFS_V4.
See PATCH 2/2

> 
> and then fails like this:
> 
> fs/built-in.o: In function `nfs_readpage_result_full':
> read.c:(.text+0x70d24): undefined reference to `nfs4_restart_rpc'
> fs/built-in.o: In function `nfs_readpage_result_partial':
> read.c:(.text+0x70e3c): undefined reference to `nfs4_restart_rpc'
> fs/built-in.o: In function `nfs_async_unlink_done':
> unlink.c:(.text+0x71af0): undefined reference to `nfs4_restart_rpc'
> fs/built-in.o: In function `nfs_writeback_done':
> (.text+0x72c00): undefined reference to `nfs4_restart_rpc'
> 
> Caused by commit c105a9f16f11aaa9fcb9f75f8342b89c5d9665cf ("nfs41: use
> rpc prepare call state for session reset") which I have reverted for
> today.

See PATCH 3/3 that inlines nfs4_restart_rpc's definition.

Benny

> 
> Trond, is all this NFS v4.1 stuff that turned up yesterday really 2.6.30
> material?  If not, it should be removed from linux-next until after
> 2.6.30-rc1 is released.  (and clearly better tested ...)
> 
> Bruce, I guess the same question applies to the nfsd tree as well.

[PATCH 1/3] SQUASHME: nfs41: always define nfs_client.cl_minorversion

[Benny Halevy]

We want to avoid using #ifdef when using it.

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Benny Halevy <bhalevy@panasas.com>
---
 include/linux/nfs_fs_sb.h |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 2ce5ada..0043bf5 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -38,7 +38,7 @@ struct nfs_client {
 	struct rpc_clnt *	cl_rpcclient;
 	const struct nfs_rpc_ops *rpc_ops;	/* NFS protocol vector */
 	int			cl_proto;	/* Network transport protocol */
-
+	u32			cl_minorversion;/* NFSv4 minorversion */
 	struct rpc_cred		*cl_machine_cred;
 
 #ifdef CONFIG_NFS_V4
@@ -70,7 +70,6 @@ struct nfs_client {
 	 */
 	char			cl_ipaddr[48];
 	unsigned char		cl_id_uniquifier;
-	u32			cl_minorversion;
 	int		     (* cl_call_sync)(struct nfs_server *server,
 					      struct rpc_message *msg,
 					      struct nfs4_sequence_args *args,
-- 
1.6.2.1

[PATCH 2/3] SQUASHME: nfs41: move nfs4_session_set_rwsize into CONFIG_NFS_V4

[Benny Halevy]

fix compiler warning:
fs/nfs/client.c:554: warning: 'nfs4_session_set_rwsize' defined but not used

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Benny Halevy <bhalevy@panasas.com>
---
 fs/nfs/client.c |   30 +++++++++++++++---------------
 1 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index eb341d3..9999f13 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -551,21 +551,6 @@ int nfs4_check_client_ready(struct nfs_client *clp)
 }
 
 /*
- * Session has been established, and the client marked ready.
- * Set the mount rsize and wsize with negotiated fore channel
- * attributes which will be bound checked in nfs_server_set_fsinfo.
- */
-static void nfs4_session_set_rwsize(struct nfs_server *server)
-{
-#ifdef CONFIG_NFS_V4_1
-	if (!nfs4_has_session(server->nfs_client))
-		return;
-	server->rsize = server->nfs_client->cl_session->fc_attrs.max_resp_sz;
-	server->wsize = server->nfs_client->cl_session->fc_attrs.max_rqst_sz;
-#endif /* CONFIG_NFS_V4_1 */
-}
-
-/*
  * Initialise the timeout values for a connection
  */
 static void nfs_init_timeout_values(struct rpc_timeout *to, int proto,
@@ -1246,6 +1231,21 @@ static void nfs4_init_session(struct nfs_client *clp,
 }
 
 /*
+ * Session has been established, and the client marked ready.
+ * Set the mount rsize and wsize with negotiated fore channel
+ * attributes which will be bound checked in nfs_server_set_fsinfo.
+ */
+static void nfs4_session_set_rwsize(struct nfs_server *server)
+{
+#ifdef CONFIG_NFS_V4_1
+	if (!nfs4_has_session(server->nfs_client))
+		return;
+	server->rsize = server->nfs_client->cl_session->fc_attrs.max_resp_sz;
+	server->wsize = server->nfs_client->cl_session->fc_attrs.max_rqst_sz;
+#endif /* CONFIG_NFS_V4_1 */
+}
+
+/*
  * Create a version 4 volume record
  */
 static int nfs4_init_server(struct nfs_server *server,
-- 
1.6.2.1

[PATCH 3/3] SQUASHME: inline nfs4_restart_rpc

[Benny Halevy]

We must always define nfs4_restart_rpc.

fs/built-in.o: In function `nfs_readpage_result_full':
read.c:(.text+0x70d24): undefined reference to `nfs4_restart_rpc'
fs/built-in.o: In function `nfs_readpage_result_partial':
read.c:(.text+0x70e3c): undefined reference to `nfs4_restart_rpc'
fs/built-in.o: In function `nfs_async_unlink_done':
unlink.c:(.text+0x71af0): undefined reference to `nfs4_restart_rpc'
fs/built-in.o: In function `nfs_writeback_done':
(.text+0x72c00): undefined reference to `nfs4_restart_rpc'

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Benny Halevy <bhalevy@panasas.com>
---
 fs/nfs/internal.h |   39 +++++++++++++++++++++++++--------------
 fs/nfs/nfs4proc.c |   12 ------------
 2 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index a812866..43d9ca4 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -2,6 +2,7 @@
  * NFS internal definitions
  */
 
+#include "nfs4_fs.h"
 #include <linux/mount.h>
 #include <linux/security.h>
 
@@ -15,6 +16,18 @@ struct nfs_string;
  */
 #define NFS_MAX_READAHEAD	(RPC_DEF_SLOT_TABLE - 1)
 
+/*
+ * Determine if sessions are in use.
+ */
+static inline int nfs4_has_session(const struct nfs_client *clp)
+{
+#ifdef CONFIG_NFS_V4_1
+	if (clp->cl_session)
+		return 1;
+#endif /* CONFIG_NFS_V4_1 */
+	return 0;
+}
+
 struct nfs_clone_mount {
 	const struct super_block *sb;
 	const struct dentry *dentry;
@@ -146,8 +159,18 @@ extern struct rpc_procinfo nfs3_procedures[];
 extern __be32 *nfs3_decode_dirent(__be32 *, struct nfs_entry *, int);
 
 /* nfs4proc.c */
-extern void nfs4_restart_rpc(struct rpc_task *task,
-			     const struct nfs_client *clp);
+static inline void nfs4_restart_rpc(struct rpc_task *task,
+				    const struct nfs_client *clp)
+{
+#ifdef CONFIG_NFS_V4_1
+	if (nfs4_has_session(clp) &&
+	    test_bit(NFS4CLNT_SESSION_SETUP, &clp->cl_state)) {
+		rpc_restart_call_prepare(task);
+		return;
+	}
+#endif /* CONFIG_NFS_V4_1 */
+	rpc_restart_call(task);
+}
 
 /* nfs4xdr.c */
 #ifdef CONFIG_NFS_V4
@@ -222,18 +245,6 @@ extern int _nfs4_call_sync_session(struct nfs_server *server,
 				   struct nfs4_sequence_res *res,
 				   int cache_reply);
 
-/*
- * Determine if sessions are in use.
- */
-static inline int nfs4_has_session(const struct nfs_client *clp)
-{
-#ifdef CONFIG_NFS_V4_1
-	if (clp->cl_session)
-		return 1;
-#endif /* CONFIG_NFS_V4_1 */
-	return 0;
-}
-
 #ifdef CONFIG_NFS_V4_1
 extern void nfs41_sequence_free_slot(const struct nfs_client *,
 				     struct nfs4_sequence_res *res);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index d6ed319..3e4194f 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -646,18 +646,6 @@ static void nfs4_sequence_done_free_slot(const struct nfs_server *server,
 	nfs4_sequence_free_slot(server->nfs_client, res);
 }
 
-void nfs4_restart_rpc(struct rpc_task *task, const struct nfs_client *clp)
-{
-#ifdef CONFIG_NFS_V4_1
-	if (nfs4_has_session(clp) &&
-	    test_bit(NFS4CLNT_SESSION_SETUP, &clp->cl_state)) {
-		rpc_restart_call_prepare(task);
-		return;
-	}
-#endif /* CONFIG_NFS_V4_1 */
-	rpc_restart_call(task);
-}
-
 static void update_changeattr(struct inode *dir, struct nfs4_change_info *cinfo)
 {
 	struct nfs_inode *nfsi = NFS_I(dir);
-- 
1.6.2.1

Re: linux-next: nfs tree build failures

[Trond Myklebust]

On Mon, 2009-04-06 at 10:43 +0300, Benny Halevy wrote:
> On Apr. 06, 2009, 9:29 +0300, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > Hi Trond,
> > 
> > Today's linux-next build (powerpc ppc44x_config) failed like this:
> > 
> > fs/nfs/client.c: In function 'nfs_match_client':
> > fs/nfs/client.c:444: error: 'struct nfs_client' has no member named 'cl_minorversion'
> > 
> > Caused by commit 10d5a514705f0687cfbb6a080c3562b857e340fe ("nfs41: Use
> > mount minorversion option").  The existence of cl_minorversion is
> > controlled by CONFIG_NFS_V4 but the reference is not.
> 
> 
> Stephan,
> 
> First, thanks! I apologize for introducing this.
> This was caused by a late fix to Trond's review comment which
> apparently was under-tested, as you mentioned.
> 
> > 
> > I added the small patch below for today.
> 
> Based on Trond's review comment:
> http://linux-nfs.org/pipermail/pnfs/2009-March/006938.html
> 
> We'd actually want to keep cl_minorversion's usage in the clear
> and rather move its definition out of the #ifdef.
> 
> Please see PATCH 1/3 in reply to this message.
> 
> > 
> > The same build gets this warning as well:
> > 
> > fs/nfs/client.c:554: warning: 'nfs4_session_set_rwsize' defined but not used
> 
> This one needs to go under CONFIG_NFS_V4.
> See PATCH 2/2
> 
> > 
> > and then fails like this:
> > 
> > fs/built-in.o: In function `nfs_readpage_result_full':
> > read.c:(.text+0x70d24): undefined reference to `nfs4_restart_rpc'
> > fs/built-in.o: In function `nfs_readpage_result_partial':
> > read.c:(.text+0x70e3c): undefined reference to `nfs4_restart_rpc'
> > fs/built-in.o: In function `nfs_async_unlink_done':
> > unlink.c:(.text+0x71af0): undefined reference to `nfs4_restart_rpc'
> > fs/built-in.o: In function `nfs_writeback_done':
> > (.text+0x72c00): undefined reference to `nfs4_restart_rpc'
> > 
> > Caused by commit c105a9f16f11aaa9fcb9f75f8342b89c5d9665cf ("nfs41: use
> > rpc prepare call state for session reset") which I have reverted for
> > today.
> 
> See PATCH 3/3 that inlines nfs4_restart_rpc's definition.
> 
> Benny

All squashed into the relevant original patches.

> > 
> > Trond, is all this NFS v4.1 stuff that turned up yesterday really 2.6.30
> > material?  If not, it should be removed from linux-next until after
> > 2.6.30-rc1 is released.  (and clearly better tested ...)
> > 
> > Bruce, I guess the same question applies to the nfsd tree as well.

This is code that isn't supposed to significantly affect the existing
NFS tree unless CONFIG_NFS_V4_1 is set. Since the corresponding NFSd
code has been merged into 2.6.30, it would be nice to have the client
code too so that people can start testing, and so that development can
continue in the mainline tree instead of in the smoky backrooms of the
linux-nfs.org git trees.

Cheers
  Trond