[PATCH 0/9] user space NFSv2/v3 lock recovery, take 2

[PATCH 0/9] user space NFSv2/v3 lock recovery, take 2

[Chuck Lever]

I've trimmed the patches that Trond objected to last time, and, at
Bruce's request, added documentation under
Documentation/filesystems/nfs/ that describes the new API.  As an
addendum to this cover letter, the patch that adds the API
documentation is first in this series.

---

Chuck Lever (9):
      lockd: Add "reboot" attribute to nsm_handles
      lockd: Refactor nlm_host_rebooted()
      lockd: Add /sys/fs/lockd/nsm_handle/*
      lockd: Add /sys/fs/lockd/mon
      lockd: Add attributes to /sys/fs/lockd/hosts/*/
      lockd: Add /sys/fs/lockd/hosts/*
      lockd: Add /sys/fs/lockd/hosts/
      lockd: Add /sys/fs/lockd
      lockd: Document new lockd user space API


 Documentation/filesystems/nfs/00-INDEX            |    2 
 Documentation/filesystems/nfs/lockd-sysfs-api.txt |  117 ++++++++
 fs/lockd/host.c                                   |  312 ++++++++++++++++++++-
 fs/lockd/mon.c                                    |  211 ++++++++++++++
 fs/lockd/svc.c                                    |   31 ++
 include/linux/lockd/lockd.h                       |    9 +
 6 files changed, 659 insertions(+), 23 deletions(-)
 create mode 100644 Documentation/filesystems/nfs/lockd-sysfs-api.txt

-- 
Chuck Lever <chuck.lever@oracle.com>

[PATCH 1/9] lockd: Document new lockd user space API

[Chuck Lever]

Add a file under Documents that describes the file system structure
under /sys/fs/lockd.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 Documentation/filesystems/nfs/00-INDEX            |    2 
 Documentation/filesystems/nfs/lockd-sysfs-api.txt |  117 +++++++++++++++++++++
 2 files changed, 119 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/filesystems/nfs/lockd-sysfs-api.txt

diff --git a/Documentation/filesystems/nfs/00-INDEX b/Documentation/filesystems/nfs/00-INDEX
index 2f68cd6..a3a8c39 100644
--- a/Documentation/filesystems/nfs/00-INDEX
+++ b/Documentation/filesystems/nfs/00-INDEX
@@ -2,6 +2,8 @@
 	- this file (nfs-related documentation).
 Exporting
 	- explanation of how to make filesystems exportable.
+lockd-sysfs-api.txt
+	- how /sys/fs/lockd/ works.
 knfsd-stats.txt
 	- statistics which the NFS server makes available to user space.
 nfs.txt
diff --git a/Documentation/filesystems/nfs/lockd-sysfs-api.txt b/Documentation/filesystems/nfs/lockd-sysfs-api.txt
new file mode 100644
index 0000000..bb6ad8d
--- /dev/null
+++ b/Documentation/filesystems/nfs/lockd-sysfs-api.txt
@@ -0,0 +1,117 @@
+An overview of the /sys/fs/lockd directory hierarchy
+
+Chuck Lever <chuck.lever@oracle.com>
+Tue Apr 27 12:26:31 EDT 2010
+
+Introduction
+------------
+
+Traditionally, lockd and statd communicate via loopback networking.
+Loopback networking can take many different forms, and sometimes is
+not even available (for example, during boot).  Supporting all of
+these configurations can add implementation complexity.
+
+In addition, we'd like to provide a mechanism for NFS server system
+administrators to remove NFS locks on clients that may no longer be
+able to contact the server.
+
+An obvious alternative to loopback networking is to provide a lockd
+user space API via the local file system.  A set of files under /sys
+can export lockd state to user space.
+
+
+File Hierarchy
+---- ---------
+
+When loaded, lockd creates directories under /sys/fs/lockd.  Currently,
+these subdirectories are created:
+
+
+  /sys/fs/lockd/hosts/
+	This directory contains a subdirectory for each cached nlm_host.
+	Each subdirectory is named after the remote's address, the
+	transport protocol it used to contact us ("udp" or "tcp"), and
+	the NLM version number in use.
+
+	Each nlm_host subdirectory contains files that expose the
+	contents of one nlm_host cache entry.  The files are named
+	after each field in the nlm_host data structure.
+
+	You can see the IP address of the remote, the source address
+	the local host uses to contact the remote, the hostname
+	provided by the remote, the NLM version and transport, and
+	other information.
+
+	A symlink is created to the nsm_handle subdirectory (under
+	/sys/fs/lockd/monitor/) associated with this nlm_host cache
+	entry.
+
+
+  /sys/fs/lockd/monitor/
+	This directory contains a subdirectory for each monitored host.
+	Each subdirectory is named after the 16-byte priv cookie lockd
+	uses to identify this host to statd.
+
+	Each host subdirectory contains files that expose the
+	contents of one nsm_handle cache entry.  The files are
+	named after each field in the nsm_handle structure.
+
+	You can see the mon_name and my_name used for this remote,
+	the RPC parms statd will use to call lockd back, the
+	remote's IP address, and other related information.
+
+	A file called "reboot" exists to allow user space to cause
+	the local lockd to initiate reboot recovery for the remote
+	represented by this nsm_handle.  The new NSM state number for
+	this remote is written to this file to trigger reboot
+	recovery.
+
+
+New subdirectories are created in each of monitor/ and hosts/ as
+remotes establish contact with the local lockd.  The entries are
+removed when lockd garbage collects them.
+
+The read-only attribute files provided by this interface are
+permitted read-only to "world."  The reboot attribute is readable
+and writable only by root.
+
+In other words, the data fields of nlm_host and nsm_handle cache
+entries are exposed to user space, one per file, via this directory
+hierarchy.  This is a straightforward, one-to-one mapping of the
+items in the nlm_host and nsm_handle cache to kobjects and thus to
+files and directories.
+
+
+Usage
+-----
+
+To cause lockd to drop locks for a particular remote, a user program:
+
+   1.  Looks up the remote's hostname or IP address in /sys/fs/lockd/
+
+   2.  Reads the current NSM state from the remote's nlm_host
+       attribute file
+
+   3.  Subtracts two from the NSM state number and writes the result
+       into the corresponding nsm_handle reboot attribute file.
+
+Unfortunately, the NSM state number arithmetic is necessary to allow
+lockd to continue to track the NSM state of the remote properly.  NSM
+state number management is an integral part of lockd.  lockd allows
+reboot recovery to start only if the incoming NSM state number is
+different than the NSM state number it currently has for that remote.
+
+New NSM state numbers are generated almost always by adding two.
+Subtracting provides some guarantee that the remote won't reuse the
+NSM state number we made up to trigger reboot recovery.
+
+System administrator capabilities (ie. EUID of 0) are required to write
+the new NSM state number into an nsm_handle's reboot attribute.  At
+some later point, lockd and statd AF_UNIX sockets may also be provided
+to allow NSM activity to proceed without the consistent presence of
+loopback networking.  This might be necessary because programs like
+statd drop root, and thus can't use the root-only interface provided by
+the nsm_handle reboot attribute.
+
+
+

[PATCH 2/9] lockd: Add /sys/fs/lockd

[Chuck Lever]

Create the parent directory for all lockd-related sysfs entries.  The
directory is created when lockd.ko is loaded, and removed when it is
unloaded.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/lockd/svc.c              |   16 +++++++++++++---
 include/linux/lockd/lockd.h |    1 +
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index f1bacf1..c84a7d5 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -75,6 +75,8 @@ static const int		nlm_port_min = 0, nlm_port_max = 65535;
 static struct ctl_table_header * nlm_sysctl_table;
 #endif
 
+struct kobject			*nlm_kobj;
+
 static unsigned long get_lockd_grace_period(void)
 {
 	/* Note: nlm_timeout should always be nonzero */
