Skip to contentSkip to conversation

dlm: use configfs

PATCH In review David Teigland 5 participants 8 messages Aug 18, 2005 – Aug 23, 2005 1 message not in the archive

lore
Share & tools
lore
#0 lore raw

PATCH 1/3 dlm: use configfs

Use configfs to configure lockspace members and node addresses. This was previously done with sysfs and ioctl.

---

 drivers/dlm/Makefile       |    1 
 drivers/dlm/config.c       |  759 ++++++++++++++++++++++++++++++++++++++++++++-
 drivers/dlm/config.h       |   12 
 drivers/dlm/dlm_internal.h |    2 
 drivers/dlm/lockspace.c    |    7 
 drivers/dlm/lowcomms.c     |  195 +----------
 drivers/dlm/lowcomms.h     |    4 
 drivers/dlm/main.c         |   18 -
 drivers/dlm/member.c       |   40 +-
 drivers/dlm/member_sysfs.c |   76 ----
 drivers/dlm/node_ioctl.c   |  126 -------
 drivers/dlm/requestqueue.c |    2 
 include/linux/dlm_node.h   |   44 --
 13 files changed, 828 insertions(+), 458 deletions(-)
drivers/dlm/Makefile +0 -1
diff -urpN a/drivers/dlm/Makefile b/drivers/dlm/Makefile
@@ -12,7 +12,6 @@ dlm-y :=			ast.o \
 				member_sysfs.o \
 				memory.o \
 				midcomms.o \
-				node_ioctl.o \
 				rcom.o \
 				recover.o \
 				recoverd.o \
drivers/dlm/config.c +748 -11

770 lines not on this page · raw message

drivers/dlm/config.h +10 -2
diff -urpN a/drivers/dlm/config.h b/drivers/dlm/config.h
@@ -14,6 +14,8 @@
 #ifndef __CONFIG_DOT_H__
 #define __CONFIG_DOT_H__
 
