[PATCHSET 0/5] nfsd: Cleanup nfsd/pnfsd headers and code placment

[PATCHSET 0/5] nfsd: Cleanup nfsd/pnfsd headers and code placment

[Boaz Harrosh]

In motivation to better organize pnfs code and have minimal impact
on the rest of the nfsd code-base, I've done some cleanups and code
movements. The begging of which was done by Andy in the last round.

The first needed thing was to re-introduce some sense into nfsd
headers. I've taken all include/linux/nfsd/*.h files, one by one,
and included them as first header in a .c file. Then fixed any
missing includes from that header.

Here is current state after this patchset:

// Still BAD
#include <linux/nfsd/const.h> 
	/* NEEDS #include <linux/types.h> in linux/sunrpc/msg_prot.h */
#include <linux/nfsd/export.h> 
	/* NEEDS #include <linux/nfsd/nfsfh.h> */
#include <linux/nfsd/syscall.h> /* Broken because of export.h above */

[Should I fix those too?]

// GOOD or fixed
#include <linux/nfsd/cache.h>
#include <linux/nfsd/debug.h>
#include <linux/nfsd/nfs4layoutxdr.h>
#include <linux/nfsd/nfs4pnfsdlm.h>
#include <linux/nfsd/nfsd4_pnfs.h>
#include <linux/nfsd/nfsd.h>
#include <linux/nfsd/nfsfh.h>
#include <linux/nfsd/pnfsd.h>
#include <linux/nfsd/state.h>
#include <linux/nfsd/stats.h>
#include <linux/nfsd/xdr.h>
#include <linux/nfsd/xdr3.h>
#include <linux/nfsd/xdr4.h>

I then proceeded to re-arrange some pnfs code/definitions into
pnfsd specific files.

And finally all files thouched in this round where include-cleaned

Here are the list of patches:

[PATCH 1/5] sunrpc: Clean never used include files

	This one is for Trond, include-cleaning

[PATCH 2/5] nfsd: Fix independence of a few nfsd related headers
[PATCH 3/5] nfsd: Fix independence of linux/nfsd/ headers

	These two fix nfsd headers independence. As explained above.
	(Should I squash these together)

[PATCH 4/5] SQUASHME pnfsd: Move pnfsd code out of nfs4state.c/h

	Now it is much easier to do this one.

[PATCH 5/5] nfsd: Remove lots of un-needed includes

	And some more include-cleaning

TODO: Some more of above magic done to more nfsd files

Thanks
Boaz

[PATCH 1/5] sunrpc: Clean never used include files

[Boaz Harrosh]

Remove include of two headers never used by this file.
I did not see any users dependency but if exist they
should be fixed since these headers are totally irrelevant
to here.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 include/linux/sunrpc/debug.h |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h
index 10709cb..c2786f2 100644
--- a/include/linux/sunrpc/debug.h
+++ b/include/linux/sunrpc/debug.h
@@ -28,9 +28,6 @@
 
 #ifdef __KERNEL__
 
-#include <linux/timer.h>
-#include <linux/workqueue.h>
-
 /*
  * Enable RPC debugging/profiling.
  */
-- 
1.6.2.1

[PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[Boaz Harrosh]

An header should be compilation independent, .i.e pull in
any header who's declarations are directly used by this header.
And not let users re-include all it's dependencies all over
again.

[At the end of the day what's the use of a header if it does
 not have more then one user?]

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 include/linux/nfs_xdr.h   |    1 +
 include/linux/nfsacl.h    |    1 +
 include/linux/posix_acl.h |    1 +
 3 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 2848a26..c316ca8 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -2,6 +2,7 @@
 #define _LINUX_NFS_XDR_H
 
 #include <linux/nfsacl.h>
+#include <linux/nfs3.h>
 
 /*
  * To change the maximum rsize and wsize supported by the NFS client, adjust
diff --git a/include/linux/nfsacl.h b/include/linux/nfsacl.h
index 43011b6..f321b57 100644
--- a/include/linux/nfsacl.h
+++ b/include/linux/nfsacl.h
@@ -29,6 +29,7 @@
 #ifdef __KERNEL__
 
 #include <linux/posix_acl.h>
+#include <linux/sunrpc/xdr.h>
 
 /* Maximum number of ACL entries over NFS */
 #define NFS_ACL_MAX_ENTRIES	1024
diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
index 065a365..0dcf674 100644
--- a/include/linux/posix_acl.h
+++ b/include/linux/posix_acl.h
@@ -9,6 +9,7 @@
 #define __LINUX_POSIX_ACL_H
 
 #include <linux/slab.h>
+#include <linux/fs.h>
 
 #define ACL_UNDEFINED_ID	(-1)
 
-- 
1.6.2.1

[PATCH 3/5] nfsd: Fix independence of linux/nfsd/ headers

[Boaz Harrosh]

An header should be compilation independent, .i.e pull in
any header who's declarations are directly used by this header.
And not let users re-include all it's dependencies all over
again.

[At the end of the day what's the use of a header if it does
 not have more then one user?]

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 include/linux/nfsd/cache.h         |    1 +
 include/linux/nfsd/nfs4layoutxdr.h |    2 ++
 include/linux/nfsd/nfsfh.h         |    1 +
 include/linux/nfsd/xdr.h           |    1 +
 include/linux/nfsd/xdr4.h          |    1 +
 5 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/include/linux/nfsd/cache.h b/include/linux/nfsd/cache.h
index 3a3f589..3890284 100644
--- a/include/linux/nfsd/cache.h
+++ b/include/linux/nfsd/cache.h
@@ -12,6 +12,7 @@
 
 #include <linux/in.h>
 #include <linux/uio.h>
+#include <linux/sunrpc/svc.h>
 
 /*
  * Representation of a reply cache entry.
diff --git a/include/linux/nfsd/nfs4layoutxdr.h b/include/linux/nfsd/nfs4layoutxdr.h
index ad7f4cb..d4d05d7 100644
--- a/include/linux/nfsd/nfs4layoutxdr.h
+++ b/include/linux/nfsd/nfs4layoutxdr.h
@@ -38,6 +38,8 @@
 
 #if defined(CONFIG_PNFSD)
 
+#include <linux/nfsd/nfsd4_pnfs.h>
+
 /* the nfsd4_pnfs_devlist dev_addr for the file layout type */
 struct pnfs_filelayout_devaddr {
 	struct xdr_netobj	r_netid;
diff --git a/include/linux/nfsd/nfsfh.h b/include/linux/nfsd/nfsfh.h
index b91760c..b4e024c 100644
--- a/include/linux/nfsd/nfsfh.h
+++ b/include/linux/nfsd/nfsfh.h
@@ -18,6 +18,7 @@
 #ifdef __KERNEL__
 # include <linux/string.h>
 # include <linux/fs.h>
+# include <linux/sunrpc/svc.h>
 #endif
 #include <linux/nfsd/const.h>
 
diff --git a/include/linux/nfsd/xdr.h b/include/linux/nfsd/xdr.h
index a0132ef..84dc0f0 100644
--- a/include/linux/nfsd/xdr.h
+++ b/include/linux/nfsd/xdr.h
@@ -10,6 +10,7 @@
 #include <linux/fs.h>
 #include <linux/vfs.h>
 #include <linux/nfs.h>
+#include <linux/nfsd/nfsd.h>
 
 struct nfsd_fhandle {
 	struct svc_fh		fh;
diff --git a/include/linux/nfsd/xdr4.h b/include/linux/nfsd/xdr4.h
index bdd4beb..9243d1b 100644
--- a/include/linux/nfsd/xdr4.h
+++ b/include/linux/nfsd/xdr4.h
@@ -40,6 +40,7 @@
 #define _LINUX_NFSD_XDR4_H
 
 #include <linux/nfs4.h>
+#include <linux/nfsd/nfsd.h>
 #include <linux/nfsd/nfsd4_pnfs.h>
 
 #define NFSD4_MAX_TAGLEN	128
-- 
1.6.2.1

[PATCH 4/5] SQUASHME pnfsd: Move pnfsd code out of nfs4state.c/h

[Boaz Harrosh]

* Introduce an fs/nfsd/nfsd_priv.h header that collects private
  nfsd definitions/declarations. Specially all none exported
  extern will move to that file.

* Include "nfsd_priv.h" in nfsd source files where needed.

* pnfs specific declarations are moved out of linux/nfsd/state.h and
  into linux/nfsd/nfsd4_pnfs.h, though removing inclusion of
  nfsd4_pnfs.h from state.h and all the #if defined(CONFIG_PNFSD)

* Finally moving remaining #if defined(CONFIG_PNFSD) code from
  nfs4state.c into nfs4pnfsd.c

TODO: I know that lots of other code from include/linux/... nfsd
headers could/should be moved into "nfsd_priv.h"

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 fs/nfsd/export.c                |    1 +
 fs/nfsd/nfs4callback.c          |    3 +
 fs/nfsd/nfs4pnfsd.c             |   83 ++++++++++++++++++++++++++++++
 fs/nfsd/nfs4proc.c              |    1 +
 fs/nfsd/nfs4state.c             |  101 +------------------------------------
 fs/nfsd/nfsd_priv.h             |  105 +++++++++++++++++++++++++++++++++++++++
 include/linux/nfsd/nfsd4_pnfs.h |   61 ++++++++++++++++++----
 include/linux/nfsd/pnfsd.h      |    8 ---
 include/linux/nfsd/state.h      |   85 +-------------------------------
 9 files changed, 246 insertions(+), 202 deletions(-)
 create mode 100644 fs/nfsd/nfsd_priv.h

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 0eea1a6..f51c413 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -32,6 +32,7 @@
 #include <linux/nfsd/nfsd.h>
 #include <linux/nfsd/nfsfh.h>
 #include <linux/nfsd/pnfsd.h>
+#include "nfsd_priv.h"
 #if defined(CONFIG_SPNFS)
 #include <linux/nfsd4_spnfs.h>
 #if defined(CONFIG_SPNFS_BLOCK)
diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index b53194c..3347749 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -311,6 +311,9 @@ encode_cb_sequence(struct xdr_stream *xdr, struct nfsd4_cb_sequence *args,
 }
 
 #if defined(CONFIG_PNFSD)
+
+#include "nfsd_priv.h"
+
 static void
 encode_cb_layout(struct xdr_stream *xdr, struct nfs4_layoutrecall *clr,
 		 struct nfs4_cb_compound_hdr *hdr)
diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index d52a2e5..fdbc447 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -33,6 +33,7 @@
 #include <linux/nfsd/xdr4.h>
 #include <linux/exportfs.h>
 #include <linux/nfsd/pnfsd.h>
+#include "nfsd_priv.h"
 
 #define NFSDDBG_FACILITY                NFSDDBG_PROC
 
@@ -1440,4 +1441,86 @@ int nfsd_device_notify_cb(struct super_block *sb,
 		__func__, status, notify_num);
 	return status;
 }
+
+/* Create a layoutrecall structure for each client based on the
+ * original structure. */
+int
+create_layout_recall_list(struct list_head *todolist, unsigned *todo_len,
+			  struct nfsd4_pnfs_cb_layout *cbl,
+			  struct nfs4_file *lrfile)
+{
+	struct nfs4_client *clp;
+	unsigned int i, len = 0;
+	int status = 0;
+
+	dprintk("%s: -->\n", __func__);
+
+	/* If client given by fs, just do single client */
+	if (cbl->cbl_seg.clientid) {
+		clp = find_confirmed_client(
+				(clientid_t *)&cbl->cbl_seg.clientid);
+		if (!clp) {
+			status = -ENOENT;
+			dprintk("%s: clientid %llx not found\n", __func__,
+				(unsigned long long)cbl->cbl_seg.clientid);
+			goto out;
+		}
+
+		status = lo_recall_per_client(clp, cbl, lrfile, todolist);
+		if (!status)
+			len++;
+		goto out;
+	}
+
+	/* Check all clients for layout matches */
+	for (i = 0; i < CLIENT_HASH_SIZE; i++)
+		list_for_each_entry(clp, &conf_str_hashtbl[i], cl_strhash) {
+			status = lo_recall_per_client(clp, cbl, lrfile,
+						      todolist);
+			if (!status)
+				len++;
+			else if (status != -ENOENT)
+				goto out;
+		}
+out:
+	*todo_len = len;
+	/* -ENOENT is a good thing don't return it if some recalls are needed */
+	if ((status == -ENOENT) && len)
+		status = 0;
+	dprintk("%s: <-- list len %u status %d\n", __func__, len, status);
+	return status;
+}
+
+/* Create a list of clients to send device notifications. */
+int
+create_device_notify_list(struct list_head *todolist,
+			  struct nfsd4_pnfs_cb_dev_list *ndl)
+{
+	int status = 0, i;
+	struct nfs4_client *clp = NULL;
+	struct nfs4_notify_device *cbnd;
+
+	nfs4_lock_state();
+
+	/* Create notify client list */
+	for (i = 0; i < CLIENT_HASH_SIZE; i++)
+		list_for_each_entry(clp, &conf_str_hashtbl[i], cl_strhash) {
+			if (atomic_read(&clp->cl_deviceref) <= 0)
+				continue;
+			cbnd = kmalloc(sizeof(*cbnd), GFP_KERNEL);
+			if (!cbnd) {
+				status = -ENOMEM;
+				goto out;
+			}
+			cbnd->nd_list = ndl;
+			cbnd->nd_client = clp;
+			list_add(&cbnd->nd_perclnt, todolist);
+			atomic_inc(&clp->cl_count);
+		}
+
+out:
+	nfs4_unlock_state();
+	return status;
+}
+
 #endif /* CONFIG_PNFSD */
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 83acf8d..dd1da22 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -52,6 +52,7 @@
 #include <linux/nfsd/pnfsd.h>
 #include <linux/exportfs.h>
 #include <linux/nfsd/nfs4layoutxdr.h>
+#include "nfsd_priv.h"
 #if defined(CONFIG_SPNFS)
 #include <linux/nfsd4_spnfs.h>
 #if defined(CONFIG_SPNFS_BLOCK)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 7495df8..018292e 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -59,6 +59,7 @@
 #if defined(CONFIG_PNFSD)
 #include <linux/nfsd/pnfsd.h>
 #endif /* CONFIG_PNFSD */
+#include "nfsd_priv.h"
 
 #define NFSDDBG_FACILITY                NFSDDBG_PROC
 
@@ -275,20 +276,6 @@ unhash_delegation(struct nfs4_delegation *dp)
 	nfs4_put_delegation(dp);
 }
 
-/* 
- * SETCLIENTID state 
- */
-
-/* Hash tables for nfs4_clientid state */
-#define CLIENT_HASH_BITS                 4
-#define CLIENT_HASH_SIZE                (1 << CLIENT_HASH_BITS)
-#define CLIENT_HASH_MASK                (CLIENT_HASH_SIZE - 1)
-
-#define clientid_hashval(id) \
-	((id) & CLIENT_HASH_MASK)
-#define clientstr_hashval(name) \
-	(opaque_hashval((name), 8) & CLIENT_HASH_MASK)
-
 /*
  * reclaim_str_hashtbl[] holds known client info from previous reset/reboot
  * used in reboot/reset lease grace period processing
@@ -308,7 +295,7 @@ unhash_delegation(struct nfs4_delegation *dp)
 static struct list_head reclaim_str_hashtbl[CLIENT_HASH_SIZE];
 static int reclaim_str_hashtbl_size = 0;
 static struct list_head	conf_id_hashtbl[CLIENT_HASH_SIZE];
-static struct list_head	conf_str_hashtbl[CLIENT_HASH_SIZE];
+struct list_head	conf_str_hashtbl[CLIENT_HASH_SIZE];
 static struct list_head	unconf_str_hashtbl[CLIENT_HASH_SIZE];
 static struct list_head	unconf_id_hashtbl[CLIENT_HASH_SIZE];
 static struct list_head client_lru;
@@ -4245,87 +4232,3 @@ nfs4_reset_lease(time_t leasetime)
 	user_lease_time = leasetime;
 }
 
-#if defined(CONFIG_PNFSD)
-
-/* Create a layoutrecall structure for each client based on the
- * original structure. */
-int
-create_layout_recall_list(struct list_head *todolist, unsigned *todo_len,
-			  struct nfsd4_pnfs_cb_layout *cbl,
-			  struct nfs4_file *lrfile)
-{
-	struct nfs4_client *clp;
-	unsigned int i, len = 0;
-	int status = 0;
-
-	dprintk("%s: -->\n", __func__);
-
-	/* If client given by fs, just do single client */
-	if (cbl->cbl_seg.clientid) {
-		clp = find_confirmed_client(
-				(clientid_t *)&cbl->cbl_seg.clientid);
-		if (!clp) {
-			status = -ENOENT;
-			dprintk("%s: clientid %llx not found\n", __func__,
-				(unsigned long long)cbl->cbl_seg.clientid);
-			goto out;
-		}
-
-		status = lo_recall_per_client(clp, cbl, lrfile, todolist);
-		if (!status)
-			len++;
-		goto out;
-	}
-
-	/* Check all clients for layout matches */
-	for (i = 0; i < CLIENT_HASH_SIZE; i++)
-		list_for_each_entry(clp, &conf_str_hashtbl[i], cl_strhash) {
-			status = lo_recall_per_client(clp, cbl, lrfile,
-						      todolist);
-			if (!status)
-				len++;
-			else if (status != -ENOENT)
-				goto out;
-		}
-out:
-	*todo_len = len;
-	/* -ENOENT is a good thing don't return it if some recalls are needed */
-	if ((status == -ENOENT) && len)
-		status = 0;
-	dprintk("%s: <-- list len %u status %d\n", __func__, len, status);
-	return status;
-}
-
-/* Create a list of clients to send device notifications. */
-int
-create_device_notify_list(struct list_head *todolist,
-			  struct nfsd4_pnfs_cb_dev_list *ndl)
-{
-	int status = 0, i;
-	struct nfs4_client *clp = NULL;
-	struct nfs4_notify_device *cbnd;
-
-	nfs4_lock_state();
-
-	/* Create notify client list */
-	for (i = 0; i < CLIENT_HASH_SIZE; i++)
-		list_for_each_entry(clp, &conf_str_hashtbl[i], cl_strhash) {
-			if (atomic_read(&clp->cl_deviceref) <= 0)
-				continue;
-			cbnd = kmalloc(sizeof(*cbnd), GFP_KERNEL);
-			if (!cbnd) {
-				status = -ENOMEM;
-				goto out;
-			}
-			cbnd->nd_list = ndl;
-			cbnd->nd_client = clp;
-			list_add(&cbnd->nd_perclnt, todolist);
-			atomic_inc(&clp->cl_count);
-		}
-
-out:
-	nfs4_unlock_state();
-	return status;
-}
-
-#endif  /* CONFIG_PNFSD */
diff --git a/fs/nfsd/nfsd_priv.h b/fs/nfsd/nfsd_priv.h
new file mode 100644
index 0000000..814fe8f
--- /dev/null
+++ b/fs/nfsd/nfsd_priv.h
@@ -0,0 +1,105 @@
+/*
+ *  nfsd_priv.h
+ *
+ *  nfsd declarations shared by a few .c files
+ *
+ *  Copyright (c) 2002 The Regents of the University of Michigan.
+ *  All rights reserved.
+ *
+ *  Redistribution and use in source and binary forms, with or without
+ *  modification, are permitted provided that the following conditions
+ *  are met:
+ *
+ *  1. Redistributions of source code must retain the above copyright
+ *     notice, this list of conditions and the following disclaimer.
+ *  2. Redistributions in binary form must reproduce the above copyright
+ *     notice, this list of conditions and the following disclaimer in the
+ *     documentation and/or other materials provided with the distribution.
+ *  3. Neither the name of the University nor the names of its
+ *     contributors may be used to endorse or promote products derived
+ *     from this software without specific prior written permission.
+ *
+ *  THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
+ *  WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+ *  MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ *  DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
+ *  FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ *  CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ *  SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ *  BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ *  LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ *  NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ *  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+#ifndef _NFSD_PRIV_H_
+#define _NFSD_PRIV_H_
+
+/* 
+ * SETCLIENTID state 
+ */
+
+/* Hash tables for nfs4_clientid state */
+#define CLIENT_HASH_BITS                 4
+#define CLIENT_HASH_SIZE                (1 << CLIENT_HASH_BITS)
+#define CLIENT_HASH_MASK                (CLIENT_HASH_SIZE - 1)
+
+#define clientid_hashval(id) \
+	((id) & CLIENT_HASH_MASK)
+#define clientstr_hashval(name) \
+	(opaque_hashval((name), 8) & CLIENT_HASH_MASK)
+
+extern struct list_head	conf_str_hashtbl[CLIENT_HASH_SIZE];
+
+#if defined(CONFIG_PNFSD)
+
+#include <linux/nfsd/nfsd4_pnfs.h>
+
+#if defined(CONFIG_PNFSD_LOCAL_EXPORT)
+extern struct sockaddr pnfsd_lexp_addr;
+extern size_t pnfs_lexp_addr_len;
+
+void pnfsd_lexp_init(struct inode *inode);
+#endif /* CONFIG_PNFSD_LOCAL_EXPORT */
+
+extern int nfsd4_init_pnfs_slabs(void);
+extern void nfsd4_free_pnfs_slabs(void);
+extern void release_pnfs_ds_dev_list(struct nfs4_stateid *stp);
+extern void pnfs_expire_client(struct nfs4_client *clp);
+
+extern void release_pnfs_ds_dev_list(struct nfs4_stateid *stp);
+extern void nfs4_pnfs_state_init(void);
+extern int put_layoutrecall(struct nfs4_layoutrecall *);
+extern void nomatching_layout(struct nfs4_layoutrecall *);
+extern void *layoutrecall_done(struct nfs4_layoutrecall *);
+extern int nfsd4_cb_layout(struct nfs4_layoutrecall *lp);
+extern void nfsd4_free_slab(struct kmem_cache **slab);
+extern struct nfs4_file * find_file(struct inode *ino);
+extern struct nfs4_file * alloc_init_file(struct inode *ino, struct svc_fh *current_fh);
+extern void put_nfs4_file(struct nfs4_file *fi);
+extern void get_nfs4_file(struct nfs4_file *fi);
+extern struct nfs4_client * find_confirmed_client(clientid_t *clid);
+extern void nfs4_bug_on_unlocked_state(void);
+extern void nfs4_ds_get_verifier(stateid_t *stateid,
+		struct super_block *sb, u32 *p);
+extern int nfs4_preprocess_pnfs_ds_stateid(struct svc_fh *, stateid_t *);
+extern void nfs4_pnfs_state_shutdown(void);
+extern int lo_recall_per_client(struct nfs4_client *clp,
+		struct nfsd4_pnfs_cb_layout *cbl, struct nfs4_file *lrfile,
+		struct list_head *todolist);
+extern int create_layout_recall_list(struct list_head *todolist,
+		unsigned *todo_len, struct nfsd4_pnfs_cb_layout *cbl,
+		struct nfs4_file *lrfile);
+extern int nfsd4_cb_notify_device(struct nfs4_notify_device *cbnd);
+extern void pnfs_clear_device_notify(struct nfs4_client *clp);
+extern int create_device_notify_list(struct list_head *todolist,
+		struct nfsd4_pnfs_cb_dev_list *ndl);
+#else /* CONFIG_PNFSD */
+static inline void nfsd4_free_pnfs_slabs(void) {}
+static inline int nfsd4_init_pnfs_slabs(void) { return 0; }
+static inline void release_pnfs_ds_dev_list(struct nfs4_stateid *stp) {}
+static inline void pnfs_expire_client(struct nfs4_client *clp) {}
+#endif /* CONFIG_PNFSD */
+
+#endif /* _NFSD_PRIV_H_ */
diff --git a/include/linux/nfsd/nfsd4_pnfs.h b/include/linux/nfsd/nfsd4_pnfs.h
index 410c459..70ca179 100644
--- a/include/linux/nfsd/nfsd4_pnfs.h
+++ b/include/linux/nfsd/nfsd4_pnfs.h
@@ -36,10 +36,9 @@
 #ifndef _LINUX_NFSD_NFSD4_PNFS_H
 #define _LINUX_NFSD_NFSD4_PNFS_H
 
+#include <linux/nfsd/state.h>
 #include <linux/nfs_xdr.h>
 #include <linux/exportfs.h>
-#include <linux/exp_xdr.h>
-#include <linux/nfsd/nfsfh.h>
 
 typedef struct {
 	uint64_t	pnfs_fsid;	/* fsid */
@@ -83,6 +82,55 @@ struct nfsd4_layout_seg {
 	u64	length;
 };
 
+/* outstanding layout stateid */
+struct nfs4_layout_state {
+	struct list_head	ls_perfile;
+	struct list_head	ls_layouts; /* list of nfs4_layouts */
+	struct kref		ls_ref;
+	struct nfs4_client	*ls_client;
+	struct nfs4_file	*ls_file;
+	stateid_t		ls_stateid;
+};
+
+/* outstanding layout */
+struct nfs4_layout {
+	struct list_head		lo_perfile;	/* hash by f_id */
+	struct list_head		lo_perclnt;	/* hash by clientid */
+	struct list_head		lo_perstate;
+	struct nfs4_file		*lo_file;	/* backpointer */
+	struct nfs4_client		*lo_client;
+	struct nfs4_layout_state	*lo_state;
+	struct nfsd4_layout_seg 	lo_seg;
+};
+
+struct nfsd4_pnfs_cb_layout {
+	u32			cbl_recall_type;	/* request */
+	struct nfsd4_layout_seg cbl_seg;		/* request */
+	u32			cbl_layoutchanged;	/* request */
+	stateid_t		cbl_sid;		/* request */
+	struct nfs4_fsid	cbl_fsid;
+	void			*cbl_cookie;		/* fs private */
+};
+
+/* layoutrecall request (from exported filesystem) */
+struct nfs4_layoutrecall {
+	struct kref			clr_ref;
+	struct nfsd4_pnfs_cb_layout	cb;	/* request */
+	struct list_head		clr_perclnt; /* on cl_layoutrecalls */
+	struct nfs4_client	       *clr_client;
+	struct nfs4_file	       *clr_file;
+	struct timespec			clr_time;	/* last activity */
+	struct super_block 		*clr_sb; /* We might not have a file */
+	struct nfs4_layoutrecall	*parent; /* The initiating recall */
+};
+
+/* notify device request (from exported filesystem) */
+struct nfs4_notify_device {
+	struct nfsd4_pnfs_cb_dev_list  *nd_list;
+	struct nfs4_client	       *nd_client;
+	struct list_head	        nd_perclnt;
+};
+
 /* Used by layout_get to encode layout (loc_body var in spec)
  * Args:
  * minlength - min number of accessible bytes given by layout
@@ -168,15 +216,6 @@ struct nfsd4_pnfs_open {
 	int			op_truncate;
 };
 
-struct nfsd4_pnfs_cb_layout {
-	u32			cbl_recall_type;	/* request */
-	struct nfsd4_layout_seg cbl_seg;		/* request */
-	u32			cbl_layoutchanged;	/* request */
-	stateid_t		cbl_sid;		/* request */
-	struct nfs4_fsid	cbl_fsid;
-	void			*cbl_cookie;		/* fs private */
-};
-
 struct nfsd4_pnfs_cb_dev_item {
 	u32			cbd_notify_type;	/* request */
 	u32			cbd_layout_type;	/* request */
diff --git a/include/linux/nfsd/pnfsd.h b/include/linux/nfsd/pnfsd.h
index d1d7395..6aa3a87 100644
--- a/include/linux/nfsd/pnfsd.h
+++ b/include/linux/nfsd/pnfsd.h
@@ -96,14 +96,6 @@ int nfs4_pnfs_return_layout(struct super_block *, struct svc_fh *,
 					struct nfsd4_pnfs_layoutreturn *);
 void pnfs_set_device_notify(clientid_t *clid, unsigned int types);
 void nfs4_pnfs_state_shutdown(void);
-
-#if defined(CONFIG_PNFSD_LOCAL_EXPORT)
-extern struct sockaddr pnfsd_lexp_addr;
-extern size_t pnfs_lexp_addr_len;
-
-void pnfsd_lexp_init(struct inode *inode);
-#endif /* CONFIG_PNFSD_LOCAL_EXPORT */
-
 #endif /* CONFIG_PNFSD */
 
 #endif /* LINUX_NFSD_PNFSD_H */
diff --git a/include/linux/nfsd/state.h b/include/linux/nfsd/state.h
index ffd5a41..7367696 100644
--- a/include/linux/nfsd/state.h
+++ b/include/linux/nfsd/state.h
@@ -40,6 +40,7 @@
 #include <linux/list.h>
 #include <linux/kref.h>
 #include <linux/sunrpc/clnt.h>
+#include <linux/nfsd/nfsfh.h>
 
 typedef struct {
 	u32             cl_boot;
@@ -245,52 +246,6 @@ struct nfs4_fsid {
 	u64	minor;
 };
 
-#if defined(CONFIG_PNFSD)
-
-#include <linux/nfsd/nfsd4_pnfs.h>
-
-/* outstanding layout stateid */
-struct nfs4_layout_state {
-	struct list_head	ls_perfile;
-	struct list_head	ls_layouts; /* list of nfs4_layouts */
-	struct kref		ls_ref;
-	struct nfs4_client	*ls_client;
-	struct nfs4_file	*ls_file;
-	stateid_t		ls_stateid;
-};
-
-/* outstanding layout */
-struct nfs4_layout {
-	struct list_head		lo_perfile;	/* hash by f_id */
-	struct list_head		lo_perclnt;	/* hash by clientid */
-	struct list_head		lo_perstate;
-	struct nfs4_file		*lo_file;	/* backpointer */
-	struct nfs4_client		*lo_client;
-	struct nfs4_layout_state	*lo_state;
-	struct nfsd4_layout_seg 	lo_seg;
-};
-
-/* layoutrecall request (from exported filesystem) */
-struct nfs4_layoutrecall {
-	struct kref			clr_ref;
-	struct nfsd4_pnfs_cb_layout	cb;	/* request */
-	struct list_head		clr_perclnt; /* on cl_layoutrecalls */
-	struct nfs4_client	       *clr_client;
-	struct nfs4_file	       *clr_file;
-	struct timespec			clr_time;	/* last activity */
-	struct super_block 		*clr_sb; /* We might not have a file */
-	struct nfs4_layoutrecall	*parent; /* The initiating recall */
-};
-
-/* notify device request (from exported filesystem) */
-struct nfs4_notify_device {
-	struct nfsd4_pnfs_cb_dev_list  *nd_list;
-	struct nfs4_client	       *nd_client;
-	struct list_head	        nd_perclnt;
-};
-
-#endif /* CONFIG_PNFSD */
-
 /* struct nfs4_client_reset
  * one per old client. Populates reset_str_hashtbl. Filled from conf_id_hashtbl
  * upon lease reset, or from upcall to state_daemon (to read in state
@@ -479,44 +434,6 @@ extern struct nfs4_delegation * find_delegation_stateid(struct inode *ino,
 		stateid_t *stid);
 extern __be32 nfs4_check_stateid(stateid_t *stateid);
 extern void expire_client_lock(struct nfs4_client *clp);
-#if defined(CONFIG_PNFSD)
-extern void release_pnfs_ds_dev_list(struct nfs4_stateid *stp);
-extern void nfs4_pnfs_state_init(void);
-extern int put_layoutrecall(struct nfs4_layoutrecall *);
-extern void nomatching_layout(struct nfs4_layoutrecall *);
-extern void *layoutrecall_done(struct nfs4_layoutrecall *);
-extern int nfsd4_cb_layout(struct nfs4_layoutrecall *lp);
-extern void nfsd4_free_pnfs_slabs(void);
-extern void nfsd4_free_slab(struct kmem_cache **slab);
-extern int nfsd4_init_pnfs_slabs(void);
-extern struct nfs4_file * find_file(struct inode *ino);
-extern struct nfs4_file * alloc_init_file(struct inode *ino, struct svc_fh *current_fh);
-extern void put_nfs4_file(struct nfs4_file *fi);
-extern void get_nfs4_file(struct nfs4_file *fi);
-extern struct nfs4_client * find_confirmed_client(clientid_t *clid);
-extern void nfs4_bug_on_unlocked_state(void);
-extern void release_pnfs_ds_dev_list(struct nfs4_stateid *stp);
-extern void nfs4_ds_get_verifier(stateid_t *stateid,
-		struct super_block *sb, u32 *p);
-extern int nfs4_preprocess_pnfs_ds_stateid(struct svc_fh *, stateid_t *);
-extern void nfs4_pnfs_state_shutdown(void);
-extern int lo_recall_per_client(struct nfs4_client *clp,
-		struct nfsd4_pnfs_cb_layout *cbl, struct nfs4_file *lrfile,
-		struct list_head *todolist);
-extern int create_layout_recall_list(struct list_head *todolist,
-		unsigned *todo_len, struct nfsd4_pnfs_cb_layout *cbl,
-		struct nfs4_file *lrfile);
-extern int nfsd4_cb_notify_device(struct nfs4_notify_device *cbnd);
-extern void pnfs_clear_device_notify(struct nfs4_client *clp);
-extern int create_device_notify_list(struct list_head *todolist,
-		struct nfsd4_pnfs_cb_dev_list *ndl);
-extern void pnfs_expire_client(struct nfs4_client *clp);
-#else /* CONFIG_PNFSD */
-static inline void nfsd4_free_pnfs_slabs(void) {}
-static inline int nfsd4_init_pnfs_slabs(void) { return 0; }
-static inline void release_pnfs_ds_dev_list(struct nfs4_stateid *stp) {}
-static inline void pnfs_expire_client(struct nfs4_client *clp) {}
-#endif /* CONFIG_PNFSD */
 
 static inline void
 nfs4_put_stateowner(struct nfs4_stateowner *so)
-- 
1.6.2.1

[PATCH 5/5] nfsd: Remove lots of un-needed includes

[Boaz Harrosh]

Once a few headers where fixed lots of includes are
not needed any more. (OK probably a few where so even
before)

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 fs/exportfs/nfs4filelayoutxdr.c |    8 --------
 fs/nfsd/export.c                |   16 ----------------
 fs/nfsd/nfs4callback.c          |   14 --------------
 fs/nfsd/nfs4pnfsd.c             |    9 ---------
 fs/nfsd/nfs4proc.c              |    9 ---------
 fs/nfsd/nfs4state.c             |   18 ------------------
 fs/nfsd/pnfsd_lexp.c            |    5 -----
 fs/nfsd/spnfs_com.c             |   13 -------------
 8 files changed, 0 insertions(+), 92 deletions(-)

diff --git a/fs/exportfs/nfs4filelayoutxdr.c b/fs/exportfs/nfs4filelayoutxdr.c
index 9ce2a9d..d07685d 100644
--- a/fs/exportfs/nfs4filelayoutxdr.c
+++ b/fs/exportfs/nfs4filelayoutxdr.c
@@ -35,15 +35,7 @@
 */
 #if defined(CONFIG_EXPORTFS_FILE_LAYOUT)
 
-#include <linux/module.h>
-#include <linux/sunrpc/svc.h>
-#include <linux/nfsd/nfsd.h>
-#include <linux/nfsd/cache.h>
-#include <linux/nfs4.h>
-#include <linux/nfsd/state.h>
-#include <linux/nfsd/xdr4.h>
 #include <linux/nfsd/nfs4layoutxdr.h>
-#include <linux/nfsd/nfsd4_pnfs.h>
 
 /* We do our-own dprintk so filesystems are not dependent on sunrpc */
 #ifdef dprintk
diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index f51c413..616980c 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -14,23 +14,11 @@
  * Copyright (C) 1995, 1996 Olaf Kirch, <okir-pn4DOG8n3UYbFoVRYvo4fw@public.gmane.org>
  */
 
-#include <linux/unistd.h>
-#include <linux/slab.h>
-#include <linux/stat.h>
-#include <linux/in.h>
-#include <linux/seq_file.h>
 #include <linux/syscalls.h>
-#include <linux/rwsem.h>
-#include <linux/dcache.h>
 #include <linux/namei.h>
-#include <linux/mount.h>
-#include <linux/hash.h>
-#include <linux/module.h>
 #include <linux/exportfs.h>
 
-#include <linux/sunrpc/svc.h>
 #include <linux/nfsd/nfsd.h>
-#include <linux/nfsd/nfsfh.h>
 #include <linux/nfsd/pnfsd.h>
 #include "nfsd_priv.h"
 #if defined(CONFIG_SPNFS)
@@ -40,10 +28,6 @@
 #endif
 #endif
 #include <linux/nfsd/syscall.h>
-#include <linux/lockd/bind.h>
-#include <linux/sunrpc/msg_prot.h>
-#include <linux/sunrpc/gss_api.h>
-#include <net/ipv6.h>
 
 #define NFSDDBG_FACILITY	NFSDDBG_EXPORT
 
diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 3347749..caf84a6 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -33,22 +33,8 @@
  *  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#include <linux/module.h>
-#include <linux/list.h>
-#include <linux/inet.h>
-#include <linux/errno.h>
-#include <linux/delay.h>
-#include <linux/sched.h>
-#include <linux/kthread.h>
-#include <linux/sunrpc/xdr.h>
-#include <linux/sunrpc/svc.h>
-#include <linux/sunrpc/clnt.h>
-#include <linux/sunrpc/svcsock.h>
 #include <linux/nfsd/nfsd.h>
 #include <linux/nfsd/state.h>
-#include <linux/sunrpc/sched.h>
-#include <linux/nfs4.h>
-#include <linux/sunrpc/xprtsock.h>
 
 #define NFSDDBG_FACILITY                NFSDDBG_PROC
 
diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index fdbc447..cfffead 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -23,16 +23,7 @@
 
 #if defined(CONFIG_PNFSD)
 
-#include <linux/param.h>
-#include <linux/slab.h>
-#include <linux/sunrpc/svc.h>
-#include <linux/sunrpc/debug.h>
 #include <linux/nfsd/nfsd.h>
-#include <linux/nfs4.h>
-#include <linux/nfsd/state.h>
-#include <linux/nfsd/xdr4.h>
-#include <linux/exportfs.h>
-#include <linux/nfsd/pnfsd.h>
 #include "nfsd_priv.h"
 
 #define NFSDDBG_FACILITY                NFSDDBG_PROC
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index dd1da22..e22a9e6 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -35,23 +35,14 @@
  *  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#include <linux/param.h>
-#include <linux/major.h>
-#include <linux/slab.h>
 #include <linux/file.h>
 
-#include <linux/sunrpc/svc.h>
 #include <linux/nfsd/nfsd.h>
 #include <linux/nfsd/cache.h>
-#include <linux/nfs4.h>
 #include <linux/nfsd/state.h>
 #include <linux/nfsd/xdr4.h>
-#include <linux/nfs4_acl.h>
-#include <linux/sunrpc/gss_api.h>
 #if defined(CONFIG_PNFSD)
 #include <linux/nfsd/pnfsd.h>
-#include <linux/exportfs.h>
-#include <linux/nfsd/nfs4layoutxdr.h>
 #include "nfsd_priv.h"
 #if defined(CONFIG_SPNFS)
 #include <linux/nfsd4_spnfs.h>
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 018292e..b635ed0 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -34,31 +34,13 @@
 *
 */
 
-#include <linux/param.h>
-#include <linux/major.h>
-#include <linux/slab.h>
-
-#include <linux/sunrpc/svc.h>
-#include <linux/nfsd/nfsd.h>
-#include <linux/nfsd/cache.h>
 #include <linux/file.h>
-#include <linux/mount.h>
-#include <linux/workqueue.h>
 #include <linux/smp_lock.h>
-#include <linux/kthread.h>
-#include <linux/nfs4.h>
 #include <linux/nfsd/state.h>
 #include <linux/nfsd/xdr4.h>
 #include <linux/namei.h>
 #include <linux/swap.h>
-#include <linux/mutex.h>
-#include <linux/lockd/bind.h>
-#include <linux/module.h>
 #include <linux/sunrpc/svcauth_gss.h>
-#include <linux/sunrpc/clnt.h>
-#if defined(CONFIG_PNFSD)
-#include <linux/nfsd/pnfsd.h>
-#endif /* CONFIG_PNFSD */
 #include "nfsd_priv.h"
 
 #define NFSDDBG_FACILITY                NFSDDBG_PROC
diff --git a/fs/nfsd/pnfsd_lexp.c b/fs/nfsd/pnfsd_lexp.c
index 2bcb6b5..a09e3c1 100644
--- a/fs/nfsd/pnfsd_lexp.c
+++ b/fs/nfsd/pnfsd_lexp.c
@@ -20,12 +20,7 @@
 
 #if defined(CONFIG_PNFSD_LOCAL_EXPORT)
 
-#include <linux/nfs_fs.h>
-#include <linux/sunrpc/svc.h>
 #include <linux/sunrpc/svc_xprt.h>
-#include <linux/nfsd/nfsfh.h>
-#include <linux/nfsd/state.h>
-#include <linux/nfsd/pnfsd.h>
 #include <linux/nfsd/nfs4layoutxdr.h>
 #include <linux/nfsd/debug.h>
  
diff --git a/fs/nfsd/spnfs_com.c b/fs/nfsd/spnfs_com.c
index 7c7d48d..deab09b 100644
--- a/fs/nfsd/spnfs_com.c
+++ b/fs/nfsd/spnfs_com.c
@@ -39,23 +39,10 @@
  */
 #if defined(CONFIG_SPNFS)
 
-#include <linux/module.h>
-#include <linux/mutex.h>
-#include <linux/init.h>
-#include <linux/types.h>
-#include <linux/slab.h>
-#include <linux/socket.h>
-#include <linux/in.h>
-#include <linux/sched.h>
 #include <linux/namei.h>
 #include <linux/mount.h>
-#include <linux/path.h>
 #include <linux/sunrpc/clnt.h>
-#include <linux/workqueue.h>
 #include <linux/sunrpc/rpc_pipe_fs.h>
-#include <linux/proc_fs.h>
-
-#include <linux/nfs_fs.h>
 
 #include <linux/nfsd4_spnfs.h>
 #include <linux/nfsd/debug.h>
-- 
1.6.2.1

Re: [pnfs] [PATCH 1/5] sunrpc: Clean never used include files

[Boaz Harrosh]

On 10/21/2009 10:11 AM, Boaz Harrosh wrote:
> Remove include of two headers never used by this file.
> I did not see any users dependency but if exist they
> should be fixed since these headers are totally irrelevant
> to here.
> 
> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
> ---
>  include/linux/sunrpc/debug.h |    3 ---
>  1 files changed, 0 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h
> index 10709cb..c2786f2 100644
> --- a/include/linux/sunrpc/debug.h
> +++ b/include/linux/sunrpc/debug.h
> @@ -28,9 +28,6 @@
>  
>  #ifdef __KERNEL__
>  
> -#include <linux/timer.h>
> -#include <linux/workqueue.h>
> -
>  /*
>   * Enable RPC debugging/profiling.
>   */

This patch exposed a missing "#include <linux/types.h>" in 
linux/sunrpc/rpc_rdma.h. I'll re-post version 2 soon 

Boaz

[PATCH version2] sunrpc: Clean never used include files

[Boaz Harrosh]

Remove include of two headers never used by this file.
Doing so exposed a missing #include <linux/types.h> in
include/linux/sunrpc/rpc_rdma.h.

I did not see any other users dependency but if exist they
should be fixed since these headers are totally irrelevant
to here.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 include/linux/sunrpc/debug.h    |    3 ---
 include/linux/sunrpc/rpc_rdma.h |    2 ++
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h
index 10709cb..c2786f2 100644
--- a/include/linux/sunrpc/debug.h
+++ b/include/linux/sunrpc/debug.h
@@ -28,9 +28,6 @@
 
 #ifdef __KERNEL__
 
-#include <linux/timer.h>
-#include <linux/workqueue.h>
-
 /*
  * Enable RPC debugging/profiling.
  */
diff --git a/include/linux/sunrpc/rpc_rdma.h b/include/linux/sunrpc/rpc_rdma.h
index 87b895d..b78f16b 100644
--- a/include/linux/sunrpc/rpc_rdma.h
+++ b/include/linux/sunrpc/rpc_rdma.h
@@ -40,6 +40,8 @@
 #ifndef _LINUX_SUNRPC_RPC_RDMA_H
 #define _LINUX_SUNRPC_RPC_RDMA_H
 
+#include <linux/types.h>
+
 struct rpcrdma_segment {
 	__be32 rs_handle;	/* Registered memory handle */
 	__be32 rs_length;	/* Length of the chunk in bytes */
-- 
1.6.2.1

Re: [PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[Trond Myklebust]

On Wed, 2009-10-21 at 10:14 +0200, Boaz Harrosh wrote:
> An header should be compilation independent, .i.e pull in
> any header who's declarations are directly used by this header.
> And not let users re-include all it's dependencies all over
> again.
> 
> [At the end of the day what's the use of a header if it does
>  not have more then one user?]


The problem with this is that you quickly end up including the same
header files over and over and over again as they get pulled in several
times over by different headers.



-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: [PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[Boaz Harrosh]

On 10/22/2009 02:28 AM, Trond Myklebust wrote:
> On Wed, 2009-10-21 at 10:14 +0200, Boaz Harrosh wrote:
>> An header should be compilation independent, .i.e pull in
>> any header who's declarations are directly used by this header.
>> And not let users re-include all it's dependencies all over
>> again.
>>
>> [At the end of the day what's the use of a header if it does
>>  not have more then one user?]
> 
> 
> The problem with this is that you quickly end up including the same
> header files over and over and over again as they get pulled in several
> times over by different headers.
> 
> 
> 

What is the problem with that?

two things:
1. it is unavoidable.
2. That's why we invented the #ifndef FOO_H #define FOO_H for.
3. Look at the current mess, to the point that you don't understand why
   the code does not compile, you end up just copy-past the include list
   of that other file, and now actually do end with extra un-needed includes.
   (Don't believe me look at the last patch as proof).
4. So I add to an header a use of a type that now needs a new include.
   All my users do not compile any more. What to do? OK I'll include it so
   not to change all existing users all over again. Now we get a double
   standard. All headers used before any users, must be carried around.
   The late comers are escaped.
5. The opposite of 4. An header is no longer needed. Extra header left at all
   users.

It used to be a problem before me and you have begun programing.
Since then the cpp looks at it's internal structures and will not re-open.
Note that the compiler never sees the second instance, ever.

That said we have no choice of the matter. It is a Kernel style guide. I
should know because I was banged on the head with it a couple of times.
And rightly so.

Come on that was a joke right
Boaz

Re: [PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[Benny Halevy]

On Oct. 22, 2009, 10:18 +0200, Boaz Harrosh <bharrosh@panasas.com> wrote:
> On 10/22/2009 02:28 AM, Trond Myklebust wrote:
>> On Wed, 2009-10-21 at 10:14 +0200, Boaz Harrosh wrote:
>>> An header should be compilation independent, .i.e pull in
>>> any header who's declarations are directly used by this header.
>>> And not let users re-include all it's dependencies all over
>>> again.
>>>
>>> [At the end of the day what's the use of a header if it does
>>>  not have more then one user?]
>>
>> The problem with this is that you quickly end up including the same
>> header files over and over and over again as they get pulled in several
>> times over by different headers.
>>
>>
>>
> 
> What is the problem with that?
> 
> two things:
> 1. it is unavoidable.
> 2. That's why we invented the #ifndef FOO_H #define FOO_H for.
> 3. Look at the current mess, to the point that you don't understand why
>    the code does not compile, you end up just copy-past the include list
>    of that other file, and now actually do end with extra un-needed includes.
>    (Don't believe me look at the last patch as proof).
> 4. So I add to an header a use of a type that now needs a new include.
>    All my users do not compile any more. What to do? OK I'll include it so
>    not to change all existing users all over again. Now we get a double
>    standard. All headers used before any users, must be carried around.
>    The late comers are escaped.
> 5. The opposite of 4. An header is no longer needed. Extra header left at all
>    users.
> 
> It used to be a problem before me and you have begun programing.
> Since then the cpp looks at it's internal structures and will not re-open.
> Note that the compiler never sees the second instance, ever.


For the fun of it, I took an average of 5 incremental build
times before and after this patchset, average user and system
times below:

before: 68.84 12.51
after:  68.74 12.48

This shows a minor improvement of about 0.14%
for user time and 0.27% for system time.

Benny

> 
> That said we have no choice of the matter. It is a Kernel style guide. I
> should know because I was banged on the head with it a couple of times.
> And rightly so.
> 
> Come on that was a joke right
> Boaz

Re: [PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[Trond Myklebust]

On Thu, 2009-10-22 at 10:18 +0200, Boaz Harrosh wrote:
> On 10/22/2009 02:28 AM, Trond Myklebust wrote:
> > On Wed, 2009-10-21 at 10:14 +0200, Boaz Harrosh wrote:
> >> An header should be compilation independent, .i.e pull in
> >> any header who's declarations are directly used by this header.
> >> And not let users re-include all it's dependencies all over
> >> again.
> >>
> >> [At the end of the day what's the use of a header if it does
> >>  not have more then one user?]
> > 
> > 
> > The problem with this is that you quickly end up including the same
> > header files over and over and over again as they get pulled in several
> > times over by different headers.
> > 
> > 
> > 
> 
> What is the problem with that?
> 
> two things:
> 1. it is unavoidable.
> 2. That's why we invented the #ifndef FOO_H #define FOO_H for.
> 3. Look at the current mess, to the point that you don't understand why
>    the code does not compile, you end up just copy-past the include list
>    of that other file, and now actually do end with extra un-needed includes.
>    (Don't believe me look at the last patch as proof).
> 4. So I add to an header a use of a type that now needs a new include.
>    All my users do not compile any more. What to do? OK I'll include it so
>    not to change all existing users all over again. Now we get a double
>    standard. All headers used before any users, must be carried around.
>    The late comers are escaped.
> 5. The opposite of 4. An header is no longer needed. Extra header left at all
>    users.
> 
> It used to be a problem before me and you have begun programing.
> Since then the cpp looks at it's internal structures and will not re-open.
> Note that the compiler never sees the second instance, ever.
> 
> That said we have no choice of the matter. It is a Kernel style guide. I
> should know because I was banged on the head with it a couple of times.
> And rightly so.
> 
> Come on that was a joke right
> Boaz

No. What I'm saying is that this doesn't have to be an absolute rule.
The Kernel style guide assumes that everything in 'include/*' is going
to be exported all around the kernel.
The problem is that we put a lot of stuff which is private to fs/nfs and
fs/nfsd in there. Those header files do not have to absolutely follow
the style guide rule, 'cos we know what is being included before and
after them...

Cheers
  Trond

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: [PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[Boaz Harrosh]

On 10/22/2009 04:02 PM, Trond Myklebust wrote:
> On Thu, 2009-10-22 at 10:18 +0200, Boaz Harrosh wrote:
>> On 10/22/2009 02:28 AM, Trond Myklebust wrote:
>>> On Wed, 2009-10-21 at 10:14 +0200, Boaz Harrosh wrote:
>>>> An header should be compilation independent, .i.e pull in
>>>> any header who's declarations are directly used by this header.
>>>> And not let users re-include all it's dependencies all over
>>>> again.
>>>>
>>>> [At the end of the day what's the use of a header if it does
>>>>  not have more then one user?]
>>>
>>>
>>> The problem with this is that you quickly end up including the same
>>> header files over and over and over again as they get pulled in several
>>> times over by different headers.
>>>
>>>
>>>
>>
>> What is the problem with that?
>>
>> two things:
>> 1. it is unavoidable.
>> 2. That's why we invented the #ifndef FOO_H #define FOO_H for.
>> 3. Look at the current mess, to the point that you don't understand why
>>    the code does not compile, you end up just copy-past the include list
>>    of that other file, and now actually do end with extra un-needed includes.
>>    (Don't believe me look at the last patch as proof).
>> 4. So I add to an header a use of a type that now needs a new include.
>>    All my users do not compile any more. What to do? OK I'll include it so
>>    not to change all existing users all over again. Now we get a double
>>    standard. All headers used before any users, must be carried around.
>>    The late comers are escaped.
>> 5. The opposite of 4. An header is no longer needed. Extra header left at all
>>    users.
>>
>> It used to be a problem before me and you have begun programing.
>> Since then the cpp looks at it's internal structures and will not re-open.
>> Note that the compiler never sees the second instance, ever.
>>
>> That said we have no choice of the matter. It is a Kernel style guide. I
>> should know because I was banged on the head with it a couple of times.
>> And rightly so.
>>
>> Come on that was a joke right
>> Boaz
> 
> No. What I'm saying is that this doesn't have to be an absolute rule.
> The Kernel style guide assumes that everything in 'include/*' is going
> to be exported all around the kernel.
> The problem is that we put a lot of stuff which is private to fs/nfs and
> fs/nfsd in there. Those header files do not have to absolutely follow
> the style guide rule, 'cos we know what is being included before and
> after them...
> 

I'm not sure I understand
You are saying that the patches are very good, but only
the rule I stated originally could be relaxed a little with private
headers where we might get lazy, if the effects are very local?

Well, that's not a problem then, right? just that I can relax a bit
if I want.

But I disagree: see 3, 4, 5 above and that last patch I submitted. That patch
is only the beginning. 85% of all source files in nfs/nfsd could receive the
same love. I only done these I touched. Code tends to stay much-much longer
then we spend time on it. Better get it in shape the first time.

> Cheers
>   Trond
> 

I've now compiled this with x86 allmodeconfig. It should stay in linux-next for a while
to make sure there's no side effects.

Boaz

Re: [PATCH 4/5] SQUASHME pnfsd: Move pnfsd code out of nfs4state.c/h

[Benny Halevy]

Boaz,  I merged this patch into the pnfs tree.
Only it at this point, until the cleanup, which I like, gets acked.

I actually split it up for pnfsd-files and restricted the use
of CONFIG_PNFSD in the private header file only for functions
that are defined under the same config variable.

Benny

On Oct. 21, 2009, 10:15 +0200, Boaz Harrosh <bharrosh@panasas.com> wrote:
> * Introduce an fs/nfsd/nfsd_priv.h header that collects private
>   nfsd definitions/declarations. Specially all none exported
>   extern will move to that file.
> 
> * Include "nfsd_priv.h" in nfsd source files where needed.
> 
> * pnfs specific declarations are moved out of linux/nfsd/state.h and
>   into linux/nfsd/nfsd4_pnfs.h, though removing inclusion of
>   nfsd4_pnfs.h from state.h and all the #if defined(CONFIG_PNFSD)
> 
> * Finally moving remaining #if defined(CONFIG_PNFSD) code from
>   nfs4state.c into nfs4pnfsd.c
> 
> TODO: I know that lots of other code from include/linux/... nfsd
> headers could/should be moved into "nfsd_priv.h"
> 
> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
> ---
>  fs/nfsd/export.c                |    1 +
>  fs/nfsd/nfs4callback.c          |    3 +
>  fs/nfsd/nfs4pnfsd.c             |   83 ++++++++++++++++++++++++++++++
>  fs/nfsd/nfs4proc.c              |    1 +
>  fs/nfsd/nfs4state.c             |  101 +------------------------------------
>  fs/nfsd/nfsd_priv.h             |  105 +++++++++++++++++++++++++++++++++++++++
>  include/linux/nfsd/nfsd4_pnfs.h |   61 ++++++++++++++++++----
>  include/linux/nfsd/pnfsd.h      |    8 ---
>  include/linux/nfsd/state.h      |   85 +-------------------------------
>  9 files changed, 246 insertions(+), 202 deletions(-)
>  create mode 100644 fs/nfsd/nfsd_priv.h
> 
> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> index 0eea1a6..f51c413 100644
> --- a/fs/nfsd/export.c
> +++ b/fs/nfsd/export.c
> @@ -32,6 +32,7 @@
>  #include <linux/nfsd/nfsd.h>
>  #include <linux/nfsd/nfsfh.h>
>  #include <linux/nfsd/pnfsd.h>
> +#include "nfsd_priv.h"
>  #if defined(CONFIG_SPNFS)
>  #include <linux/nfsd4_spnfs.h>
>  #if defined(CONFIG_SPNFS_BLOCK)
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index b53194c..3347749 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -311,6 +311,9 @@ encode_cb_sequence(struct xdr_stream *xdr, struct nfsd4_cb_sequence *args,
>  }
>  
>  #if defined(CONFIG_PNFSD)
> +
> +#include "nfsd_priv.h"
> +
>  static void
>  encode_cb_layout(struct xdr_stream *xdr, struct nfs4_layoutrecall *clr,
>  		 struct nfs4_cb_compound_hdr *hdr)
> diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
> index d52a2e5..fdbc447 100644
> --- a/fs/nfsd/nfs4pnfsd.c
> +++ b/fs/nfsd/nfs4pnfsd.c
> @@ -33,6 +33,7 @@
>  #include <linux/nfsd/xdr4.h>
>  #include <linux/exportfs.h>
>  #include <linux/nfsd/pnfsd.h>
> +#include "nfsd_priv.h"
>  
>  #define NFSDDBG_FACILITY                NFSDDBG_PROC
>  
> @@ -1440,4 +1441,86 @@ int nfsd_device_notify_cb(struct super_block *sb,
>  		__func__, status, notify_num);
>  	return status;
>  }
> +
> +/* Create a layoutrecall structure for each client based on the
> + * original structure. */
> +int
> +create_layout_recall_list(struct list_head *todolist, unsigned *todo_len,
> +			  struct nfsd4_pnfs_cb_layout *cbl,
> +			  struct nfs4_file *lrfile)
> +{
> +	struct nfs4_client *clp;
> +	unsigned int i, len = 0;
> +	int status = 0;
> +
> +	dprintk("%s: -->\n", __func__);
> +
> +	/* If client given by fs, just do single client */
> +	if (cbl->cbl_seg.clientid) {
> +		clp = find_confirmed_client(
> +				(clientid_t *)&cbl->cbl_seg.clientid);
> +		if (!clp) {
> +			status = -ENOENT;
> +			dprintk("%s: clientid %llx not found\n", __func__,
> +				(unsigned long long)cbl->cbl_seg.clientid);
> +			goto out;
> +		}
> +
> +		status = lo_recall_per_client(clp, cbl, lrfile, todolist);
> +		if (!status)
> +			len++;
> +		goto out;
> +	}
> +
> +	/* Check all clients for layout matches */
> +	for (i = 0; i < CLIENT_HASH_SIZE; i++)
> +		list_for_each_entry(clp, &conf_str_hashtbl[i], cl_strhash) {
> +			status = lo_recall_per_client(clp, cbl, lrfile,
> +						      todolist);
> +			if (!status)
> +				len++;
> +			else if (status != -ENOENT)
> +				goto out;
> +		}
> +out:
> +	*todo_len = len;
> +	/* -ENOENT is a good thing don't return it if some recalls are needed */
> +	if ((status == -ENOENT) && len)
> +		status = 0;
> +	dprintk("%s: <-- list len %u status %d\n", __func__, len, status);
> +	return status;
> +}
> +
> +/* Create a list of clients to send device notifications. */
> +int
> +create_device_notify_list(struct list_head *todolist,
> +			  struct nfsd4_pnfs_cb_dev_list *ndl)
> +{
> +	int status = 0, i;
> +	struct nfs4_client *clp = NULL;
> +	struct nfs4_notify_device *cbnd;
> +
> +	nfs4_lock_state();
> +
> +	/* Create notify client list */
> +	for (i = 0; i < CLIENT_HASH_SIZE; i++)
> +		list_for_each_entry(clp, &conf_str_hashtbl[i], cl_strhash) {
> +			if (atomic_read(&clp->cl_deviceref) <= 0)
> +				continue;
> +			cbnd = kmalloc(sizeof(*cbnd), GFP_KERNEL);
> +			if (!cbnd) {
> +				status = -ENOMEM;
> +				goto out;
> +			}
> +			cbnd->nd_list = ndl;
> +			cbnd->nd_client = clp;
> +			list_add(&cbnd->nd_perclnt, todolist);
> +			atomic_inc(&clp->cl_count);
> +		}
> +
> +out:
> +	nfs4_unlock_state();
> +	return status;
> +}
> +
>  #endif /* CONFIG_PNFSD */
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 83acf8d..dd1da22 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -52,6 +52,7 @@
>  #include <linux/nfsd/pnfsd.h>
>  #include <linux/exportfs.h>
>  #include <linux/nfsd/nfs4layoutxdr.h>
> +#include "nfsd_priv.h"
>  #if defined(CONFIG_SPNFS)
>  #include <linux/nfsd4_spnfs.h>
>  #if defined(CONFIG_SPNFS_BLOCK)
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 7495df8..018292e 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -59,6 +59,7 @@
>  #if defined(CONFIG_PNFSD)
>  #include <linux/nfsd/pnfsd.h>
>  #endif /* CONFIG_PNFSD */
> +#include "nfsd_priv.h"
>  
>  #define NFSDDBG_FACILITY                NFSDDBG_PROC
>  
> @@ -275,20 +276,6 @@ unhash_delegation(struct nfs4_delegation *dp)
>  	nfs4_put_delegation(dp);
>  }
>  
> -/* 
> - * SETCLIENTID state 
> - */
> -
> -/* Hash tables for nfs4_clientid state */
> -#define CLIENT_HASH_BITS                 4
> -#define CLIENT_HASH_SIZE                (1 << CLIENT_HASH_BITS)
> -#define CLIENT_HASH_MASK                (CLIENT_HASH_SIZE - 1)
> -
> -#define clientid_hashval(id) \
> -	((id) & CLIENT_HASH_MASK)
> -#define clientstr_hashval(name) \
> -	(opaque_hashval((name), 8) & CLIENT_HASH_MASK)
> -
>  /*
>   * reclaim_str_hashtbl[] holds known client info from previous reset/reboot
>   * used in reboot/reset lease grace period processing
> @@ -308,7 +295,7 @@ unhash_delegation(struct nfs4_delegation *dp)
>  static struct list_head reclaim_str_hashtbl[CLIENT_HASH_SIZE];
>  static int reclaim_str_hashtbl_size = 0;
>  static struct list_head	conf_id_hashtbl[CLIENT_HASH_SIZE];
> -static struct list_head	conf_str_hashtbl[CLIENT_HASH_SIZE];
> +struct list_head	conf_str_hashtbl[CLIENT_HASH_SIZE];
>  static struct list_head	unconf_str_hashtbl[CLIENT_HASH_SIZE];
>  static struct list_head	unconf_id_hashtbl[CLIENT_HASH_SIZE];
>  static struct list_head client_lru;
> @@ -4245,87 +4232,3 @@ nfs4_reset_lease(time_t leasetime)
>  	user_lease_time = leasetime;
>  }
>  
> -#if defined(CONFIG_PNFSD)
> -
> -/* Create a layoutrecall structure for each client based on the
> - * original structure. */
> -int
> -create_layout_recall_list(struct list_head *todolist, unsigned *todo_len,
> -			  struct nfsd4_pnfs_cb_layout *cbl,
> -			  struct nfs4_file *lrfile)
> -{
> -	struct nfs4_client *clp;
> -	unsigned int i, len = 0;
> -	int status = 0;
> -
> -	dprintk("%s: -->\n", __func__);
> -
> -	/* If client given by fs, just do single client */
> -	if (cbl->cbl_seg.clientid) {
> -		clp = find_confirmed_client(
> -				(clientid_t *)&cbl->cbl_seg.clientid);
> -		if (!clp) {
> -			status = -ENOENT;
> -			dprintk("%s: clientid %llx not found\n", __func__,
> -				(unsigned long long)cbl->cbl_seg.clientid);
> -			goto out;
> -		}
> -
> -		status = lo_recall_per_client(clp, cbl, lrfile, todolist);
> -		if (!status)
> -			len++;
> -		goto out;
> -	}
> -
> -	/* Check all clients for layout matches */
> -	for (i = 0; i < CLIENT_HASH_SIZE; i++)
> -		list_for_each_entry(clp, &conf_str_hashtbl[i], cl_strhash) {
> -			status = lo_recall_per_client(clp, cbl, lrfile,
> -						      todolist);
> -			if (!status)
> -				len++;
> -			else if (status != -ENOENT)
> -				goto out;
> -		}
> -out:
> -	*todo_len = len;
> -	/* -ENOENT is a good thing don't return it if some recalls are needed */
> -	if ((status == -ENOENT) && len)
> -		status = 0;
> -	dprintk("%s: <-- list len %u status %d\n", __func__, len, status);
> -	return status;
> -}
> -
> -/* Create a list of clients to send device notifications. */
> -int
> -create_device_notify_list(struct list_head *todolist,
> -			  struct nfsd4_pnfs_cb_dev_list *ndl)
> -{
> -	int status = 0, i;
> -	struct nfs4_client *clp = NULL;
> -	struct nfs4_notify_device *cbnd;
> -
> -	nfs4_lock_state();
> -
> -	/* Create notify client list */
> -	for (i = 0; i < CLIENT_HASH_SIZE; i++)
> -		list_for_each_entry(clp, &conf_str_hashtbl[i], cl_strhash) {
> -			if (atomic_read(&clp->cl_deviceref) <= 0)
> -				continue;
> -			cbnd = kmalloc(sizeof(*cbnd), GFP_KERNEL);
> -			if (!cbnd) {
> -				status = -ENOMEM;
> -				goto out;
> -			}
> -			cbnd->nd_list = ndl;
> -			cbnd->nd_client = clp;
> -			list_add(&cbnd->nd_perclnt, todolist);
> -			atomic_inc(&clp->cl_count);
> -		}
> -
> -out:
> -	nfs4_unlock_state();
> -	return status;
> -}
> -
> -#endif  /* CONFIG_PNFSD */
> diff --git a/fs/nfsd/nfsd_priv.h b/fs/nfsd/nfsd_priv.h
> new file mode 100644
> index 0000000..814fe8f
> --- /dev/null
> +++ b/fs/nfsd/nfsd_priv.h
> @@ -0,0 +1,105 @@
> +/*
> + *  nfsd_priv.h
> + *
> + *  nfsd declarations shared by a few .c files
> + *
> + *  Copyright (c) 2002 The Regents of the University of Michigan.
> + *  All rights reserved.
> + *
> + *  Redistribution and use in source and binary forms, with or without
> + *  modification, are permitted provided that the following conditions
> + *  are met:
> + *
> + *  1. Redistributions of source code must retain the above copyright
> + *     notice, this list of conditions and the following disclaimer.
> + *  2. Redistributions in binary form must reproduce the above copyright
> + *     notice, this list of conditions and the following disclaimer in the
> + *     documentation and/or other materials provided with the distribution.
> + *  3. Neither the name of the University nor the names of its
> + *     contributors may be used to endorse or promote products derived
> + *     from this software without specific prior written permission.
> + *
> + *  THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + *  WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + *  MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> + *  DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
> + *  FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + *  CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + *  SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> + *  BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
> + *  LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
> + *  NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
> + *  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + */
> +
> +#ifndef _NFSD_PRIV_H_
> +#define _NFSD_PRIV_H_
> +
> +/* 
> + * SETCLIENTID state 
> + */
> +
> +/* Hash tables for nfs4_clientid state */
> +#define CLIENT_HASH_BITS                 4
> +#define CLIENT_HASH_SIZE                (1 << CLIENT_HASH_BITS)
> +#define CLIENT_HASH_MASK                (CLIENT_HASH_SIZE - 1)
> +
> +#define clientid_hashval(id) \
> +	((id) & CLIENT_HASH_MASK)
> +#define clientstr_hashval(name) \
> +	(opaque_hashval((name), 8) & CLIENT_HASH_MASK)
> +
> +extern struct list_head	conf_str_hashtbl[CLIENT_HASH_SIZE];
> +
> +#if defined(CONFIG_PNFSD)
> +
> +#include <linux/nfsd/nfsd4_pnfs.h>
> +
> +#if defined(CONFIG_PNFSD_LOCAL_EXPORT)
> +extern struct sockaddr pnfsd_lexp_addr;
> +extern size_t pnfs_lexp_addr_len;
> +
> +void pnfsd_lexp_init(struct inode *inode);
> +#endif /* CONFIG_PNFSD_LOCAL_EXPORT */
> +
> +extern int nfsd4_init_pnfs_slabs(void);
> +extern void nfsd4_free_pnfs_slabs(void);
> +extern void release_pnfs_ds_dev_list(struct nfs4_stateid *stp);
> +extern void pnfs_expire_client(struct nfs4_client *clp);
> +
> +extern void release_pnfs_ds_dev_list(struct nfs4_stateid *stp);
> +extern void nfs4_pnfs_state_init(void);
> +extern int put_layoutrecall(struct nfs4_layoutrecall *);
> +extern void nomatching_layout(struct nfs4_layoutrecall *);
> +extern void *layoutrecall_done(struct nfs4_layoutrecall *);
> +extern int nfsd4_cb_layout(struct nfs4_layoutrecall *lp);
> +extern void nfsd4_free_slab(struct kmem_cache **slab);
> +extern struct nfs4_file * find_file(struct inode *ino);
> +extern struct nfs4_file * alloc_init_file(struct inode *ino, struct svc_fh *current_fh);
> +extern void put_nfs4_file(struct nfs4_file *fi);
> +extern void get_nfs4_file(struct nfs4_file *fi);
> +extern struct nfs4_client * find_confirmed_client(clientid_t *clid);
> +extern void nfs4_bug_on_unlocked_state(void);
> +extern void nfs4_ds_get_verifier(stateid_t *stateid,
> +		struct super_block *sb, u32 *p);
> +extern int nfs4_preprocess_pnfs_ds_stateid(struct svc_fh *, stateid_t *);
> +extern void nfs4_pnfs_state_shutdown(void);
> +extern int lo_recall_per_client(struct nfs4_client *clp,
> +		struct nfsd4_pnfs_cb_layout *cbl, struct nfs4_file *lrfile,
> +		struct list_head *todolist);
> +extern int create_layout_recall_list(struct list_head *todolist,
> +		unsigned *todo_len, struct nfsd4_pnfs_cb_layout *cbl,
> +		struct nfs4_file *lrfile);
> +extern int nfsd4_cb_notify_device(struct nfs4_notify_device *cbnd);
> +extern void pnfs_clear_device_notify(struct nfs4_client *clp);
> +extern int create_device_notify_list(struct list_head *todolist,
> +		struct nfsd4_pnfs_cb_dev_list *ndl);
> +#else /* CONFIG_PNFSD */
> +static inline void nfsd4_free_pnfs_slabs(void) {}
> +static inline int nfsd4_init_pnfs_slabs(void) { return 0; }
> +static inline void release_pnfs_ds_dev_list(struct nfs4_stateid *stp) {}
> +static inline void pnfs_expire_client(struct nfs4_client *clp) {}
> +#endif /* CONFIG_PNFSD */
> +
> +#endif /* _NFSD_PRIV_H_ */
> diff --git a/include/linux/nfsd/nfsd4_pnfs.h b/include/linux/nfsd/nfsd4_pnfs.h
> index 410c459..70ca179 100644
> --- a/include/linux/nfsd/nfsd4_pnfs.h
> +++ b/include/linux/nfsd/nfsd4_pnfs.h
> @@ -36,10 +36,9 @@
>  #ifndef _LINUX_NFSD_NFSD4_PNFS_H
>  #define _LINUX_NFSD_NFSD4_PNFS_H
>  
> +#include <linux/nfsd/state.h>
>  #include <linux/nfs_xdr.h>
>  #include <linux/exportfs.h>
> -#include <linux/exp_xdr.h>
> -#include <linux/nfsd/nfsfh.h>
>  
>  typedef struct {
>  	uint64_t	pnfs_fsid;	/* fsid */
> @@ -83,6 +82,55 @@ struct nfsd4_layout_seg {
>  	u64	length;
>  };
>  
> +/* outstanding layout stateid */
> +struct nfs4_layout_state {
> +	struct list_head	ls_perfile;
> +	struct list_head	ls_layouts; /* list of nfs4_layouts */
> +	struct kref		ls_ref;
> +	struct nfs4_client	*ls_client;
> +	struct nfs4_file	*ls_file;
> +	stateid_t		ls_stateid;
> +};
> +
> +/* outstanding layout */
> +struct nfs4_layout {
> +	struct list_head		lo_perfile;	/* hash by f_id */
> +	struct list_head		lo_perclnt;	/* hash by clientid */
> +	struct list_head		lo_perstate;
> +	struct nfs4_file		*lo_file;	/* backpointer */
> +	struct nfs4_client		*lo_client;
> +	struct nfs4_layout_state	*lo_state;
> +	struct nfsd4_layout_seg 	lo_seg;
> +};
> +
> +struct nfsd4_pnfs_cb_layout {
> +	u32			cbl_recall_type;	/* request */
> +	struct nfsd4_layout_seg cbl_seg;		/* request */
> +	u32			cbl_layoutchanged;	/* request */
> +	stateid_t		cbl_sid;		/* request */
> +	struct nfs4_fsid	cbl_fsid;
> +	void			*cbl_cookie;		/* fs private */
> +};
> +
> +/* layoutrecall request (from exported filesystem) */
> +struct nfs4_layoutrecall {
> +	struct kref			clr_ref;
> +	struct nfsd4_pnfs_cb_layout	cb;	/* request */
> +	struct list_head		clr_perclnt; /* on cl_layoutrecalls */
> +	struct nfs4_client	       *clr_client;
> +	struct nfs4_file	       *clr_file;
> +	struct timespec			clr_time;	/* last activity */
> +	struct super_block 		*clr_sb; /* We might not have a file */
> +	struct nfs4_layoutrecall	*parent; /* The initiating recall */
> +};
> +
> +/* notify device request (from exported filesystem) */
> +struct nfs4_notify_device {
> +	struct nfsd4_pnfs_cb_dev_list  *nd_list;
> +	struct nfs4_client	       *nd_client;
> +	struct list_head	        nd_perclnt;
> +};
> +
>  /* Used by layout_get to encode layout (loc_body var in spec)
>   * Args:
>   * minlength - min number of accessible bytes given by layout
> @@ -168,15 +216,6 @@ struct nfsd4_pnfs_open {
>  	int			op_truncate;
>  };
>  
> -struct nfsd4_pnfs_cb_layout {
> -	u32			cbl_recall_type;	/* request */
> -	struct nfsd4_layout_seg cbl_seg;		/* request */
> -	u32			cbl_layoutchanged;	/* request */
> -	stateid_t		cbl_sid;		/* request */
> -	struct nfs4_fsid	cbl_fsid;
> -	void			*cbl_cookie;		/* fs private */
> -};
> -
>  struct nfsd4_pnfs_cb_dev_item {
>  	u32			cbd_notify_type;	/* request */
>  	u32			cbd_layout_type;	/* request */
> diff --git a/include/linux/nfsd/pnfsd.h b/include/linux/nfsd/pnfsd.h
> index d1d7395..6aa3a87 100644
> --- a/include/linux/nfsd/pnfsd.h
> +++ b/include/linux/nfsd/pnfsd.h
> @@ -96,14 +96,6 @@ int nfs4_pnfs_return_layout(struct super_block *, struct svc_fh *,
>  					struct nfsd4_pnfs_layoutreturn *);
>  void pnfs_set_device_notify(clientid_t *clid, unsigned int types);
>  void nfs4_pnfs_state_shutdown(void);
> -
> -#if defined(CONFIG_PNFSD_LOCAL_EXPORT)
> -extern struct sockaddr pnfsd_lexp_addr;
> -extern size_t pnfs_lexp_addr_len;
> -
> -void pnfsd_lexp_init(struct inode *inode);
> -#endif /* CONFIG_PNFSD_LOCAL_EXPORT */
> -
>  #endif /* CONFIG_PNFSD */
>  
>  #endif /* LINUX_NFSD_PNFSD_H */
> diff --git a/include/linux/nfsd/state.h b/include/linux/nfsd/state.h
> index ffd5a41..7367696 100644
> --- a/include/linux/nfsd/state.h
> +++ b/include/linux/nfsd/state.h
> @@ -40,6 +40,7 @@
>  #include <linux/list.h>
>  #include <linux/kref.h>
>  #include <linux/sunrpc/clnt.h>
> +#include <linux/nfsd/nfsfh.h>
>  
>  typedef struct {
>  	u32             cl_boot;
> @@ -245,52 +246,6 @@ struct nfs4_fsid {
>  	u64	minor;
>  };
>  
> -#if defined(CONFIG_PNFSD)
> -
> -#include <linux/nfsd/nfsd4_pnfs.h>
> -
> -/* outstanding layout stateid */
> -struct nfs4_layout_state {
> -	struct list_head	ls_perfile;
> -	struct list_head	ls_layouts; /* list of nfs4_layouts */
> -	struct kref		ls_ref;
> -	struct nfs4_client	*ls_client;
> -	struct nfs4_file	*ls_file;
> -	stateid_t		ls_stateid;
> -};
> -
> -/* outstanding layout */
> -struct nfs4_layout {
> -	struct list_head		lo_perfile;	/* hash by f_id */
> -	struct list_head		lo_perclnt;	/* hash by clientid */
> -	struct list_head		lo_perstate;
> -	struct nfs4_file		*lo_file;	/* backpointer */
> -	struct nfs4_client		*lo_client;
> -	struct nfs4_layout_state	*lo_state;
> -	struct nfsd4_layout_seg 	lo_seg;
> -};
> -
> -/* layoutrecall request (from exported filesystem) */
> -struct nfs4_layoutrecall {
> -	struct kref			clr_ref;
> -	struct nfsd4_pnfs_cb_layout	cb;	/* request */
> -	struct list_head		clr_perclnt; /* on cl_layoutrecalls */
> -	struct nfs4_client	       *clr_client;
> -	struct nfs4_file	       *clr_file;
> -	struct timespec			clr_time;	/* last activity */
> -	struct super_block 		*clr_sb; /* We might not have a file */
> -	struct nfs4_layoutrecall	*parent; /* The initiating recall */
> -};
> -
> -/* notify device request (from exported filesystem) */
> -struct nfs4_notify_device {
> -	struct nfsd4_pnfs_cb_dev_list  *nd_list;
> -	struct nfs4_client	       *nd_client;
> -	struct list_head	        nd_perclnt;
> -};
> -
> -#endif /* CONFIG_PNFSD */
> -
>  /* struct nfs4_client_reset
>   * one per old client. Populates reset_str_hashtbl. Filled from conf_id_hashtbl
>   * upon lease reset, or from upcall to state_daemon (to read in state
> @@ -479,44 +434,6 @@ extern struct nfs4_delegation * find_delegation_stateid(struct inode *ino,
>  		stateid_t *stid);
>  extern __be32 nfs4_check_stateid(stateid_t *stateid);
>  extern void expire_client_lock(struct nfs4_client *clp);
> -#if defined(CONFIG_PNFSD)
> -extern void release_pnfs_ds_dev_list(struct nfs4_stateid *stp);
> -extern void nfs4_pnfs_state_init(void);
> -extern int put_layoutrecall(struct nfs4_layoutrecall *);
> -extern void nomatching_layout(struct nfs4_layoutrecall *);
> -extern void *layoutrecall_done(struct nfs4_layoutrecall *);
> -extern int nfsd4_cb_layout(struct nfs4_layoutrecall *lp);
> -extern void nfsd4_free_pnfs_slabs(void);
> -extern void nfsd4_free_slab(struct kmem_cache **slab);
> -extern int nfsd4_init_pnfs_slabs(void);
> -extern struct nfs4_file * find_file(struct inode *ino);
> -extern struct nfs4_file * alloc_init_file(struct inode *ino, struct svc_fh *current_fh);
> -extern void put_nfs4_file(struct nfs4_file *fi);
> -extern void get_nfs4_file(struct nfs4_file *fi);
> -extern struct nfs4_client * find_confirmed_client(clientid_t *clid);
> -extern void nfs4_bug_on_unlocked_state(void);
> -extern void release_pnfs_ds_dev_list(struct nfs4_stateid *stp);
> -extern void nfs4_ds_get_verifier(stateid_t *stateid,
> -		struct super_block *sb, u32 *p);
> -extern int nfs4_preprocess_pnfs_ds_stateid(struct svc_fh *, stateid_t *);
> -extern void nfs4_pnfs_state_shutdown(void);
> -extern int lo_recall_per_client(struct nfs4_client *clp,
> -		struct nfsd4_pnfs_cb_layout *cbl, struct nfs4_file *lrfile,
> -		struct list_head *todolist);
> -extern int create_layout_recall_list(struct list_head *todolist,
> -		unsigned *todo_len, struct nfsd4_pnfs_cb_layout *cbl,
> -		struct nfs4_file *lrfile);
> -extern int nfsd4_cb_notify_device(struct nfs4_notify_device *cbnd);
> -extern void pnfs_clear_device_notify(struct nfs4_client *clp);
> -extern int create_device_notify_list(struct list_head *todolist,
> -		struct nfsd4_pnfs_cb_dev_list *ndl);
> -extern void pnfs_expire_client(struct nfs4_client *clp);
> -#else /* CONFIG_PNFSD */
> -static inline void nfsd4_free_pnfs_slabs(void) {}
> -static inline int nfsd4_init_pnfs_slabs(void) { return 0; }
> -static inline void release_pnfs_ds_dev_list(struct nfs4_stateid *stp) {}
> -static inline void pnfs_expire_client(struct nfs4_client *clp) {}
> -#endif /* CONFIG_PNFSD */
>  
>  static inline void
>  nfs4_put_stateowner(struct nfs4_stateowner *so)

Re: [PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[J. Bruce Fields]

On Thu, Oct 22, 2009 at 05:59:33PM +0200, Boaz Harrosh wrote:
> On 10/22/2009 04:02 PM, Trond Myklebust wrote:
> > No. What I'm saying is that this doesn't have to be an absolute rule.
> > The Kernel style guide assumes that everything in 'include/*' is going
> > to be exported all around the kernel.
> > The problem is that we put a lot of stuff which is private to fs/nfs and
> > fs/nfsd in there. Those header files do not have to absolutely follow
> > the style guide rule, 'cos we know what is being included before and
> > after them...
> > 
> 
> I'm not sure I understand
> You are saying that the patches are very good, but only
> the rule I stated originally could be relaxed a little with private
> headers where we might get lazy, if the effects are very local?
> 
> Well, that's not a problem then, right? just that I can relax a bit
> if I want.
> 
> But I disagree: see 3, 4, 5 above and that last patch I submitted. That patch
> is only the beginning. 85% of all source files in nfs/nfsd could receive the
> same love. I only done these I touched. Code tends to stay much-much longer
> then we spend time on it. Better get it in shape the first time.

I'm assuming Trond's objection is just to the patch changelog
(specifically, to the statement that any header "should be compilation
independent"), not to these specific changes.

--b.

Re: [PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[Boaz Harrosh]

On 11/05/2009 12:09 AM, J. Bruce Fields wrote:
> On Thu, Oct 22, 2009 at 05:59:33PM +0200, Boaz Harrosh wrote:
>> On 10/22/2009 04:02 PM, Trond Myklebust wrote:
>>> No. What I'm saying is that this doesn't have to be an absolute rule.
>>> The Kernel style guide assumes that everything in 'include/*' is going
>>> to be exported all around the kernel.
>>> The problem is that we put a lot of stuff which is private to fs/nfs and
>>> fs/nfsd in there. Those header files do not have to absolutely follow
>>> the style guide rule, 'cos we know what is being included before and
>>> after them...
>>>
>>
>> I'm not sure I understand
>> You are saying that the patches are very good, but only
>> the rule I stated originally could be relaxed a little with private
>> headers where we might get lazy, if the effects are very local?
>>
>> Well, that's not a problem then, right? just that I can relax a bit
>> if I want.
>>
>> But I disagree: see 3, 4, 5 above and that last patch I submitted. That patch
>> is only the beginning. 85% of all source files in nfs/nfsd could receive the
>> same love. I only done these I touched. Code tends to stay much-much longer
>> then we spend time on it. Better get it in shape the first time.
> 
> I'm assuming Trond's objection is just to the patch changelog
> (specifically, to the statement that any header "should be compilation
> independent"), not to these specific changes.
> 
> --b.

Speaking of which, Bruce I have a question.

There are a few files in include/linux/ that define xdr definitions
these are used by exportfs nfs and nfsd. Some of it gets exposed
to filesystems. With the pNFS tree we are adding lots more of these,
specifically I'm now to move the pnfs_osd_xdr stuff as well, and blocks.

Could I open up a new  include/linux/exportfs/ folder and put there any thing
xdr and exportfs related?

What should be the scope of the move, should any include/linux/ common
files used both by nfs & nfsd be moved there?

Thanks
Boaz

Re: [PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[J. Bruce Fields]

On Thu, Nov 05, 2009 at 10:48:15AM +0200, Boaz Harrosh wrote:
> On 11/05/2009 12:09 AM, J. Bruce Fields wrote:
> > On Thu, Oct 22, 2009 at 05:59:33PM +0200, Boaz Harrosh wrote:
> >> On 10/22/2009 04:02 PM, Trond Myklebust wrote:
> >>> No. What I'm saying is that this doesn't have to be an absolute rule.
> >>> The Kernel style guide assumes that everything in 'include/*' is going
> >>> to be exported all around the kernel.
> >>> The problem is that we put a lot of stuff which is private to fs/nfs and
> >>> fs/nfsd in there. Those header files do not have to absolutely follow
> >>> the style guide rule, 'cos we know what is being included before and
> >>> after them...
> >>>
> >>
> >> I'm not sure I understand
> >> You are saying that the patches are very good, but only
> >> the rule I stated originally could be relaxed a little with private
> >> headers where we might get lazy, if the effects are very local?
> >>
> >> Well, that's not a problem then, right? just that I can relax a bit
> >> if I want.
> >>
> >> But I disagree: see 3, 4, 5 above and that last patch I submitted. That patch
> >> is only the beginning. 85% of all source files in nfs/nfsd could receive the
> >> same love. I only done these I touched. Code tends to stay much-much longer
> >> then we spend time on it. Better get it in shape the first time.
> > 
> > I'm assuming Trond's objection is just to the patch changelog
> > (specifically, to the statement that any header "should be compilation
> > independent"), not to these specific changes.
> > 
> > --b.
> 
> Speaking of which, Bruce I have a question.
> 
> There are a few files in include/linux/ that define xdr definitions
> these are used by exportfs nfs and nfsd. Some of it gets exposed
> to filesystems.

Which specifically?

> With the pNFS tree we are adding lots more of these,
> specifically I'm now to move the pnfs_osd_xdr stuff as well, and blocks.
> 
> Could I open up a new  include/linux/exportfs/ folder and put there any thing
> xdr and exportfs related?

Well, if it's just a few things, maybe include/linux/exportfs.h?

But, sure, makes sense if there's includes that are specific to pNFS and
that wouldn't be needed by any filesystem that included exportfs.h.

> What should be the scope of the move, should any include/linux/ common
> files used both by nfs & nfsd be moved there?

I don't see why we'd want to do that, but maybe I'm missing something.

--b.

Re: [PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[J. Bruce Fields]

By the way, along the lines of your .h moves, while looking at steved's
export stuff I was reminded again of how much in include/linux/nfsd is
really internal to nfsd, so I'm thinking of committing this.

--b.

>From 0142013a3f49ca296f8880829c67f2afd85b4bfe Mon Sep 17 00:00:00 2001
From: J. Bruce Fields <bfields@citi.umich.edu>
Date: Wed, 4 Nov 2009 18:12:35 -0500
Subject: [PATCH] nfsd: make nfsd/vfs.h for common includes

None of this stuff is used outside nfsd, so move it out of the common
linux include directory.

Actually, probably none of the stuff in include/linux/nfsd/nfsd.h really
belongs there, so later we may remove that file entirely.

Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 fs/nfsd/lockd.c           |    1 +
 fs/nfsd/nfs2acl.c         |    1 +
 fs/nfsd/nfs3acl.c         |    1 +
 fs/nfsd/nfs3proc.c        |    1 +
 fs/nfsd/nfs4proc.c        |    1 +
 fs/nfsd/nfs4recover.c     |    1 +
 fs/nfsd/nfs4state.c       |    1 +
 fs/nfsd/nfs4xdr.c         |    1 +
 fs/nfsd/nfsfh.c           |    1 +
 fs/nfsd/nfsproc.c         |    1 +
 fs/nfsd/nfssvc.c          |    1 +
 fs/nfsd/vfs.c             |    1 +
 fs/nfsd/vfs.h             |   98 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/nfsd/nfsd.h |   87 +---------------------------------------
 14 files changed, 111 insertions(+), 86 deletions(-)
 create mode 100644 fs/nfsd/vfs.h

diff --git a/fs/nfsd/lockd.c b/fs/nfsd/lockd.c
index b2786a5..812bc64 100644
--- a/fs/nfsd/lockd.c
+++ b/fs/nfsd/lockd.c
@@ -16,6 +16,7 @@
 #include <linux/sunrpc/svc.h>
 #include <linux/nfsd/nfsd.h>
 #include <linux/lockd/bind.h>
+#include "vfs.h"
 
 #define NFSDDBG_FACILITY		NFSDDBG_LOCKD
 
diff --git a/fs/nfsd/nfs2acl.c b/fs/nfsd/nfs2acl.c
index 4e3219e..5e8ef72 100644
--- a/fs/nfsd/nfs2acl.c
+++ b/fs/nfsd/nfs2acl.c
@@ -14,6 +14,7 @@
 #include <linux/nfsd/xdr3.h>
 #include <linux/posix_acl.h>
 #include <linux/nfsacl.h>
+#include "vfs.h"
 
 #define NFSDDBG_FACILITY		NFSDDBG_PROC
 #define RETURN_STATUS(st)	{ resp->status = (st); return (st); }
diff --git a/fs/nfsd/nfs3acl.c b/fs/nfsd/nfs3acl.c
index 9981dbb..354bf0f 100644
--- a/fs/nfsd/nfs3acl.c
+++ b/fs/nfsd/nfs3acl.c
@@ -13,6 +13,7 @@
 #include <linux/nfsd/xdr3.h>
 #include <linux/posix_acl.h>
 #include <linux/nfsacl.h>
+#include "vfs.h"
 
 #define RETURN_STATUS(st)	{ resp->status = (st); return (st); }
 
diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index a713c41..1a259d3 100644
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -25,6 +25,7 @@
 #include <linux/nfsd/cache.h>
 #include <linux/nfsd/xdr3.h>
 #include <linux/nfs3.h>
+#include "vfs.h"
 
 #define NFSDDBG_FACILITY		NFSDDBG_PROC
 
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index bebc0c2..60a93cd 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -48,6 +48,7 @@
 #include <linux/nfsd/xdr4.h>
 #include <linux/nfs4_acl.h>
 #include <linux/sunrpc/gss_api.h>
+#include "vfs.h"
 
 #define NFSDDBG_FACILITY		NFSDDBG_PROC
 
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index b534840..c7a6b24 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -47,6 +47,7 @@
 #include <linux/crypto.h>
 #include <linux/sched.h>
 #include <linux/mount.h>
+#include "vfs.h"
 
 #define NFSDDBG_FACILITY                NFSDDBG_PROC
 
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index fcb9817..d3fad54 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -56,6 +56,7 @@
 #include <linux/module.h>
 #include <linux/sunrpc/svcauth_gss.h>
 #include <linux/sunrpc/clnt.h>
+#include "vfs.h"
 
 #define NFSDDBG_FACILITY                NFSDDBG_PROC
 
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 0fbd50c..db0fc55 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -57,6 +57,7 @@
 #include <linux/nfs4_acl.h>
 #include <linux/sunrpc/gss_api.h>
 #include <linux/sunrpc/svcauth_gss.h>
+#include "vfs.h"
 
 #define NFSDDBG_FACILITY		NFSDDBG_XDR
 
diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index 01965b2..d0d8a21 100644
--- a/fs/nfsd/nfsfh.c
+++ b/fs/nfsd/nfsfh.c
@@ -22,6 +22,7 @@
 #include <linux/sunrpc/svc.h>
 #include <linux/sunrpc/svcauth_gss.h>
 #include <linux/nfsd/nfsd.h>
+#include "vfs.h"
 #include "auth.h"
 
 #define NFSDDBG_FACILITY		NFSDDBG_FH
diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
index c5393d1..6c967e1 100644
--- a/fs/nfsd/nfsproc.c
+++ b/fs/nfsd/nfsproc.c
@@ -24,6 +24,7 @@
 #include <linux/nfsd/nfsd.h>
 #include <linux/nfsd/cache.h>
 #include <linux/nfsd/xdr.h>
+#include "vfs.h"
 
 typedef struct svc_rqst	svc_rqst;
 typedef struct svc_buf	svc_buf;
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 67ea83e..2944b31 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -35,6 +35,7 @@
 #include <linux/lockd/bind.h>
 #include <linux/nfsacl.h>
 #include <linux/seq_file.h>
+#include "vfs.h"
 
 #define NFSDDBG_FACILITY	NFSDDBG_SVC
 
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 6385739..a7038ed 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -56,6 +56,7 @@
 #endif /* CONFIG_NFSD_V4 */
 #include <linux/jhash.h>
 #include <linux/ima.h>
+#include "vfs.h"
 
 #include <asm/uaccess.h>
 
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
new file mode 100644
index 0000000..b8011fd
--- /dev/null
+++ b/fs/nfsd/vfs.h
@@ -0,0 +1,98 @@
+/*
+ * Copyright (C) 1995-1997 Olaf Kirch <okir-pn4DOG8n3UYbFoVRYvo4fw@public.gmane.org>
+ */
+
+#ifndef LINUX_NFSD_VFS_H
+#define LINUX_NFSD_VFS_H
+
+/*
+ * Flags for nfsd_permission
+ */
+#define NFSD_MAY_NOP		0
+#define NFSD_MAY_EXEC		1 /* == MAY_EXEC */
+#define NFSD_MAY_WRITE		2 /* == MAY_WRITE */
+#define NFSD_MAY_READ		4 /* == MAY_READ */
+#define NFSD_MAY_SATTR		8
+#define NFSD_MAY_TRUNC		16
+#define NFSD_MAY_LOCK		32
+#define NFSD_MAY_OWNER_OVERRIDE	64
+#define NFSD_MAY_LOCAL_ACCESS	128 /* IRIX doing local access check on device special file*/
+#define NFSD_MAY_BYPASS_GSS_ON_ROOT 256
+
+#define NFSD_MAY_CREATE		(NFSD_MAY_EXEC|NFSD_MAY_WRITE)
+#define NFSD_MAY_REMOVE		(NFSD_MAY_EXEC|NFSD_MAY_WRITE|NFSD_MAY_TRUNC)
+
+/*
+ * Callback function for readdir
+ */
+typedef int (*nfsd_dirop_t)(struct inode *, struct dentry *, int, int);
+
+/* nfsd/vfs.c */
+int		fh_lock_parent(struct svc_fh *, struct dentry *);
+int		nfsd_racache_init(int);
+void		nfsd_racache_shutdown(void);
+int		nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp,
+		                struct svc_export **expp);
+__be32		nfsd_lookup(struct svc_rqst *, struct svc_fh *,
+				const char *, unsigned int, struct svc_fh *);
+__be32		 nfsd_lookup_dentry(struct svc_rqst *, struct svc_fh *,
+				const char *, unsigned int,
+				struct svc_export **, struct dentry **);
+__be32		nfsd_setattr(struct svc_rqst *, struct svc_fh *,
+				struct iattr *, int, time_t);
+#ifdef CONFIG_NFSD_V4
+__be32          nfsd4_set_nfs4_acl(struct svc_rqst *, struct svc_fh *,
+                    struct nfs4_acl *);
+int             nfsd4_get_nfs4_acl(struct svc_rqst *, struct dentry *, struct nfs4_acl **);
+#endif /* CONFIG_NFSD_V4 */
+__be32		nfsd_create(struct svc_rqst *, struct svc_fh *,
+				char *name, int len, struct iattr *attrs,
+				int type, dev_t rdev, struct svc_fh *res);
+#ifdef CONFIG_NFSD_V3
+__be32		nfsd_access(struct svc_rqst *, struct svc_fh *, u32 *, u32 *);
+__be32		nfsd_create_v3(struct svc_rqst *, struct svc_fh *,
+				char *name, int len, struct iattr *attrs,
+				struct svc_fh *res, int createmode,
+				u32 *verifier, int *truncp, int *created);
+__be32		nfsd_commit(struct svc_rqst *, struct svc_fh *,
+				loff_t, unsigned long);
+#endif /* CONFIG_NFSD_V3 */
+__be32		nfsd_open(struct svc_rqst *, struct svc_fh *, int,
+				int, struct file **);
+void		nfsd_close(struct file *);
+__be32 		nfsd_read(struct svc_rqst *, struct svc_fh *, struct file *,
+				loff_t, struct kvec *, int, unsigned long *);
+__be32 		nfsd_write(struct svc_rqst *, struct svc_fh *,struct file *,
+				loff_t, struct kvec *,int, unsigned long *, int *);
+__be32		nfsd_readlink(struct svc_rqst *, struct svc_fh *,
+				char *, int *);
+__be32		nfsd_symlink(struct svc_rqst *, struct svc_fh *,
+				char *name, int len, char *path, int plen,
+				struct svc_fh *res, struct iattr *);
+__be32		nfsd_link(struct svc_rqst *, struct svc_fh *,
+				char *, int, struct svc_fh *);
+__be32		nfsd_rename(struct svc_rqst *,
+				struct svc_fh *, char *, int,
+				struct svc_fh *, char *, int);
+__be32		nfsd_remove(struct svc_rqst *,
+				struct svc_fh *, char *, int);
+__be32		nfsd_unlink(struct svc_rqst *, struct svc_fh *, int type,
+				char *name, int len);
+int		nfsd_truncate(struct svc_rqst *, struct svc_fh *,
+				unsigned long size);
+__be32		nfsd_readdir(struct svc_rqst *, struct svc_fh *,
+			     loff_t *, struct readdir_cd *, filldir_t);
+__be32		nfsd_statfs(struct svc_rqst *, struct svc_fh *,
+				struct kstatfs *, int access);
+
+int		nfsd_notify_change(struct inode *, struct iattr *);
+__be32		nfsd_permission(struct svc_rqst *, struct svc_export *,
+				struct dentry *, int);
+int		nfsd_sync_dir(struct dentry *dp);
+
+#if defined(CONFIG_NFSD_V2_ACL) || defined(CONFIG_NFSD_V3_ACL)
+struct posix_acl *nfsd_get_posix_acl(struct svc_fh *, int);
+int nfsd_set_posix_acl(struct svc_fh *, int, struct posix_acl *);
+#endif
+
+#endif /* LINUX_NFSD_VFS_H */
diff --git a/include/linux/nfsd/nfsd.h b/include/linux/nfsd/nfsd.h
index 510ffdd..e4518d0 100644
--- a/include/linux/nfsd/nfsd.h
+++ b/include/linux/nfsd/nfsd.h
@@ -25,30 +25,10 @@
  */
 #define NFSD_SUPPORTED_MINOR_VERSION	1
 
-/*
- * Flags for nfsd_permission
- */
-#define NFSD_MAY_NOP		0
-#define NFSD_MAY_EXEC		1 /* == MAY_EXEC */
-#define NFSD_MAY_WRITE		2 /* == MAY_WRITE */
-#define NFSD_MAY_READ		4 /* == MAY_READ */
-#define NFSD_MAY_SATTR		8
-#define NFSD_MAY_TRUNC		16
-#define NFSD_MAY_LOCK		32
-#define NFSD_MAY_OWNER_OVERRIDE	64
-#define NFSD_MAY_LOCAL_ACCESS	128 /* IRIX doing local access check on device special file*/
-#define NFSD_MAY_BYPASS_GSS_ON_ROOT 256
-
-#define NFSD_MAY_CREATE		(NFSD_MAY_EXEC|NFSD_MAY_WRITE)
-#define NFSD_MAY_REMOVE		(NFSD_MAY_EXEC|NFSD_MAY_WRITE|NFSD_MAY_TRUNC)
-
-/*
- * Callback function for readdir
- */
 struct readdir_cd {
 	__be32			err;	/* 0, nfserr, or nfserr_eof */
 };
-typedef int (*nfsd_dirop_t)(struct inode *, struct dentry *, int, int);
+
 
 extern struct svc_program	nfsd_program;
 extern struct svc_version	nfsd_version2, nfsd_version3,
@@ -73,69 +53,6 @@ int		nfsd_nrpools(void);
 int		nfsd_get_nrthreads(int n, int *);
 int		nfsd_set_nrthreads(int n, int *);
 
-/* nfsd/vfs.c */
-int		fh_lock_parent(struct svc_fh *, struct dentry *);
-int		nfsd_racache_init(int);
-void		nfsd_racache_shutdown(void);
-int		nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp,
-		                struct svc_export **expp);
-__be32		nfsd_lookup(struct svc_rqst *, struct svc_fh *,
-				const char *, unsigned int, struct svc_fh *);
-__be32		 nfsd_lookup_dentry(struct svc_rqst *, struct svc_fh *,
-				const char *, unsigned int,
-				struct svc_export **, struct dentry **);
-__be32		nfsd_setattr(struct svc_rqst *, struct svc_fh *,
-				struct iattr *, int, time_t);
-#ifdef CONFIG_NFSD_V4
-__be32          nfsd4_set_nfs4_acl(struct svc_rqst *, struct svc_fh *,
-                    struct nfs4_acl *);
-int             nfsd4_get_nfs4_acl(struct svc_rqst *, struct dentry *, struct nfs4_acl **);
-#endif /* CONFIG_NFSD_V4 */
-__be32		nfsd_create(struct svc_rqst *, struct svc_fh *,
-				char *name, int len, struct iattr *attrs,
-				int type, dev_t rdev, struct svc_fh *res);
-#ifdef CONFIG_NFSD_V3
-__be32		nfsd_access(struct svc_rqst *, struct svc_fh *, u32 *, u32 *);
-__be32		nfsd_create_v3(struct svc_rqst *, struct svc_fh *,
-				char *name, int len, struct iattr *attrs,
-				struct svc_fh *res, int createmode,
-				u32 *verifier, int *truncp, int *created);
-__be32		nfsd_commit(struct svc_rqst *, struct svc_fh *,
-				loff_t, unsigned long);
-#endif /* CONFIG_NFSD_V3 */
-__be32		nfsd_open(struct svc_rqst *, struct svc_fh *, int,
-				int, struct file **);
-void		nfsd_close(struct file *);
-__be32 		nfsd_read(struct svc_rqst *, struct svc_fh *, struct file *,
-				loff_t, struct kvec *, int, unsigned long *);
-__be32 		nfsd_write(struct svc_rqst *, struct svc_fh *,struct file *,
-				loff_t, struct kvec *,int, unsigned long *, int *);
-__be32		nfsd_readlink(struct svc_rqst *, struct svc_fh *,
-				char *, int *);
-__be32		nfsd_symlink(struct svc_rqst *, struct svc_fh *,
-				char *name, int len, char *path, int plen,
-				struct svc_fh *res, struct iattr *);
-__be32		nfsd_link(struct svc_rqst *, struct svc_fh *,
-				char *, int, struct svc_fh *);
-__be32		nfsd_rename(struct svc_rqst *,
-				struct svc_fh *, char *, int,
-				struct svc_fh *, char *, int);
-__be32		nfsd_remove(struct svc_rqst *,
-				struct svc_fh *, char *, int);
-__be32		nfsd_unlink(struct svc_rqst *, struct svc_fh *, int type,
-				char *name, int len);
-int		nfsd_truncate(struct svc_rqst *, struct svc_fh *,
-				unsigned long size);
-__be32		nfsd_readdir(struct svc_rqst *, struct svc_fh *,
-			     loff_t *, struct readdir_cd *, filldir_t);
-__be32		nfsd_statfs(struct svc_rqst *, struct svc_fh *,
-				struct kstatfs *, int access);
-
-int		nfsd_notify_change(struct inode *, struct iattr *);
-__be32		nfsd_permission(struct svc_rqst *, struct svc_export *,
-				struct dentry *, int);
-int		nfsd_sync_dir(struct dentry *dp);
-
 #if defined(CONFIG_NFSD_V2_ACL) || defined(CONFIG_NFSD_V3_ACL)
 #ifdef CONFIG_NFSD_V2_ACL
 extern struct svc_version nfsd_acl_version2;
@@ -147,8 +64,6 @@ extern struct svc_version nfsd_acl_version3;
 #else
 #define nfsd_acl_version3 NULL
 #endif
-struct posix_acl *nfsd_get_posix_acl(struct svc_fh *, int);
-int nfsd_set_posix_acl(struct svc_fh *, int, struct posix_acl *);
 #endif
 
 enum vers_op {NFSD_SET, NFSD_CLEAR, NFSD_TEST, NFSD_AVAIL };
-- 
1.6.3.3

Re: [PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[Trond Myklebust]

On Thu, 2009-11-05 at 11:41 -0500, J. Bruce Fields wrote:
> By the way, along the lines of your .h moves, while looking at steved's
> export stuff I was reminded again of how much in include/linux/nfsd is
> really internal to nfsd, so I'm thinking of committing this.

Right. Most of the NFS client include files should really be moved into
fs/nfs rather than into a new directory in include/linux.

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: [PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[J. Bruce Fields]

On Thu, Nov 05, 2009 at 08:59:02AM -0800, Trond Myklebust wrote:
> On Thu, 2009-11-05 at 11:41 -0500, J. Bruce Fields wrote:
> > By the way, along the lines of your .h moves, while looking at steved's
> > export stuff I was reminded again of how much in include/linux/nfsd is
> > really internal to nfsd, so I'm thinking of committing this.
> 
> Right. Most of the NFS client include files should really be moved into
> fs/nfs rather than into a new directory in include/linux.

On the server side my greatest source of confusion here is some includes
of nfsd headers in arch-specific code.  Was it needed for some sort of
compatibility glue for the nfsservctl system call, maybe?  I've never
figured it out.

--b.

Re: [PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[Trond Myklebust]

On Thu, 2009-11-05 at 12:03 -0500, J. Bruce Fields wrote:
> On Thu, Nov 05, 2009 at 08:59:02AM -0800, Trond Myklebust wrote:
> > On Thu, 2009-11-05 at 11:41 -0500, J. Bruce Fields wrote:
> > > By the way, along the lines of your .h moves, while looking at steved's
> > > export stuff I was reminded again of how much in include/linux/nfsd is
> > > really internal to nfsd, so I'm thinking of committing this.
> > 
> > Right. Most of the NFS client include files should really be moved into
> > fs/nfs rather than into a new directory in include/linux.
> 
> On the server side my greatest source of confusion here is some includes
> of nfsd headers in arch-specific code.  Was it needed for some sort of
> compatibility glue for the nfsservctl system call, maybe?  I've never
> figured it out.

Possibly for the 32-bit emulation code on 64-bit systems?

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: [PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[Boaz Harrosh]

On 11/05/2009 12:09 AM, J. Bruce Fields wrote:
> On Thu, Oct 22, 2009 at 05:59:33PM +0200, Boaz Harrosh wrote:
> 
> I'm assuming Trond's objection is just to the patch changelog
> (specifically, to the statement that any header "should be compilation
> independent"), not to these specific changes.
> 
> --b.

Ping

Bruce? Trond? whatsup?

Can Benny put these patches in his tree? He said he would be happy to hold
them for a while, but only if they will be eventually accepted into the
tree as a pnfs pre-requisite. Please ACK on these patches?

I have to make all these put-the-includes-back patches to just make the tree
compile.

Boaz

Re: [PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[J. Bruce Fields]

On Wed, Nov 11, 2009 at 04:57:46PM +0200, Boaz Harrosh wrote:
> On 11/05/2009 12:09 AM, J. Bruce Fields wrote:
> > On Thu, Oct 22, 2009 at 05:59:33PM +0200, Boaz Harrosh wrote:
> > 
> > I'm assuming Trond's objection is just to the patch changelog
> > (specifically, to the statement that any header "should be compilation
> > independent"), not to these specific changes.
> > 
> > --b.
> 
> Ping
> 
> Bruce? Trond? whatsup?
> 
> Can Benny put these patches in his tree? He said he would be happy to hold
> them for a while, but only if they will be eventually accepted into the
> tree as a pnfs pre-requisite. Please ACK on these patches?
> 
> I have to make all these put-the-includes-back patches to just make the tree
> compile.

They're fine by me.

(Can't speak for Trond, but maybe his initial objection would be met
just editing the changelog to replace the absolute "Any header should be
compilation independent" by the particular advantages you saw in this
case.)

--b.

> 
> Boaz
> 

Re: [PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[Boaz Harrosh]

On 11/11/2009 07:36 PM, J. Bruce Fields wrote:
> On Wed, Nov 11, 2009 at 04:57:46PM +0200, Boaz Harrosh wrote:
>> On 11/05/2009 12:09 AM, J. Bruce Fields wrote:
>>> On Thu, Oct 22, 2009 at 05:59:33PM +0200, Boaz Harrosh wrote:
>>>
>>> I'm assuming Trond's objection is just to the patch changelog
>>> (specifically, to the statement that any header "should be compilation
>>> independent"), not to these specific changes.
>>>
>>> --b.
>>
>> Ping
>>
>> Bruce? Trond? whatsup?
>>
>> Can Benny put these patches in his tree? He said he would be happy to hold
>> them for a while, but only if they will be eventually accepted into the
>> tree as a pnfs pre-requisite. Please ACK on these patches?
>>
>> I have to make all these put-the-includes-back patches to just make the tree
>> compile.
> 
> They're fine by me.
> 
> (Can't speak for Trond, but maybe his initial objection would be met
> just editing the changelog to replace the absolute "Any header should be
> compilation independent" by the particular advantages you saw in this
> case.)
> 

I don't see why. Please advise? 

"should be compilation independent", from what I understand of the English
language, is suggestive and advisory only. Now, if I was using "must" or
"shall" like the standard do then that would mean a mandatory directive.
But I'm only saying "should" which is like saying: "I suggest", or
"it is recommended". Am I misunderstanding the language?

Any way the commit log is just my saying so, my sign-off it's not the word
of Linux-god, is it?

> --b.
> 
>>
>> Boaz
>>

Thanks
Boaz

Re: [PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[J. Bruce Fields]

On Wed, Nov 11, 2009 at 07:59:21PM +0200, Boaz Harrosh wrote:
> On 11/11/2009 07:36 PM, J. Bruce Fields wrote:
> > On Wed, Nov 11, 2009 at 04:57:46PM +0200, Boaz Harrosh wrote:
> >> On 11/05/2009 12:09 AM, J. Bruce Fields wrote:
> >>> On Thu, Oct 22, 2009 at 05:59:33PM +0200, Boaz Harrosh wrote:
> >>>
> >>> I'm assuming Trond's objection is just to the patch changelog
> >>> (specifically, to the statement that any header "should be compilation
> >>> independent"), not to these specific changes.
> >>>
> >>> --b.
> >>
> >> Ping
> >>
> >> Bruce? Trond? whatsup?
> >>
> >> Can Benny put these patches in his tree? He said he would be happy to hold
> >> them for a while, but only if they will be eventually accepted into the
> >> tree as a pnfs pre-requisite. Please ACK on these patches?
> >>
> >> I have to make all these put-the-includes-back patches to just make the tree
> >> compile.
> > 
> > They're fine by me.
> > 
> > (Can't speak for Trond, but maybe his initial objection would be met
> > just editing the changelog to replace the absolute "Any header should be
> > compilation independent" by the particular advantages you saw in this
> > case.)
> > 
> 
> I don't see why. Please advise? 
> 
> "should be compilation independent", from what I understand of the English
> language, is suggestive and advisory only. Now, if I was using "must" or
> "shall" like the standard do then that would mean a mandatory directive.
> But I'm only saying "should" which is like saying: "I suggest", or
> "it is recommended". Am I misunderstanding the language?
> 
> Any way the commit log is just my saying so, my sign-off it's not the word
> of Linux-god, is it?

I really don't care.  Do what you think best.

--b.

Re: [pnfs] [PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[Boaz Harrosh]

On 10/21/2009 10:14 AM, Boaz Harrosh wrote:
> An header should be compilation independent, .i.e pull in
> any header who's declarations are directly used by this header.
> And not let users re-include all it's dependencies all over
> again.
> 
> [At the end of the day what's the use of a header if it does
>  not have more then one user?]
> 
> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>

Trond do I have an ACK on this patch.
If not, then what should be changed to get it accepted?

> ---
>  include/linux/nfs_xdr.h   |    1 +

This header is used exclusively by fs/nfs/... files and could just be moved
there. The include must be fixed as below though.

>  include/linux/nfsacl.h    |    1 +

This file is used mixed between fs/nfs && fs/nfsd

>  include/linux/posix_acl.h |    1 +

Used by nfsd and filesystems

>  3 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 2848a26..c316ca8 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -2,6 +2,7 @@
>  #define _LINUX_NFS_XDR_H
>  
>  #include <linux/nfsacl.h>
> +#include <linux/nfs3.h>
>  
>  /*
>   * To change the maximum rsize and wsize supported by the NFS client, adjust
> diff --git a/include/linux/nfsacl.h b/include/linux/nfsacl.h
> index 43011b6..f321b57 100644
> --- a/include/linux/nfsacl.h
> +++ b/include/linux/nfsacl.h
> @@ -29,6 +29,7 @@
>  #ifdef __KERNEL__
>  
>  #include <linux/posix_acl.h>
> +#include <linux/sunrpc/xdr.h>
>  
>  /* Maximum number of ACL entries over NFS */
>  #define NFS_ACL_MAX_ENTRIES	1024
> diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
> index 065a365..0dcf674 100644
> --- a/include/linux/posix_acl.h
> +++ b/include/linux/posix_acl.h
> @@ -9,6 +9,7 @@
>  #define __LINUX_POSIX_ACL_H
>  
>  #include <linux/slab.h>
> +#include <linux/fs.h>
>  
>  #define ACL_UNDEFINED_ID	(-1)
>  

Re: [pnfs] [PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[Trond Myklebust]

On Thu, 2009-11-12 at 12:28 +0200, Boaz Harrosh wrote:
> On 10/21/2009 10:14 AM, Boaz Harrosh wrote:
> > An header should be compilation independent, .i.e pull in
> > any header who's declarations are directly used by this header.
> > And not let users re-include all it's dependencies all over
> > again.
> > 
> > [At the end of the day what's the use of a header if it does
> >  not have more then one user?]
> > 
> > Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
> 
> Trond do I have an ACK on this patch.
> If not, then what should be changed to get it accepted?
> 
> > ---
> >  include/linux/nfs_xdr.h   |    1 +
> 
> This header is used exclusively by fs/nfs/... files and could just be moved
> there. The include must be fixed as below though.
> 
> >  include/linux/nfsacl.h    |    1 +
> 
> This file is used mixed between fs/nfs && fs/nfsd
> 
> >  include/linux/posix_acl.h |    1 +
> 
> Used by nfsd and filesystems
> 
> >  3 files changed, 3 insertions(+), 0 deletions(-)
> > 
> > diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> > index 2848a26..c316ca8 100644
> > --- a/include/linux/nfs_xdr.h
> > +++ b/include/linux/nfs_xdr.h
> > @@ -2,6 +2,7 @@
> >  #define _LINUX_NFS_XDR_H
> >  
> >  #include <linux/nfsacl.h>
> > +#include <linux/nfs3.h>
> >  
> >  /*
> >   * To change the maximum rsize and wsize supported by the NFS client, adjust
> > diff --git a/include/linux/nfsacl.h b/include/linux/nfsacl.h
> > index 43011b6..f321b57 100644
> > --- a/include/linux/nfsacl.h
> > +++ b/include/linux/nfsacl.h
> > @@ -29,6 +29,7 @@
> >  #ifdef __KERNEL__
> >  
> >  #include <linux/posix_acl.h>
> > +#include <linux/sunrpc/xdr.h>
> >  
> >  /* Maximum number of ACL entries over NFS */
> >  #define NFS_ACL_MAX_ENTRIES	1024
> > diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
> > index 065a365..0dcf674 100644
> > --- a/include/linux/posix_acl.h
> > +++ b/include/linux/posix_acl.h
> > @@ -9,6 +9,7 @@
> >  #define __LINUX_POSIX_ACL_H
> >  
> >  #include <linux/slab.h>
> > +#include <linux/fs.h>

NACK to this. Pretty much _all_ filesystems already include linux/fs.h
somewhere in their include chains. There should be no need to include it
in posix_acl.h too.

> >  
> >  #define ACL_UNDEFINED_ID	(-1)
> >  
> 

So, what is the motivation for all this? We have no dependency problems
here today. What is changing in the pNFS tree that makes this so
necessary?

Trond

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: [pnfs] [PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[Boaz Harrosh]

On 11/12/2009 12:35 PM, Trond Myklebust wrote:
> On Thu, 2009-11-12 at 12:28 +0200, Boaz Harrosh wrote:
>> On 10/21/2009 10:14 AM, Boaz Harrosh wrote:
>>> An header should be compilation independent, .i.e pull in
>>> any header who's declarations are directly used by this header.
>>> And not let users re-include all it's dependencies all over
>>> again.
>>>
>>> [At the end of the day what's the use of a header if it does
>>>  not have more then one user?]
>>>
>>> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
>>
>> Trond do I have an ACK on this patch.
>> If not, then what should be changed to get it accepted?
>>
>>> ---
>>>  include/linux/nfs_xdr.h   |    1 +
>>
>> This header is used exclusively by fs/nfs/... files and could just be moved
>> there. The include must be fixed as below though.
>>
>>>  include/linux/nfsacl.h    |    1 +
>>
>> This file is used mixed between fs/nfs && fs/nfsd
>>
>>>  include/linux/posix_acl.h |    1 +
>>
>> Used by nfsd and filesystems
>>
>>>  3 files changed, 3 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
>>> index 2848a26..c316ca8 100644
>>> --- a/include/linux/nfs_xdr.h
>>> +++ b/include/linux/nfs_xdr.h
>>> @@ -2,6 +2,7 @@
>>>  #define _LINUX_NFS_XDR_H
>>>  
>>>  #include <linux/nfsacl.h>
>>> +#include <linux/nfs3.h>
>>>  
>>>  /*
>>>   * To change the maximum rsize and wsize supported by the NFS client, adjust
>>> diff --git a/include/linux/nfsacl.h b/include/linux/nfsacl.h
>>> index 43011b6..f321b57 100644
>>> --- a/include/linux/nfsacl.h
>>> +++ b/include/linux/nfsacl.h
>>> @@ -29,6 +29,7 @@
>>>  #ifdef __KERNEL__
>>>  
>>>  #include <linux/posix_acl.h>
>>> +#include <linux/sunrpc/xdr.h>
>>>  
>>>  /* Maximum number of ACL entries over NFS */
>>>  #define NFS_ACL_MAX_ENTRIES	1024
>>> diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
>>> index 065a365..0dcf674 100644
>>> --- a/include/linux/posix_acl.h
>>> +++ b/include/linux/posix_acl.h
>>> @@ -9,6 +9,7 @@
>>>  #define __LINUX_POSIX_ACL_H
>>>  
>>>  #include <linux/slab.h>
>>> +#include <linux/fs.h>
> 
> NACK to this. Pretty much _all_ filesystems already include linux/fs.h
> somewhere in their include chains. There should be no need to include it
> in posix_acl.h too.
> 

from posix_acl.h:
extern int posix_acl_permission(struct inode *, const struct posix_acl *, int);

stuct inode is defined in fs.h. hence the direct dependency.

Again a double inclusion is not a bad thing, it costs absolutely *nothing*.

A miss-inclusion on the other hand is a bad thing. it causes a miss-compilation.

It does not matter that filesystems include fs.h or not. What matters is that they
now have to do this headers ordering magic. One places code compiles fine, another
place it does not. When it does not, people *never* analyze the missing dependency
what they do is copy-paste an include list from another file that works.

Proof of the matter the last patches in this patchset.

>>>  
>>>  #define ACL_UNDEFINED_ID	(-1)
>>>  
>>
> 
> So, what is the motivation for all this? We have no dependency problems
> here today. What is changing in the pNFS tree that makes this so
> necessary?
> 

We do have a dependency problem today!! look at the patchset. It is a grate
cleanup and improvement of code today. And a much smoother ride for the future.

What changed is that all this code was touched today. I have not done the cleanup for
any files not touched by pnfsd. Though I could and should, because they will greatly
improve just like these I did touch. Should I go head and do the reset of the files?

And please note that this particular file is an NFSD and vfs related file only, it
has nothing to do with nfs.

> Trond
> 

Boaz

Re: [pnfs] [PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[Benny Halevy]

On Nov. 12, 2009, 14:48 +0200, Boaz Harrosh <bharrosh@panasas.com> wrote:
> On 11/12/2009 12:35 PM, Trond Myklebust wrote:
>> On Thu, 2009-11-12 at 12:28 +0200, Boaz Harrosh wrote:
>>> On 10/21/2009 10:14 AM, Boaz Harrosh wrote:
>>>> An header should be compilation independent, .i.e pull in
>>>> any header who's declarations are directly used by this header.
>>>> And not let users re-include all it's dependencies all over
>>>> again.
>>>>
>>>> [At the end of the day what's the use of a header if it does
>>>>  not have more then one user?]
>>>>
>>>> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
>>> Trond do I have an ACK on this patch.
>>> If not, then what should be changed to get it accepted?
>>>
>>>> ---
>>>>  include/linux/nfs_xdr.h   |    1 +
>>> This header is used exclusively by fs/nfs/... files and could just be moved
>>> there. The include must be fixed as below though.
>>>
>>>>  include/linux/nfsacl.h    |    1 +
>>> This file is used mixed between fs/nfs && fs/nfsd
>>>
>>>>  include/linux/posix_acl.h |    1 +
>>> Used by nfsd and filesystems
>>>
>>>>  3 files changed, 3 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
>>>> index 2848a26..c316ca8 100644
>>>> --- a/include/linux/nfs_xdr.h
>>>> +++ b/include/linux/nfs_xdr.h
>>>> @@ -2,6 +2,7 @@
>>>>  #define _LINUX_NFS_XDR_H
>>>>  
>>>>  #include <linux/nfsacl.h>
>>>> +#include <linux/nfs3.h>
>>>>  
>>>>  /*
>>>>   * To change the maximum rsize and wsize supported by the NFS client, adjust
>>>> diff --git a/include/linux/nfsacl.h b/include/linux/nfsacl.h
>>>> index 43011b6..f321b57 100644
>>>> --- a/include/linux/nfsacl.h
>>>> +++ b/include/linux/nfsacl.h
>>>> @@ -29,6 +29,7 @@
>>>>  #ifdef __KERNEL__
>>>>  
>>>>  #include <linux/posix_acl.h>
>>>> +#include <linux/sunrpc/xdr.h>
>>>>  
>>>>  /* Maximum number of ACL entries over NFS */
>>>>  #define NFS_ACL_MAX_ENTRIES	1024
>>>> diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
>>>> index 065a365..0dcf674 100644
>>>> --- a/include/linux/posix_acl.h
>>>> +++ b/include/linux/posix_acl.h
>>>> @@ -9,6 +9,7 @@
>>>>  #define __LINUX_POSIX_ACL_H
>>>>  
>>>>  #include <linux/slab.h>
>>>> +#include <linux/fs.h>
>> NACK to this. Pretty much _all_ filesystems already include linux/fs.h
>> somewhere in their include chains. There should be no need to include it
>> in posix_acl.h too.
>>
> 
> from posix_acl.h:
> extern int posix_acl_permission(struct inode *, const struct posix_acl *, int);

get_cached_acl's inline definition would make a better example
as it actually needs the formal definition of struct inode;

> 
> stuct inode is defined in fs.h. hence the direct dependency.
> 
> Again a double inclusion is not a bad thing, it costs absolutely *nothing*.
> 
> A miss-inclusion on the other hand is a bad thing. it causes a miss-compilation.
> 
> It does not matter that filesystems include fs.h or not. What matters is that they
> now have to do this headers ordering magic. One places code compiles fine, another
> place it does not. When it does not, people *never* analyze the missing dependency
> what they do is copy-paste an include list from another file that works.
> 
> Proof of the matter the last patches in this patchset.
> 
>>>>  
>>>>  #define ACL_UNDEFINED_ID	(-1)
>>>>  
>> So, what is the motivation for all this? We have no dependency problems
>> here today. What is changing in the pNFS tree that makes this so
>> necessary?
>>

The motivation for this patchset was mainly patch 4/5
pnfsd: Move pnfsd code out of nfs4state.c/h
Boaz meant to amortize the cleanup effort on this mini-project.
Otherwise, by itself, I don't think we'd have started doing it...

Benny

> 
> We do have a dependency problem today!! look at the patchset. It is a grate
> cleanup and improvement of code today. And a much smoother ride for the future.
> 
> What changed is that all this code was touched today. I have not done the cleanup for
> any files not touched by pnfsd. Though I could and should, because they will greatly
> improve just like these I did touch. Should I go head and do the reset of the files?
> 
> And please note that this particular file is an NFSD and vfs related file only, it
> has nothing to do with nfs.
> 
>> Trond
>>
> 
> Boaz

Re: [pnfs] [PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[Trond Myklebust]

On Thu, 2009-11-12 at 14:48 +0200, Boaz Harrosh wrote:
> On 11/12/2009 12:35 PM, Trond Myklebust wrote:
> > On Thu, 2009-11-12 at 12:28 +0200, Boaz Harrosh wrote:
> >> On 10/21/2009 10:14 AM, Boaz Harrosh wrote:
> >>> An header should be compilation independent, .i.e pull in
> >>> any header who's declarations are directly used by this header.
> >>> And not let users re-include all it's dependencies all over
> >>> again.
> >>>
> >>> [At the end of the day what's the use of a header if it does
> >>>  not have more then one user?]
> >>>
> >>> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
> >>
> >> Trond do I have an ACK on this patch.
> >> If not, then what should be changed to get it accepted?
> >>
> >>> ---
> >>>  include/linux/nfs_xdr.h   |    1 +
> >>
> >> This header is used exclusively by fs/nfs/... files and could just be moved
> >> there. The include must be fixed as below though.
> >>
> >>>  include/linux/nfsacl.h    |    1 +
> >>
> >> This file is used mixed between fs/nfs && fs/nfsd
> >>
> >>>  include/linux/posix_acl.h |    1 +
> >>
> >> Used by nfsd and filesystems
> >>
> >>>  3 files changed, 3 insertions(+), 0 deletions(-)
> >>>
> >>> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> >>> index 2848a26..c316ca8 100644
> >>> --- a/include/linux/nfs_xdr.h
> >>> +++ b/include/linux/nfs_xdr.h
> >>> @@ -2,6 +2,7 @@
> >>>  #define _LINUX_NFS_XDR_H
> >>>  
> >>>  #include <linux/nfsacl.h>
> >>> +#include <linux/nfs3.h>
> >>>  
> >>>  /*
> >>>   * To change the maximum rsize and wsize supported by the NFS client, adjust
> >>> diff --git a/include/linux/nfsacl.h b/include/linux/nfsacl.h
> >>> index 43011b6..f321b57 100644
> >>> --- a/include/linux/nfsacl.h
> >>> +++ b/include/linux/nfsacl.h
> >>> @@ -29,6 +29,7 @@
> >>>  #ifdef __KERNEL__
> >>>  
> >>>  #include <linux/posix_acl.h>
> >>> +#include <linux/sunrpc/xdr.h>
> >>>  
> >>>  /* Maximum number of ACL entries over NFS */
> >>>  #define NFS_ACL_MAX_ENTRIES	1024
> >>> diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
> >>> index 065a365..0dcf674 100644
> >>> --- a/include/linux/posix_acl.h
> >>> +++ b/include/linux/posix_acl.h
> >>> @@ -9,6 +9,7 @@
> >>>  #define __LINUX_POSIX_ACL_H
> >>>  
> >>>  #include <linux/slab.h>
> >>> +#include <linux/fs.h>
> > 
> > NACK to this. Pretty much _all_ filesystems already include linux/fs.h
> > somewhere in their include chains. There should be no need to include it
> > in posix_acl.h too.
> > 
> 
> from posix_acl.h:
> extern int posix_acl_permission(struct inode *, const struct posix_acl *, int);
> 
> stuct inode is defined in fs.h. hence the direct dependency.

Add a forward declaration of the form

struct inode;

posix_acl.h, and you're done. No need to read the entire contents of
fs.h plus all the crap it drags in with it, in order to satisfy a single
pointer dependence.

> Again a double inclusion is not a bad thing, it costs absolutely *nothing*.
>
> A miss-inclusion on the other hand is a bad thing. it causes a miss-compilation.

Really?

> It does not matter that filesystems include fs.h or not. What matters is that they
> now have to do this headers ordering magic. One places code compiles fine, another
> place it does not. When it does not, people *never* analyze the missing dependency
> what they do is copy-paste an include list from another file that works.
> 
> Proof of the matter the last patches in this patchset.
> 
> >>>  
> >>>  #define ACL_UNDEFINED_ID	(-1)
> >>>  
> >>
> > 
> > So, what is the motivation for all this? We have no dependency problems
> > here today. What is changing in the pNFS tree that makes this so
> > necessary?
> > 
> 
> We do have a dependency problem today!! look at the patchset. It is a grate
> cleanup and improvement of code today. And a much smoother ride for the future.

> What changed is that all this code was touched today. I have not done the cleanup for
> any files not touched by pnfsd. Though I could and should, because they will greatly
> improve just like these I did touch. Should I go head and do the reset of the files?
> 
> And please note that this particular file is an NFSD and vfs related file only, it
> has nothing to do with nfs.

...and hence you need to send it to the VFS maintainer. I'm not touching
that patch, or the patchset that "requires" it.

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: [pnfs] [PATCH 2/5] nfsd: Fix independence of a few nfsd related headers

[Boaz Harrosh]

On 11/12/2009 03:36 PM, Trond Myklebust wrote:
> On Thu, 2009-11-12 at 14:48 +0200, Boaz Harrosh wrote:
>>
>> from posix_acl.h:
>> extern int posix_acl_permission(struct inode *, const struct posix_acl *, int);
>>
>> stuct inode is defined in fs.h. hence the direct dependency.
> 
> Add a forward declaration of the form
> 
> struct inode;
> 

This will not work.

<quote>
...

static inline struct posix_acl *get_cached_acl(struct inode *inode, int type)
{
	struct posix_acl **p, *acl;
	switch (type) {
	case ACL_TYPE_ACCESS:
		p = &inode->i_acl;
		break;
	case ACL_TYPE_DEFAULT:
		p = &inode->i_default_acl;
		break;
	default:
		return ERR_PTR(-EINVAL);
	}
...
</quote>

the header has inlines that directly manipulate inode members
a forward declaration is not enough.

<snip>
>> And please note that this particular file is an NFSD and vfs related file only, it
>> has nothing to do with nfs.
> 
> ...and hence you need to send it to the VFS maintainer. I'm not touching
> that patch, or the patchset that "requires" it.
> 

OK This particular hunk I will send to fsdevel, anything else?

Boaz