@@ -514,12 +516,19 @@ module_param(nlm_max_connections, uint, 0644);
 
 static int __init init_nlm(void)
 {
+	nlm_kobj = kobject_create_and_add("lockd", fs_kobj);
+	if (!nlm_kobj)
+		return -ENOMEM;
+
 #ifdef CONFIG_SYSCTL
 	nlm_sysctl_table = register_sysctl_table(nlm_sysctl_root);
-	return nlm_sysctl_table ? 0 : -ENOMEM;
-#else
-	return 0;
+	if (!nlm_sysctl_table) {
+		kobject_put(nlm_kobj);
+		return -ENOMEM;
+	}
 #endif
+
+	return 0;
 }
 
 static void __exit exit_nlm(void)
@@ -529,6 +538,7 @@ static void __exit exit_nlm(void)
 #ifdef CONFIG_SYSCTL
 	unregister_sysctl_table(nlm_sysctl_table);
 #endif
+	kobject_put(nlm_kobj);
 }
 
 module_init(init_nlm);
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index a34dea4..56c749e 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -196,6 +196,7 @@ extern int			nlmsvc_grace_period;
 extern unsigned long		nlmsvc_timeout;
 extern int			nsm_use_hostnames;
 extern u32			nsm_local_state;
+extern struct kobject		*nlm_kobj;
 
 /*
  * Lockd client functions

[PATCH 3/9] lockd: Add /sys/fs/lockd/hosts/

[Chuck Lever]

Add parent directory to contain directories representing entries in
lockd's host cache.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/lockd/host.c             |   46 +++++++++++++++++++++++++++++++++++++++++++
 fs/lockd/svc.c              |    7 +++++++
 include/linux/lockd/lockd.h |    2 ++
 3 files changed, 55 insertions(+), 0 deletions(-)

diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index bb464d1..be3f87c 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -45,6 +45,10 @@ struct nlm_lookup_host_info {
 	const int		noresvport;	/* use non-priv port */
 };
 
+static struct kobject		*nlm_hosts_kobj;
+static struct kobject		*nlm_hosts_client_kobj;
+static struct kobject		*nlm_hosts_server_kobj;
+
 /*
  * Hash function must work well on big- and little-endian platforms
  */
@@ -567,3 +571,45 @@ nlm_gc_hosts(void)
 
 	next_gc = jiffies + NLM_HOST_COLLECT;
 }