+#define DLM_MAX_ADDR_COUNT 3
+
 struct dlm_config_info {
 	int tcp_port;
 	int buffer_size;
@@ -27,8 +29,14 @@ struct dlm_config_info {
 
 extern struct dlm_config_info dlm_config;
 
-extern int dlm_config_init(void);
-extern void dlm_config_exit(void);
+int dlm_config_init(void);
+void dlm_config_exit(void);
+int dlm_node_weight(char *lsname, int nodeid);
+int dlm_nodeid_list(char *lsname, int **ids_out);
+int dlm_nodeid_to_addr(int nodeid, struct sockaddr_storage *addr);
+int dlm_addr_to_nodeid(struct sockaddr_storage *addr, int *nodeid);
+int dlm_our_nodeid(void);
+int dlm_our_addr(struct sockaddr_storage *addr, int num);
 
 #endif				/* __CONFIG_DOT_H__ */
 
drivers/dlm/dlm_internal.h +0 -2
diff -urpN a/drivers/dlm/dlm_internal.h b/drivers/dlm/dlm_internal.h
@@ -457,8 +457,6 @@ struct dlm_ls {
 	int			ls_low_nodeid;
 	int			ls_total_weight;
 	int			*ls_node_array;
-	int			*ls_nodeids_next;
-	int			ls_nodeids_next_count;
 
 	struct dlm_rsb		ls_stub_rsb;	/* for returning errors */
 	struct dlm_lkb		ls_stub_lkb;	/* for returning errors */
drivers/dlm/lockspace.c +5 -2
diff -urpN a/drivers/dlm/lockspace.c b/drivers/dlm/lockspace.c
@@ -94,6 +94,11 @@ static struct dlm_ls *find_lockspace_nam
 	return ls;
 }
 
+struct dlm_ls *dlm_find_lockspace_name(char *name, int namelen)
+{
+	return find_lockspace_name(name, namelen);
+}
+
 struct dlm_ls *dlm_find_lockspace_global(uint32_t id)
 {
 	struct dlm_ls *ls;
@@ -261,8 +266,6 @@ static int new_lockspace(char *name, int
 	ls->ls_low_nodeid = 0;
 	ls->ls_total_weight = 0;
 	ls->ls_node_array = NULL;
-	ls->ls_nodeids_next = NULL;
-	ls->ls_nodeids_next_count = 0;
 
 	memset(&ls->ls_stub_rsb, 0, sizeof(struct dlm_rsb));
 	ls->ls_stub_rsb.res_ls = ls;
drivers/dlm/lowcomms.c +33 -162

264 lines not on this page · raw message

drivers/dlm/lowcomms.h +0 -4
diff -urpN a/drivers/dlm/lowcomms.h b/drivers/dlm/lowcomms.h
@@ -20,10 +20,6 @@ int dlm_lowcomms_start(void);
 void dlm_lowcomms_stop(void);
 void *dlm_lowcomms_get_buffer(int nodeid, int len, int allocation, char **ppc);
 void dlm_lowcomms_commit_buffer(void *mh);
-int dlm_set_node(int nodeid, int weight, char *addr_buf);
-int dlm_set_local(int nodeid, int weight, char *addr_buf);
-int dlm_our_nodeid(void);
-int dlm_node_weight(int nodeid);
 
 #endif				/* __LOWCOMMS_DOT_H__ */
 
drivers/dlm/main.c +8 -10
diff -urpN a/drivers/dlm/main.c b/drivers/dlm/main.c
@@ -18,6 +18,7 @@
 #include "device.h"
 #include "memory.h"
 #include "lowcomms.h"
+#include "config.h"
 
 #ifdef CONFIG_DLM_DEBUG
 int dlm_register_debugfs(void);
@@ -27,9 +28,6 @@ static inline int dlm_register_debugfs(v
 static inline void dlm_unregister_debugfs(void) { }
 #endif
 
-int dlm_node_ioctl_init(void);
-void dlm_node_ioctl_exit(void);
-
 static int __init init_dlm(void)
 {
 	int error;
@@ -42,17 +40,17 @@ static int __init init_dlm(void)
 	if (error)
 		goto out_mem;
 
-	error = dlm_node_ioctl_init();
+	error = dlm_member_sysfs_init();
 	if (error)
 		goto out_mem;
 
-	error = dlm_member_sysfs_init();
+	error = dlm_config_init();
 	if (error)
-		goto out_node;
+		goto out_member;
 
 	error = dlm_register_debugfs();
 	if (error)
-		goto out_member;
+		goto out_config;
 
 	error = dlm_lowcomms_init();
 	if (error)
@@ -64,10 +62,10 @@ static int __init init_dlm(void)
 
  out_debug:
 	dlm_unregister_debugfs();
+ out_config:
+	dlm_config_exit();
  out_member:
 	dlm_member_sysfs_exit();
- out_node:
-	dlm_node_ioctl_exit();
  out_mem:
 	dlm_memory_exit();
  out:
@@ -78,7 +76,7 @@ static void __exit exit_dlm(void)
 {
 	dlm_lowcomms_exit();
 	dlm_member_sysfs_exit();
-	dlm_node_ioctl_exit();
+	dlm_config_exit();
 	dlm_memory_exit();
 	dlm_unregister_debugfs();
 }
drivers/dlm/member.c +22 -18
diff -urpN a/drivers/dlm/member.c b/drivers/dlm/member.c
@@ -11,13 +11,13 @@
 ******************************************************************************/
 
 #include "dlm_internal.h"
-#include "member_sysfs.h"
 #include "lockspace.h"
 #include "member.h"
 #include "recoverd.h"
 #include "recover.h"
 #include "lowcomms.h"
 #include "rcom.h"
+#include "config.h"
 
 /*
  * Following called by dlm_recoverd thread
@@ -50,13 +50,18 @@ static void add_ordered_member(struct dl
 static int dlm_add_member(struct dlm_ls *ls, int nodeid)
 {
 	struct dlm_member *memb;
+	int w;
 
 	memb = kmalloc(sizeof(struct dlm_member), GFP_KERNEL);
 	if (!memb)
 		return -ENOMEM;
 
+	w = dlm_node_weight(ls->ls_name, nodeid);
+	if (w < 0)
+		return w;
+
 	memb->nodeid = nodeid;
-	memb->weight = dlm_node_weight(nodeid);
+	memb->weight = w;
 	add_ordered_member(ls, memb);
 	ls->ls_num_nodes++;
 	return 0;
@@ -262,14 +267,19 @@ int dlm_ls_stop(struct dlm_ls *ls)
 
 int dlm_ls_start(struct dlm_ls *ls)
 {
-	struct dlm_recover *rv, *rv_old;
-	int error = 0;
+	struct dlm_recover *rv = NULL, *rv_old;
+	int *ids = NULL;
+	int error, count;
 
 	rv = kmalloc(sizeof(struct dlm_recover), GFP_KERNEL);
 	if (!rv)
 		return -ENOMEM;
 	memset(rv, 0, sizeof(struct dlm_recover));
 
+	error = count = dlm_nodeid_list(ls->ls_name, &ids);
+	if (error <= 0)
+		goto fail;
+
 	spin_lock(&ls->ls_recover_lock);
 
 	/* the lockspace needs to be stopped before it can be started */
@@ -277,22 +287,12 @@ int dlm_ls_start(struct dlm_ls *ls)
 	if (!dlm_locking_stopped(ls)) {
 		spin_unlock(&ls->ls_recover_lock);
 		log_error(ls, "start ignored: lockspace running");
-		kfree(rv);
-		error = -EINVAL;
-		goto out;
-	}
-
-	if (!ls->ls_nodeids_next) {
-		spin_unlock(&ls->ls_recover_lock);
-		log_error(ls, "start ignored: existing nodeids_next");
-		kfree(rv);
 		error = -EINVAL;
-		goto out;
+		goto fail;
 	}
 
-	rv->nodeids = ls->ls_nodeids_next;
-	ls->ls_nodeids_next = NULL;
-	rv->node_count = ls->ls_nodeids_next_count;
+	rv->nodeids = ids;
+	rv->node_count = count;
 	rv->seq = ++ls->ls_recover_seq;
 	rv_old = ls->ls_recover_args;
 	ls->ls_recover_args = rv;
@@ -304,7 +304,11 @@ int dlm_ls_start(struct dlm_ls *ls)
 	}
 
 	dlm_recoverd_kick(ls);
- out:
+	return 0;
+
+ fail:
+	kfree(rv);
+	kfree(ids);
 	return error;
 }
 
drivers/dlm/member_sysfs.c +1 -75

95 lines not on this page · raw message

drivers/dlm/node_ioctl.c +0 -126

126 lines not on this page · raw message

drivers/dlm/requestqueue.c +1 -1
diff -urpN a/drivers/dlm/requestqueue.c b/drivers/dlm/requestqueue.c
@@ -14,7 +14,7 @@
 #include "member.h"
 #include "lock.h"
 #include "dir.h"
-#include "lowcomms.h"
+#include "config.h"
 
 struct rq_entry {
 	struct list_head list;
include/linux/dlm_node.h +0 -44

44 lines not on this page · raw message

#1 lore raw
Quoted 3 lines · David Teigland

Use configfs to configure lockspace members and node addresses. This was
previously done with sysfs and ioctl.

Fair enough. This really means that the configfs patch should be split out of the ocfs2 megapatch...

A message that is not in the archive (sent off-list or not yet received): 20050818210747.GC22742@insight

#3 lore raw
Quoted 4 lines · Joel Becker
Quoted 2 lines · Andrew Morton

Fair enough. This really means that the configfs patch should be split out of the ocfs2 megapatch...

Quoted 8 lines

Easy to do, it's a separate commit in the ocfs2.git repository.
Would you rather

a) Do the diffs yourself (configfs commit, remaining ocfs2 commits)
b) Have two repositories, configfs.git and ocfs2.git, where ocfs2.git is configfs.git+ocfs2
c) Just take the configfs patch (which really hasn't changed in months)

Well, I included the patch in my last email. For the latest
spin, I've created http://oss.oracle.com/git/configfs.git. The ocfs2 git repositories (http://oss.oracle.com/git/ocfs2-dev.git,
http://oss.oracle.com/git/ocfs2.git) are now based on the configfs one.
If there's any other way you want me to do it, let me know.

Joel

Signature · 6 lines
"If the human brain were so simple we could understand it, we would
 be so simple that we could not."
	- W. A. Clouston

			http://www.jlbec.org/
			[email protected]
#4 lore raw
In reply to David Teigland
Quoted 4 lines · David Teigland

Use configfs to configure lockspace members and node addresses. This was previously done with sysfs and ioctl.

Are you the official maintainer of the DLM subsystem? Could you submit a patch to add a MAINTAINERS entry? I was looking for a maintainer to send the dlm portion of my schedule_timeout() fixes to, but there wasn't one listed.

Thanks,
Nish

#5 lore raw
Quoted 2 lines

Are you the official maintainer of the DLM subsystem? Could you submit a patch to add a MAINTAINERS entry? I was looking for a maintainer to

yes

MAINTAINERS +7 -0
diff -urpN a/MAINTAINERS b/MAINTAINERS
@@ -748,6 +748,13 @@ M:	[email protected]
 L:	[email protected]
 S:	Maintained
 
+DLM, DISTRIBUTED LOCK MANAGER
+P:	David Teigland
+M:	[email protected]
+L:	[email protected]
+W:	http://sources.redhat.com/cluster
+S:	Maintained
+
 DAVICOM FAST ETHERNET (DMFE) NETWORK DRIVER
 P:	Tobias Ringstrom
 M:	[email protected]
#6 lore raw
In reply to David Teigland

Hi David,

Quoted 8 lines · David Teigland

+/*
+ * /config/dlm/<cluster>/spaces/<space>/nodes/<node>/nodeid
+ * /config/dlm/<cluster>/spaces/<space>/nodes/<node>/weight
+ * /config/dlm/<cluster>/comms/<comm>/nodeid
+ * /config/dlm/<cluster>/comms/<comm>/local
+ * /config/dlm/<cluster>/comms/<comm>/addr
+ * The <cluster> level is useless, but I haven't figured out how to avoid it.
+ */

So what happened to factoring out the common parts of
ocfs2_nodemanager? I was quite a big fan of that approach :) Or am I just misunderstanding what these patches do?

	--Mark

--
Mark Fasheh
Senior Software Developer, Oracle
[email protected]

#7 lore raw
Quoted 11 lines · Mark Fasheh
Quoted 5 lines · David Teigland

+ * /config/dlm/<cluster>/spaces/<space>/nodes/<node>/nodeid
+ * /config/dlm/<cluster>/spaces/<space>/nodes/<node>/weight
+ * /config/dlm/<cluster>/comms/<comm>/nodeid
+ * /config/dlm/<cluster>/comms/<comm>/local
+ * /config/dlm/<cluster>/comms/<comm>/addr

So what happened to factoring out the common parts of ocfs2_nodemanager? I was quite a big fan of that approach :) Or am I just misunderstanding what these patches do?

The nodemanager RFC I sent a month ago
http://marc.theaimsgroup.com/?l=linux-kernel&m=112166723919347&w=2

amounts to half of dlm/config.c (everything under comms/ above) moved into a separate kernel module. That would be trivial to do, and is still an option to bat around.

I question whether factoring such a small chunk into a separate module is really worth it, though? Making all of config.c (all of /config/dlm/ above) into a separate module wouldn't seem quite so strange. It would require just a few lines of code to turn it into a stand alone module.

Dave

#8 lore raw
Quoted 6 lines · David Teigland

The nodemanager RFC I sent a month ago
http://marc.theaimsgroup.com/?l=linux-kernel&m=112166723919347&w=2

amounts to half of dlm/config.c (everything under comms/ above) moved into a separate kernel module. That would be trivial to do, and is still an option to bat around.

Yeah ok, so the address/id/local part is still there. As is much of the API to query those attributes.

Quoted 2 lines

I question whether factoring such a small chunk into a separate module is really worth it, though?

IMHO, yes. Mostly because we both have very similar basic requirements there and it seems a waste to have duplicated code (even if it's not a huge amount). Future projects wanting to query basic node information from the kernel could have simply used that API without having to further duplicate code too. That said, I'm not sure it has to be done *now*

Was there anything in my comments which made going forward with that approach difficult for dlm?

Quoted 3 lines

Making all of config.c (all of /config/dlm/ above) into a separate module wouldn't seem quite so strange. It would require just a few lines of code to turn it into a stand alone module.

Without the dlm specifics, right? It's perfectly fine with me if dlm has a couple more attributes that it wants on a node object - OCFS2 simply won't query them.

	--Mark

--
Mark Fasheh
Senior Software Developer, Oracle
[email protected]