+
+/**
+ * nlm_host_init - Called when lockd module is loaded
+ *
+ */
+int nlm_host_init(void)
+{
+	nlm_hosts_kobj = kobject_create_and_add("hosts", nlm_kobj);
+	if (nlm_hosts_kobj == NULL)
+		return -ENOMEM;
+
+	nlm_hosts_server_kobj = kobject_create_and_add("server", nlm_hosts_kobj);
+	if (nlm_hosts_server_kobj == NULL) {
+		kobject_put(nlm_hosts_kobj);
+		return -ENOMEM;
+	}
+
+	nlm_hosts_client_kobj = kobject_create_and_add("client", nlm_hosts_kobj);
+	if (nlm_hosts_client_kobj == NULL) {
+		kobject_put(nlm_hosts_server_kobj);
+		kobject_put(nlm_hosts_kobj);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+/**
+ * nlm_host_shutdown - Called when lockd module is about to be unloaded
+ *
+ */
+void nlm_host_shutdown(void)
+{
+	if (nrhosts != 0) {
+		printk(KERN_WARNING "NLM hosts remain at shutdown");
+		return;
+ 	}
+
+	kobject_put(nlm_hosts_client_kobj);
+	kobject_put(nlm_hosts_server_kobj);
+	kobject_put(nlm_hosts_kobj);
+}
diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index c84a7d5..148623d 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -520,9 +520,15 @@ static int __init init_nlm(void)
 	if (!nlm_kobj)
 		return -ENOMEM;
 
+	if (nlm_host_init()) {
+		kobject_put(nlm_kobj);
+		return -ENOMEM;
+	}
+
 #ifdef CONFIG_SYSCTL
 	nlm_sysctl_table = register_sysctl_table(nlm_sysctl_root);
 	if (!nlm_sysctl_table) {
+		nlm_host_shutdown();
 		kobject_put(nlm_kobj);
 		return -ENOMEM;
 	}
@@ -538,6 +544,7 @@ static void __exit exit_nlm(void)
 #ifdef CONFIG_SYSCTL
 	unregister_sysctl_table(nlm_sysctl_table);
 #endif
+	nlm_host_shutdown();
 	kobject_put(nlm_kobj);
 }
 
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index 56c749e..2d89cdb 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -217,6 +217,8 @@ void		  nlmclnt_next_cookie(struct nlm_cookie *);
 /*
  * Host cache
  */
+int		  nlm_host_init(void);
+void		  nlm_host_shutdown(void);
 struct nlm_host  *nlmclnt_lookup_host(const struct sockaddr *sap,
 					const size_t salen,
 					const unsigned short protocol,

[PATCH 4/9] lockd: Add /sys/fs/lockd/hosts/*

[Chuck Lever]

Populate the new hosts/ directory as nlm_hosts come and go.
The new subdirectories are named after the IP address, transport
protocol, and NLM protocol version of each entry in the nlm_hosts
cache.

Introduce a simple-minded approach to managing the kref in the
nlm_host's kobject.  Unfortunately the existing nlm_host reference
counting is more complex than other typical kref usage in the kernel,
so we retain the h_count field.  The kref is always 1 until the
nlm_host is ready to be destroyed.


Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/lockd/host.c             |   89 ++++++++++++++++++++++++++++++++++++++++++-
 include/linux/lockd/lockd.h |    1 
 2 files changed, 87 insertions(+), 3 deletions(-)

diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index be3f87c..2dce492 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -30,6 +30,7 @@ static unsigned long		next_gc;
 static int			nrhosts;
 static DEFINE_MUTEX(nlm_host_mutex);
 
+static void			nlm_kobj_release(struct kobject *kobj);
 static void			nlm_gc_hosts(void);
 
 struct nlm_lookup_host_info {
@@ -49,6 +50,48 @@ static struct kobject		*nlm_hosts_kobj;
 static struct kobject		*nlm_hosts_client_kobj;
 static struct kobject		*nlm_hosts_server_kobj;
 
+struct nlm_host_attr {
+	struct attribute	attr;
+	ssize_t (*show)(struct nlm_host *host, char *buf);
+	ssize_t (*store)(struct nlm_host *host, const char *buf, size_t count);
+};
+
+static ssize_t nlm_kobj_show(struct kobject *kobj, struct attribute *attr,
+			     char *buf)
+{
+	struct nlm_host_attr *hattr = container_of(attr,
+						struct nlm_host_attr, attr);
+	struct nlm_host *host = container_of(kobj, struct nlm_host, h_kobj);
+	if (hattr->show)
+		return hattr->show(host, buf);
+	return 0;
+}
+
+static ssize_t nlm_kobj_store(struct kobject *kobj, struct attribute *attr,
+			      const char *buf, size_t count)
+{
+	struct nlm_host_attr *hattr = container_of(attr,
+						struct nlm_host_attr, attr);
+	struct nlm_host *host = container_of(kobj, struct nlm_host, h_kobj);
+	if (hattr->store)
+		return hattr->store(host, buf, count);
+	return 0;
+}
+
+static struct sysfs_ops nlm_kobj_sysfs_ops = {
+	.show		= nlm_kobj_show,
+	.store		= nlm_kobj_store,
+};
+
+static struct attribute *nlm_kobj_attrs[] = {
+};
+
+static struct kobj_type nlm_host_ktype = {
+	.release		= nlm_kobj_release,
+	.sysfs_ops		= &nlm_kobj_sysfs_ops,
+	.default_attrs		= nlm_kobj_attrs,
+};
+
 /*
  * Hash function must work well on big- and little-endian platforms
  */
@@ -92,6 +135,35 @@ static unsigned int nlm_hash_address(const struct sockaddr *sap)
 }
 
 /*
+ * New kobjects are placed in either client/ or server/
+ * subdirectory and are named "ip-proto-vers".
+ */
+int nlm_add_kobject(struct nlm_host *host)
+{
+	struct kobject *parent;
+	char *proto_name;
+
+	parent = nlm_hosts_server_kobj;
+	if (host->h_server)
+		parent = nlm_hosts_client_kobj;
+
+	switch (host->h_proto) {
+	case IPPROTO_UDP:
+		proto_name = "udp";
+		break;
+	case IPPROTO_TCP:
+		proto_name = "tcp";
+		break;
+	default:
+		return -EPROTONOSUPPORT;
+	}
+
+	return kobject_init_and_add(&host->h_kobj, &nlm_host_ktype,
+				parent, "%s-%s-%u", host->h_addrbuf,
+				proto_name, host->h_version);
+}
+
+/*
  * Common host lookup routine for server & client
  */
 static struct nlm_host *nlm_lookup_host(struct nlm_lookup_host_info *ni)
@@ -100,6 +172,7 @@ static struct nlm_host *nlm_lookup_host(struct nlm_lookup_host_info *ni)
 	struct hlist_node *pos;
 	struct nlm_host	*host;
 	struct nsm_handle *nsm = NULL;
+	int ret;
 
 	mutex_lock(&nlm_host_mutex);
 
@@ -191,6 +264,16 @@ static struct nlm_host *nlm_lookup_host(struct nlm_lookup_host_info *ni)
 	INIT_LIST_HEAD(&host->h_granted);
 	INIT_LIST_HEAD(&host->h_reclaim);
 
+	ret = nlm_add_kobject(host);
+	if (unlikely(ret != 0)) {
+		atomic_set(&host->h_count, 0);
+		kobject_put(&host->h_kobj);	/* releases nsm too */
+		dprintk("lockd: nlm_lookup_host "
+				"could not add nlm_host kobject\n");
+		host = NULL;
+		goto out;
+	}
+
 	nrhosts++;
 
 	dprintk("lockd: nlm_lookup_host created host %s\n",
@@ -204,9 +287,9 @@ out:
 /*
  * Destroy a host
  */
-static void
-nlm_destroy_host(struct nlm_host *host)
+static void nlm_kobj_release(struct kobject *kobj)
 {
+	struct nlm_host *host = container_of(kobj, struct nlm_host, h_kobj);
 	struct rpc_clnt	*clnt;
 
 	BUG_ON(!list_empty(&host->h_lockowners));
@@ -564,7 +647,7 @@ nlm_gc_hosts(void)
 			dprintk("lockd: delete host %s\n", host->h_name);
 			hlist_del_init(&host->h_hash);
 
-			nlm_destroy_host(host);
+			kobject_put(&host->h_kobj);
 			nrhosts--;
 		}
 	}
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index 2d89cdb..bdb87a6 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -39,6 +39,7 @@
  * Lockd host handle (used both by the client and server personality).
  */
 struct nlm_host {
+	struct kobject		h_kobj;
 	struct hlist_node	h_hash;		/* doubly linked list */
 	struct sockaddr_storage	h_addr;		/* peer address */
 	size_t			h_addrlen;

[PATCH 5/9] lockd: Add attributes to /sys/fs/lockd/hosts/*/

[Chuck Lever]

Expose fields in each nlm_host cache entry via files under
/sys/fs/lockd/hosts/{client,server}/*/.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/lockd/host.c |  137 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 137 insertions(+), 0 deletions(-)

diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index 2dce492..d6f3141 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -83,7 +83,144 @@ static struct sysfs_ops nlm_kobj_sysfs_ops = {
 	.store		= nlm_kobj_store,
 };
 
+static ssize_t address_show(struct nlm_host *host, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%s\n", host->h_addrbuf);
+}
+
+static ssize_t source_address_show(struct nlm_host *host, char *buf)
+{
+	struct sockaddr *sap = (struct sockaddr *)(char *)&host->h_srcaddr;
+	switch (sap->sa_family) {
+	case AF_UNSPEC:
+		return snprintf(buf, PAGE_SIZE, "unspecified address\n");
+	case AF_INET:
+		return snprintf(buf, PAGE_SIZE, "%pI4\n", sap);
+	case AF_INET6:
+		return snprintf(buf, PAGE_SIZE, "%pI6c\n", sap);
+	}
+	return snprintf(buf, PAGE_SIZE, "unrecognized address\n");
+}
+
+static ssize_t name_show(struct nlm_host *host, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%s\n", host->h_name);
+}
+
+static ssize_t nlm_version_show(struct nlm_host *host, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%u\n", host->h_version);
+}
+
+static ssize_t transport_show(struct nlm_host *host, char *buf)
+{
+	switch (host->h_proto) {
+	case IPPROTO_UDP:
+		return snprintf(buf, PAGE_SIZE, "udp\n");
+	case IPPROTO_TCP:
+		return snprintf(buf, PAGE_SIZE, "tcp\n");
+	}
+	return snprintf(buf, PAGE_SIZE, "unknown\n");
+}
+
+static ssize_t reclaiming_show(struct nlm_host *host, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%s\n",
+				host->h_reclaiming ? "in progress" : "no");
+}
+
+static ssize_t peer_show(struct nlm_host *host, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%s\n",
+				host->h_server ? "client" : "server");
+}
+
+static ssize_t resvport_show(struct nlm_host *host, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%s\n",
+				host->h_noresvport ? "no" : "yes");
+}
+
+static ssize_t in_use_show(struct nlm_host *host, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%s\n",
+				host->h_inuse ? "yes" : "no");
+}
+
+static ssize_t pseudo_state_show(struct nlm_host *host, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%u\n", host->h_state);
+}
+
+static ssize_t nsm_state_show(struct nlm_host *host, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%u\n", host->h_nsmstate);
+}
+
+static ssize_t pidcount_show(struct nlm_host *host, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%u\n", host->h_pidcount);
+}
+
+static ssize_t refcount_show(struct nlm_host *host, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%d\n", atomic_read(&host->h_count));
+}
+
+static ssize_t nextrebind_show(struct nlm_host *host, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%lu seconds \n",
+				(host->h_nextrebind - jiffies) / HZ);
+}
+
+static ssize_t expires_show(struct nlm_host *host, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%lu seconds\n",
+				(host->h_expires - jiffies) / HZ);
+}
+
+#ifdef __ATTR_RO
+#undef __ATTR_RO
+#endif
+
+#define __ATTR_RO(_name) { \
+	.attr	= { .name = __stringify(_name), .mode = S_IRUSR, }, \
+ 	.show	= _name##_show,                                 \
+}
+
+static struct nlm_host_attr address_attr	= __ATTR_RO(address);
+static struct nlm_host_attr source_address_attr	= __ATTR_RO(source_address);
+static struct nlm_host_attr name_attr		= __ATTR_RO(name);
+static struct nlm_host_attr nlm_version_attr	= __ATTR_RO(nlm_version);
+static struct nlm_host_attr transport_attr	= __ATTR_RO(transport);
+static struct nlm_host_attr reclaiming_attr	= __ATTR_RO(reclaiming);
+static struct nlm_host_attr peer_attr		= __ATTR_RO(peer);
+static struct nlm_host_attr resvport_attr	= __ATTR_RO(resvport);
+static struct nlm_host_attr in_use_attr		= __ATTR_RO(in_use);
+static struct nlm_host_attr pseudo_state_attr	= __ATTR_RO(pseudo_state);
+static struct nlm_host_attr nsm_state_attr	= __ATTR_RO(nsm_state);
+static struct nlm_host_attr pidcount_attr	= __ATTR_RO(pidcount);
+static struct nlm_host_attr refcount_attr	= __ATTR_RO(refcount);
+static struct nlm_host_attr nextrebind_attr	= __ATTR_RO(nextrebind);
+static struct nlm_host_attr expires_attr	= __ATTR_RO(expires);
+
 static struct attribute *nlm_kobj_attrs[] = {
+	&address_attr.attr,
+	&source_address_attr.attr,
+	&name_attr.attr,
+	&nlm_version_attr.attr,
+	&transport_attr.attr,
+	&reclaiming_attr.attr,
+	&peer_attr.attr,
+	&resvport_attr.attr,
+	&in_use_attr.attr,
+	&pseudo_state_attr.attr,
+	&nsm_state_attr.attr,
+	&pidcount_attr.attr,
+	&refcount_attr.attr,
+	&nextrebind_attr.attr,
+	&expires_attr.attr,
+	NULL,
 };
 
 static struct kobj_type nlm_host_ktype = {

[PATCH 6/9] lockd: Add /sys/fs/lockd/mon

[Chuck Lever]

Add parent directory to contain entries representing each entry in
lockd's nsm_handles cache.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/lockd/mon.c              |   27 +++++++++++++++++++++++++++
 fs/lockd/svc.c              |    8 ++++++++
 include/linux/lockd/lockd.h |    2 ++
 3 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c
index e301546..6e16a28 100644
--- a/fs/lockd/mon.c
+++ b/fs/lockd/mon.c
@@ -57,6 +57,8 @@ static				DEFINE_SPINLOCK(nsm_lock);
 u32	__read_mostly		nsm_local_state;
 int	__read_mostly		nsm_use_hostnames;
 
+static struct kobject		*nsm_handles_kobj;
+
 static inline struct sockaddr *nsm_addr(const struct nsm_handle *nsm)
 {
 	return (struct sockaddr *)&nsm->sm_addr;
@@ -393,6 +395,31 @@ void nsm_release(struct nsm_handle *nsm)
 	}
 }
 
+/**
+ * nsm_init - Called when lockd module is loaded
+ *
+ */
+int nsm_init(void)
+{
+	nsm_handles_kobj = kobject_create_and_add("monitor", nlm_kobj);
+	if (nsm_handles_kobj == NULL)
+		return -ENOMEM;
+	return 0;
+}
+
+/**
+ * nsm_shutdown - Called when lockd module is about to be unloaded
+ *
+ */
+void nsm_shutdown(void)
+{
+	if (!list_empty(&nsm_handles)) {
+		printk(KERN_WARNING "NSM handles remain at shutdown");
+		return;
+	}
+	kobject_put(nsm_handles_kobj);
+}
+
 /*
  * XDR functions for NSM.
  *
diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index 148623d..1b484bf 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -525,9 +525,16 @@ static int __init init_nlm(void)
 		return -ENOMEM;
 	}
 
+	if (nsm_init()) {
+		nlm_host_shutdown();
+		kobject_put(nlm_kobj);
+		return -ENOMEM;
+	}
+
 #ifdef CONFIG_SYSCTL
 	nlm_sysctl_table = register_sysctl_table(nlm_sysctl_root);
 	if (!nlm_sysctl_table) {
+		nsm_shutdown();
 		nlm_host_shutdown();
 		kobject_put(nlm_kobj);
 		return -ENOMEM;
@@ -544,6 +551,7 @@ static void __exit exit_nlm(void)
 #ifdef CONFIG_SYSCTL
 	unregister_sysctl_table(nlm_sysctl_table);
 #endif
+	nsm_shutdown();
 	nlm_host_shutdown();
 	kobject_put(nlm_kobj);
 }
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index bdb87a6..8622a28 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -239,6 +239,8 @@ void		  nlm_host_rebooted(const struct nlm_reboot *);
 /*
  * Host monitoring
  */
+int		  nsm_init(void);
+void		  nsm_shutdown(void);
 int		  nsm_monitor(const struct nlm_host *host);
 void		  nsm_unmonitor(const struct nlm_host *host);
 

[PATCH 7/9] lockd: Add /sys/fs/lockd/nsm_handle/*

[Chuck Lever]

Populate the new nsm_handle/ directory as nsm_handles come and go.
The entries are named by their priv cookie, which is always guaranteed
to be unique.

Like the nlm_hosts cache, the kref is fixed at one when the handle
is created, and goes to zero only when the handle is ready to be
destroyed.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/lockd/host.c             |    7 ++
 fs/lockd/mon.c              |  166 +++++++++++++++++++++++++++++++++++++++++--
 include/linux/lockd/lockd.h |    1 
 3 files changed, 166 insertions(+), 8 deletions(-)

diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index d6f3141..20dab62 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -279,6 +279,7 @@ int nlm_add_kobject(struct nlm_host *host)
 {
 	struct kobject *parent;
 	char *proto_name;
+	int ret;
 
 	parent = nlm_hosts_server_kobj;
 	if (host->h_server)
@@ -295,9 +296,13 @@ int nlm_add_kobject(struct nlm_host *host)
 		return -EPROTONOSUPPORT;
 	}
 
-	return kobject_init_and_add(&host->h_kobj, &nlm_host_ktype,
+	ret = kobject_init_and_add(&host->h_kobj, &nlm_host_ktype,
 				parent, "%s-%s-%u", host->h_addrbuf,
 				proto_name, host->h_version);
+	if (likely(ret == 0))
+		ret = sysfs_create_link(&host->h_kobj,
+				&host->h_nsmhandle->sm_kobj, "nsm_handle");
+	return ret;
 }
 
 /*
diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c
index 6e16a28..851ad59 100644
--- a/fs/lockd/mon.c
+++ b/fs/lockd/mon.c
@@ -59,6 +59,129 @@ int	__read_mostly		nsm_use_hostnames;
 
 static struct kobject		*nsm_handles_kobj;
 
+struct nsm_handle_attr {
+	struct attribute	attr;
+	ssize_t (*show)(struct nsm_handle *nsm, char *buf);
+	ssize_t (*store)(struct nsm_handle *nsm, const char *buf, size_t count);
+};
+
+static ssize_t nsm_kobj_show(struct kobject *kobj, struct attribute *attr,
+			     char *buf)
+{
+	struct nsm_handle_attr *nattr = container_of(attr,
+						struct nsm_handle_attr, attr);
+	struct nsm_handle *nsm = container_of(kobj, struct nsm_handle, sm_kobj);
+	if (nattr->show)
+		return nattr->show(nsm, buf);
+	return 0;
+}
+
+static ssize_t nsm_kobj_store(struct kobject *kobj, struct attribute *attr,
+			      const char *buf, size_t count)
+{
+	struct nsm_handle_attr *nattr = container_of(attr,
+						struct nsm_handle_attr, attr);
+	struct nsm_handle *nsm = container_of(kobj, struct nsm_handle, sm_kobj);
+	if (nattr->store)
+		return nattr->store(nsm, buf, count);
+	return 0;
+}
+
+static void nsm_kobj_release(struct kobject *kobj)
+{
+	struct nsm_handle *nsm = container_of(kobj,
+					struct nsm_handle, sm_kobj);
+
+	dprintk("lockd: destroyed nsm_handle for %s (%s)\n",
+			nsm->sm_name, nsm->sm_addrbuf);
+	kfree(nsm);
+}
+
+static struct sysfs_ops nsm_kobj_sysfs_ops = {
+	.show		= nsm_kobj_show,
+	.store		= nsm_kobj_store,
+};
+
+static ssize_t mon_name_show(struct nsm_handle *nsm, char *buf)
+{
+	char *mon_name = nsm->sm_mon_name;
+
+	/* When this host is unmonitored, sm_mon_name is NULL,
+	 * so make up something reasonable. */
+	if (mon_name == NULL)
+		mon_name = nsm_use_hostnames ? nsm->sm_name : nsm->sm_addrbuf;
+
+	return snprintf(buf, PAGE_SIZE, "%s\n", mon_name);
+}
+
+static ssize_t my_name_show(struct nsm_handle *nsm, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%s\n", utsname()->nodename);
+}
+
+static ssize_t address_show(struct nsm_handle *nsm, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%s\n", nsm->sm_addrbuf);
+}
+
+static ssize_t rpcparms_show(struct nsm_handle *nsm, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%u %u %u\n",
+			NLM_PROGRAM, 3, NLMPROC_NSM_NOTIFY);
+}
+
+static ssize_t status_show(struct nsm_handle *nsm, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%smonitored\n",
+			nsm->sm_monitored ? "" : "un" );
+}
+
+static ssize_t sticky_show(struct nsm_handle *nsm, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%s\n",
+			nsm->sm_sticky ? "yes" : "no" );
+}
+
+static ssize_t refcount_show(struct nsm_handle *nsm, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%d\n",
+			atomic_read(&nsm->sm_count));
+}
+
+#ifdef __ATTR_RO
+#undef __ATTR_RO
+#endif
+
+#define __ATTR_RO(_name) { \
+	.attr	= { .name = __stringify(_name), .mode = S_IRUSR, }, \
+ 	.show	= _name##_show,                                 \
+}
+
+static struct nsm_handle_attr mon_name_attr	= __ATTR_RO(mon_name);
+static struct nsm_handle_attr my_name_attr	= __ATTR_RO(my_name);
+static struct nsm_handle_attr address_attr	= __ATTR_RO(address);
+static struct nsm_handle_attr rpcparms_attr	= __ATTR_RO(rpcparms);
+static struct nsm_handle_attr status_attr	= __ATTR_RO(status);
+static struct nsm_handle_attr sticky_attr	= __ATTR_RO(sticky);
+static struct nsm_handle_attr refcount_attr	= __ATTR_RO(refcount);
+
+static struct attribute *nsm_kobj_attrs[] = {
+	&mon_name_attr.attr,
+	&my_name_attr.attr,
+	&address_attr.attr,
+	&rpcparms_attr.attr,
+	&status_attr.attr,
+	&sticky_attr.attr,
+	&refcount_attr.attr,
+	NULL,
+};
+
+static struct kobj_type nsm_handle_ktype = {
+	.release		= nsm_kobj_release,
+	.sysfs_ops		= &nsm_kobj_sysfs_ops,
+	.default_attrs		= nsm_kobj_attrs,
+};
+
 static inline struct sockaddr *nsm_addr(const struct nsm_handle *nsm)
 {
 	return (struct sockaddr *)&nsm->sm_addr;
@@ -190,8 +313,10 @@ void nsm_unmonitor(const struct nlm_host *host)
 		if (status < 0)
 			printk(KERN_NOTICE "lockd: cannot unmonitor %s\n",
 					nsm->sm_name);
-		else
+		else {
 			nsm->sm_monitored = 0;
+			nsm->sm_mon_name = NULL;
+		}
 	}
 }
 
@@ -262,7 +387,10 @@ static struct nsm_handle *nsm_create_handle(const struct sockaddr *sap,
 					    const char *hostname,
 					    const size_t hostname_len)
 {
+	char *p, buf[SM_PRIV_SIZE * 2 + 1];
 	struct nsm_handle *new;
+	size_t remaining;
+	int i, len, ret;
 
 	new = kzalloc(sizeof(*new) + hostname_len + 1, GFP_KERNEL);
 	if (unlikely(new == NULL))
@@ -281,7 +409,33 @@ static struct nsm_handle *nsm_create_handle(const struct sockaddr *sap,
 	memcpy(new->sm_name, hostname, hostname_len);
 	new->sm_name[hostname_len] = '\0';
 
+	p = buf;
+	remaining = sizeof(buf);
+	for (i = 0; i < SM_PRIV_SIZE; i++) {
+		len = snprintf(p, remaining, "%02x",
+				(u32)(new->sm_priv.data[i]) & 0xff);
+		if (len != 2)
+			goto out;
+		p += len; 
+		remaining -= (size_t)len;
+	}
+	if (remaining < 1)
+		goto out;
+	strcat(buf, "\0");
+
+	/* Named after "priv" cookie, which is guaranteed unique */
+	ret = kobject_init_and_add(&new->sm_kobj, &nsm_handle_ktype,
+					nsm_handles_kobj, "%s", buf);
+	if (ret != 0)
+		goto out;
+
 	return new;
+
+out:
+	kobject_put(&new->sm_kobj);
+	dprintk("lockd: %s failed; could not add kobject\n",
+			__func__);
+	return NULL;
 }
 
 /**
@@ -324,7 +478,7 @@ retry:
 	if (cached != NULL) {
 		atomic_inc(&cached->sm_count);
 		spin_unlock(&nsm_lock);
-		kfree(new);
+		kobject_put(&new->sm_kobj);
 		dprintk("lockd: found nsm_handle for %s (%s), "
 				"cnt %d\n", cached->sm_name,
 				cached->sm_addrbuf,
@@ -387,11 +541,9 @@ struct nsm_handle *nsm_reboot_lookup(const struct nlm_reboot *info)
 void nsm_release(struct nsm_handle *nsm)
 {
 	if (atomic_dec_and_lock(&nsm->sm_count, &nsm_lock)) {
-		list_del(&nsm->sm_link);
+		list_del(&nsm->sm_link);	/* kobject_del ? */
 		spin_unlock(&nsm_lock);
-		dprintk("lockd: destroyed nsm_handle for %s (%s)\n",
-				nsm->sm_name, nsm->sm_addrbuf);
-		kfree(nsm);
+		kobject_put(&nsm->sm_kobj);
 	}
 }
 
@@ -401,7 +553,7 @@ void nsm_release(struct nsm_handle *nsm)
  */
 int nsm_init(void)
 {
-	nsm_handles_kobj = kobject_create_and_add("monitor", nlm_kobj);
+	nsm_handles_kobj = kobject_create_and_add("nsm_handles", nlm_kobj);
 	if (nsm_handles_kobj == NULL)
 		return -ENOMEM;
 	return 0;
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index 8622a28..3d1c594 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -78,6 +78,7 @@ struct nlm_host {
 #define NSM_ADDRBUF		((8 * 4 + 7) + (1 + 10) + 1)
 
 struct nsm_handle {
+	struct kobject		sm_kobj;
 	struct list_head	sm_link;
 	atomic_t		sm_count;
 	char			*sm_mon_name;

[PATCH 8/9] lockd: Refactor nlm_host_rebooted()

[Chuck Lever]

We want to be able to call the lock recovery logic directly with an
nsm_handle.  Factor that piece out of nlm_host_rebooted().

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/lockd/host.c             |   35 ++++++++++++++++++++++++-----------
 include/linux/lockd/lockd.h |    2 ++
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index 20dab62..17b83b2 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -669,26 +669,39 @@ void nlm_release_host(struct nlm_host *host)
  */
 void nlm_host_rebooted(const struct nlm_reboot *info)
 {
-	struct hlist_head *chain;
-	struct hlist_node *pos;
 	struct nsm_handle *nsm;
-	struct nlm_host	*host;
 
 	nsm = nsm_reboot_lookup(info);
 	if (unlikely(nsm == NULL))
 		return;
 
-	/* Mark all hosts tied to this NSM state as having rebooted.
-	 * We run the loop repeatedly, because we drop the host table
-	 * lock for this.
-	 * To avoid processing a host several times, we match the nsmstate.
-	 */
-again:	mutex_lock(&nlm_host_mutex);
+	nlm_host_rebooted_nsm(nsm, info->state);
+}
+
+/**
+ * nlm_host_rebooted_nsm - start reboot recovery on remote peer
+ * @nsm: pointer to NSM handle for rebooting peer
+ * @nsmstate: new NSM state number of rebooting peer
+ *
+ * Mark all hosts tied to this NSM state as having rebooted.  We run
+ * the loop repeatedly, because we drop the host table lock for this.
+ *
+ * To avoid processing a host several times, we match the nsmstate.
+ */
+void nlm_host_rebooted_nsm(struct nsm_handle *nsm, const int nsmstate)
+{
+	struct hlist_head *chain;
+	struct hlist_node *pos;
+	struct nlm_host	*host;
+
+again:
+	mutex_lock(&nlm_host_mutex);
+
 	for (chain = nlm_hosts; chain < nlm_hosts + NLM_HOST_NRHASH; ++chain) {
 		hlist_for_each_entry(host, pos, chain, h_hash) {
 			if (host->h_nsmhandle == nsm
-			 && host->h_nsmstate != info->state) {
-				host->h_nsmstate = info->state;
+			 && host->h_nsmstate != nsmstate) {
+				host->h_nsmstate = nsmstate;
 				host->h_state++;
 
 				nlm_get_host(host);
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index 3d1c594..1d62ca4 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -235,6 +235,8 @@ void		  nlm_rebind_host(struct nlm_host *);
 struct nlm_host * nlm_get_host(struct nlm_host *);
 void		  nlm_release_host(struct nlm_host *);
 void		  nlm_shutdown_hosts(void);
+void		  nlm_host_rebooted_nsm(struct nsm_handle *nsm,
+					const int nsmstate);
 void		  nlm_host_rebooted(const struct nlm_reboot *);
 
 /*

[PATCH 9/9] lockd: Add "reboot" attribute to nsm_handles

[Chuck Lever]

There can be multiple nlm_host cache entries associated with one
remote peer, but there is only one nsm_handle per peer.  So the
right place to add an API to trigger lock recovery for a particular
remote peer is with the nsm_handle cache entry for that remote.

Add a "reboot" attribute to each subdirectory in nsm_handles/.

Writing the new NSM state into the peer's nsm_handles "reboot"
attribute invokes the kernel's lock recovery for that peer.  A user
space tool, for example, could be constructed so an administrator
can remove stale locks on an NFS server.

Note that this new API can replace statd's NLM downcalls as well.
Instead of calling a specified RPC program/version/procedure with
the new state number and the priv cookie, statd can walk through
/sys/fs/lockd looking for a directory named after the priv cookie,
and write the new state number into its "reboot" attribute.

Using this API instead of an RPC downcall would eliminate one lockd
dependency on IPv4 loopback.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/lockd/mon.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c
index 851ad59..80dccb6 100644
--- a/fs/lockd/mon.c
+++ b/fs/lockd/mon.c
@@ -148,6 +148,23 @@ static ssize_t refcount_show(struct nsm_handle *nsm, char *buf)
 			atomic_read(&nsm->sm_count));
 }
 
+static ssize_t reboot_show(struct nsm_handle *nsm, char *buf)
+{
+	return 0;
+}
+
+static ssize_t reboot_store(struct nsm_handle *nsm, 
+				const char *buf, size_t count)
+{
+	int state;
+
+	if (sscanf(buf, "%d", &state) != 1)
+		return -EINVAL;
+
+	nlm_host_rebooted_nsm(nsm, state);
+	return count;
+}
+
 #ifdef __ATTR_RO
 #undef __ATTR_RO
 #endif
@@ -164,6 +181,8 @@ static struct nsm_handle_attr rpcparms_attr	= __ATTR_RO(rpcparms);
 static struct nsm_handle_attr status_attr	= __ATTR_RO(status);
 static struct nsm_handle_attr sticky_attr	= __ATTR_RO(sticky);
 static struct nsm_handle_attr refcount_attr	= __ATTR_RO(refcount);
+static struct nsm_handle_attr reboot_attr	= __ATTR(reboot,
+			S_IWUSR | S_IRUSR, reboot_show, reboot_store);
 
 static struct attribute *nsm_kobj_attrs[] = {
 	&mon_name_attr.attr,
@@ -173,6 +192,7 @@ static struct attribute *nsm_kobj_attrs[] = {
 	&status_attr.attr,
 	&sticky_attr.attr,
 	&refcount_attr.attr,
+	&reboot_attr.attr,
 	NULL,
 };
 

Re: [PATCH 0/9] user space NFSv2/v3 lock recovery, take 2

[J. Bruce Fields]

On Tue, Apr 27, 2010 at 02:58:09PM -0400, Chuck Lever wrote:
> I've trimmed the patches that Trond objected to last time, and, at
> Bruce's request, added documentation under
> Documentation/filesystems/nfs/ that describes the new API.

Thanks!

--b.

> As an
> addendum to this cover letter, the patch that adds the API
> documentation is first in this series.
> 
> ---
> 
> Chuck Lever (9):
>       lockd: Add "reboot" attribute to nsm_handles
>       lockd: Refactor nlm_host_rebooted()
>       lockd: Add /sys/fs/lockd/nsm_handle/*
>       lockd: Add /sys/fs/lockd/mon
>       lockd: Add attributes to /sys/fs/lockd/hosts/*/
>       lockd: Add /sys/fs/lockd/hosts/*
>       lockd: Add /sys/fs/lockd/hosts/
>       lockd: Add /sys/fs/lockd
>       lockd: Document new lockd user space API
> 
> 
>  Documentation/filesystems/nfs/00-INDEX            |    2 
>  Documentation/filesystems/nfs/lockd-sysfs-api.txt |  117 ++++++++
>  fs/lockd/host.c                                   |  312 ++++++++++++++++++++-
>  fs/lockd/mon.c                                    |  211 ++++++++++++++
>  fs/lockd/svc.c                                    |   31 ++
>  include/linux/lockd/lockd.h                       |    9 +
>  6 files changed, 659 insertions(+), 23 deletions(-)
>  create mode 100644 Documentation/filesystems/nfs/lockd-sysfs-api.txt
> 
> -- 
> Chuck Lever <chuck.lever@oracle.com>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/9] lockd: Add /sys/fs/lockd

[Neil Brown]

On Tue, 27 Apr 2010 14:58:30 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:

> Create the parent directory for all lockd-related sysfs entries.  The
> directory is created when lockd.ko is loaded, and removed when it is
> unloaded.

Can I argue about the colour of the bike shed please?

lockd is not a filesystem, so I would not put it in /sys/fs.

I think lockd has more in common with 'fscache' than with
'fuse' and 'ext4', so I think it should go in /sys/kernel
rather than /sys/fs.

Mind you, this comment is based on the assumption that /sys is different
to /proc in that it is better organised.  But I am beginning to wonder.


I know you would much rather a review of the rest of the series that this one
point and I apologise for not providing one.... maybe I'll motivate myself
sometime.

NeilBrown


> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> 
>  fs/lockd/svc.c              |   16 +++++++++++++---
>  include/linux/lockd/lockd.h |    1 +
>  2 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
> index f1bacf1..c84a7d5 100644
> --- a/fs/lockd/svc.c
> +++ b/fs/lockd/svc.c
> @@ -75,6 +75,8 @@ static const int		nlm_port_min = 0, nlm_port_max = 65535;
>  static struct ctl_table_header * nlm_sysctl_table;
>  #endif
>  
> +struct kobject			*nlm_kobj;
> +
>  static unsigned long get_lockd_grace_period(void)
>  {
>  	/* Note: nlm_timeout should always be nonzero */
> @@ -514,12 +516,19 @@ module_param(nlm_max_connections, uint, 0644);
>  
>  static int __init init_nlm(void)
>  {
> +	nlm_kobj = kobject_create_and_add("lockd", fs_kobj);
> +	if (!nlm_kobj)
> +		return -ENOMEM;
> +
>  #ifdef CONFIG_SYSCTL
>  	nlm_sysctl_table = register_sysctl_table(nlm_sysctl_root);
> -	return nlm_sysctl_table ? 0 : -ENOMEM;
> -#else
> -	return 0;
> +	if (!nlm_sysctl_table) {
> +		kobject_put(nlm_kobj);
> +		return -ENOMEM;
> +	}
>  #endif
> +
> +	return 0;
>  }
>  
>  static void __exit exit_nlm(void)
> @@ -529,6 +538,7 @@ static void __exit exit_nlm(void)
>  #ifdef CONFIG_SYSCTL
>  	unregister_sysctl_table(nlm_sysctl_table);
>  #endif
> +	kobject_put(nlm_kobj);
>  }
>  
>  module_init(init_nlm);
> diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
> index a34dea4..56c749e 100644
> --- a/include/linux/lockd/lockd.h
> +++ b/include/linux/lockd/lockd.h
> @@ -196,6 +196,7 @@ extern int			nlmsvc_grace_period;
>  extern unsigned long		nlmsvc_timeout;
>  extern int			nsm_use_hostnames;
>  extern u32			nsm_local_state;
> +extern struct kobject		*nlm_kobj;
>  
>  /*
>   * Lockd client functions
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/9] lockd: Add /sys/fs/lockd

[Chuck Lever]

On 05/11/2010 11:54 PM, Neil Brown wrote:
> On Tue, 27 Apr 2010 14:58:30 -0400
> Chuck Lever<chuck.lever@oracle.com>  wrote:
>
>> Create the parent directory for all lockd-related sysfs entries.  The
>> directory is created when lockd.ko is loaded, and removed when it is
>> unloaded.
>
> Can I argue about the colour of the bike shed please?
>
> lockd is not a filesystem, so I would not put it in /sys/fs.
>
> I think lockd has more in common with 'fscache' than with
> 'fuse' and 'ext4', so I think it should go in /sys/kernel
> rather than /sys/fs.
>
> Mind you, this comment is based on the assumption that /sys is different
> to /proc in that it is better organised.  But I am beginning to wonder.
>
>
> I know you would much rather a review of the rest of the series that this one
> point and I apologise for not providing one.... maybe I'll motivate myself
> sometime.

I think it's a legitimate comment.  I actually had this under 
/sys/kernel at an earlier point.  I'm somewhat agnostic about it.

> NeilBrown
>
>
>>
>> Signed-off-by: Chuck Lever<chuck.lever@oracle.com>
>> ---
>>
>>   fs/lockd/svc.c              |   16 +++++++++++++---
>>   include/linux/lockd/lockd.h |    1 +
>>   2 files changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
>> index f1bacf1..c84a7d5 100644
>> --- a/fs/lockd/svc.c
>> +++ b/fs/lockd/svc.c
>> @@ -75,6 +75,8 @@ static const int		nlm_port_min = 0, nlm_port_max = 65535;
>>   static struct ctl_table_header * nlm_sysctl_table;
>>   #endif
>>
>> +struct kobject			*nlm_kobj;
>> +
>>   static unsigned long get_lockd_grace_period(void)
>>   {
>>   	/* Note: nlm_timeout should always be nonzero */
>> @@ -514,12 +516,19 @@ module_param(nlm_max_connections, uint, 0644);
>>
>>   static int __init init_nlm(void)
>>   {
>> +	nlm_kobj = kobject_create_and_add("lockd", fs_kobj);
>> +	if (!nlm_kobj)
>> +		return -ENOMEM;
>> +
>>   #ifdef CONFIG_SYSCTL
>>   	nlm_sysctl_table = register_sysctl_table(nlm_sysctl_root);
>> -	return nlm_sysctl_table ? 0 : -ENOMEM;
>> -#else
>> -	return 0;
>> +	if (!nlm_sysctl_table) {
>> +		kobject_put(nlm_kobj);
>> +		return -ENOMEM;
>> +	}
>>   #endif
>> +
>> +	return 0;
>>   }
>>
>>   static void __exit exit_nlm(void)
>> @@ -529,6 +538,7 @@ static void __exit exit_nlm(void)
>>   #ifdef CONFIG_SYSCTL
>>   	unregister_sysctl_table(nlm_sysctl_table);
>>   #endif
>> +	kobject_put(nlm_kobj);
>>   }
>>
>>   module_init(init_nlm);
>> diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
>> index a34dea4..56c749e 100644
>> --- a/include/linux/lockd/lockd.h
>> +++ b/include/linux/lockd/lockd.h
>> @@ -196,6 +196,7 @@ extern int			nlmsvc_grace_period;
>>   extern unsigned long		nlmsvc_timeout;
>>   extern int			nsm_use_hostnames;
>>   extern u32			nsm_local_state;
>> +extern struct kobject		*nlm_kobj;
>>
>>   /*
>>    * Lockd client functions
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
chuck[dot]lever[at]oracle[dot]com