Re: Fear and loathing in ipcname_gettotal.
John Hughes <[email protected]> Thu, 26 Mar 2009 17:35:38 +0100
| Newsgroups | gmane.linux.cluster.ssic.devel |
|---|---|
| Message-ID | <[email protected]> |
Here is a patch to clean up the ipcname_gettotal ugliness.
After this patch it is the job of callers of ipcname_gettotal (or
cli_ipcname_gettotal) to allocate the memory. We avoid the memory leak
where a caller allocates the memory then calls ipcname_gettotal that
reallocates it.
We also pass the (node, key) lists around as a nice struct instead of
some int's bashed into an array of chars.
Before in ipc/shm.c:
if (!local_view) {
size = -1;
cli_ipcname_gettotal(NAME_SERVICE_SHM, &node_id_pairs, &size);
if (size <= 0) goto done;
node_id_pairs = (char *)kmalloc(size * sizeof(int), GFP_KERNEL);
if (node_id_pairs == NULL)
goto done;
memset(node_id_pairs, 0, size * sizeof(int));
cli_ipcname_gettotal(NAME_SERVICE_SHM, &node_id_pairs, &size);
tmp_pairs = (char *)node_id_pairs;
}
id_count = SHM_MAX_ID;
for(i = 0; i <= id_count; i++) {
struct shmid_kernel* shp;
shp = NULL;
if (!local_view) {
node_num = *((int *)tmp_pairs);
tmp_pairs += sizeof(int);
ipc_id = *((key_t *)tmp_pairs);
tmp_pairs += sizeof(key_t);
i++;
(Notice that sneaky little i++ there? It's because ipcname_gettotal
returns the number of int's, not the number of (node,key) pairs).
After:
if (!local_view) {
size = 30; /* Random guess */
for (;;) {
int allocated = size;
node_id_pairs = kmalloc(size * sizeof *node_id_pairs, GFP_KERNEL);
if (!node_id_pairs) goto done;
cli_ipcname_gettotal(NAME_SERVICE_SHM, &node_id_pairs, &size);
if (size <= allocated) break;
kfree (node_id_pairs);
}
}
id_count = SHM_MAX_ID;
for(i = 0; i <= id_count; i++) {
struct shmid_kernel* shp;
key_t ipc_id = i;
int segsize=0, cprid=0;
shp = NULL;
if (!local_view) {
ipc_id = node_id_pairs[i].ipc_id;
node_num = node_id_pairs[i].node_num;
------------------------------------------------------------------------------
_______________________________________________
ssic-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ssic-linux-devel
ci-ipcname_gettotal.patch
(text/x-patch, 545 B)
Index: kernel/include/cluster/gen/ics_proto_gen.h.list =================================================================== RCS file: /usr/local/lib/cvs-repo/sourceforge-ci/kernel/include/cluster/gen/ics_proto_gen.h.list,v retrieving revision 1.9 diff -u -r1.9 ics_proto_gen.h.list --- kernel/include/cluster/gen/ics_proto_gen.h.list 5 Mar 2005 02:43:04 -0000 1.9 +++ kernel/include/cluster/gen/ics_proto_gen.h.list 26 Mar 2009 16:19:27 -0000 @@ -84,6 +84,7 @@ pusage_dev rmtfb_svrhandle ssipty_entry +ssi_nodeid_pair ucred vproc_caredata
openssi-ipcname_gettotal.patch
(text/x-patch, 15 KB)
Index: kernel/cluster/ssi/ipc/namesvr_clnt.c
===================================================================
RCS file: /usr/local/lib/cvs-repo/sourceforge-openssi/kernel/cluster/ssi/ipc/namesvr_clnt.c,v
retrieving revision 1.6
diff -u -r1.6 namesvr_clnt.c
--- kernel/cluster/ssi/ipc/namesvr_clnt.c 7 Aug 2007 03:06:09 -0000 1.6
+++ kernel/cluster/ssi/ipc/namesvr_clnt.c 26 Mar 2009 13:53:17 -0000
@@ -46,7 +46,7 @@
extern int ipcname_getid(int service, key_t key, int in_flag,
global_id_t *glid, clusternode_t *server, int view, int *size);
-extern int ipcname_gettotal(int service, char **node_id_pairs, int *size);
+extern int ipcname_gettotal(int service, struct ssi_nodeid_pair *node_id_pairs, int *size);
extern int ipcname_findid(int, global_id_t, key_t *, clusternode_t *,
int *, int *, int *);
@@ -185,7 +185,7 @@
return status;
}
-int cli_ipcname_gettotal(int service, char **node_id_pairs, int *sz)
+int cli_ipcname_gettotal(int service, struct ssi_nodeid_pair **node_id_pairs, int *sz)
{
int status;
clusternode_t server_node;
@@ -200,14 +200,14 @@
{
if (ipcname_failover_flag)
clms_waitfor_key_service(ipc_key_service);
- status = ipcname_gettotal(service, node_id_pairs, sz);
+ status = ipcname_gettotal(service, *node_id_pairs, sz);
}
else
{
ssi_procstate_t pstate;
- if (*sz != -1)
- len = (*sz) * sizeof(int);
+ if (*sz > 0)
+ len = *sz * sizeof(struct ssi_nodeid_pair);
ssi_procstate_get(&pstate);
status = RIPC_IPCNAME_GETTOTAL(server_node,&rval,service,&pstate, node_id_pairs, &len, sz);
Index: kernel/cluster/ssi/ipc/namesvr_func.c
===================================================================
RCS file: /usr/local/lib/cvs-repo/sourceforge-openssi/kernel/cluster/ssi/ipc/namesvr_func.c,v
retrieving revision 1.13.2.1
diff -u -r1.13.2.1 namesvr_func.c
--- kernel/cluster/ssi/ipc/namesvr_func.c 9 Mar 2009 11:33:22 -0000 1.13.2.1
+++ kernel/cluster/ssi/ipc/namesvr_func.c 26 Mar 2009 13:52:00 -0000
@@ -597,12 +597,11 @@
}
int
ipcname_gettotal(int service,
- char **node_id_pairs, /* [OUT] nodenum-id pairs */
+ struct ssi_nodeid_pair *node_id_pairs, /* [OUT] nodenum-id pairs */
int *size) /* [INOUT] the number of ipc structs */
{
ipc_obj_db_t *odbp;
- char *buf;
- int idx=0, count;
+ int ix, ex;
odbp = &nsc_name_odb[service];
#ifdef NSC_IPC_RWLOCK_DOWNGRADE
@@ -613,33 +612,18 @@
#ifdef TEST_IPC
printk("%s\n","Message nameserver locked in getid");
#endif
- for (count=0; count < odbp->iodb_size; count++) {
- if (odbp->iodb_active[count] != NULL)
- idx = idx + 2;
- }
- if ((idx <= 0) || (*size == -1)) goto done;
-
- *node_id_pairs = (char*)kmalloc(idx * sizeof(int),GFP_KERNEL);
- if (*node_id_pairs == NULL)
- {
- idx = 0;
- goto done;
- }
- buf = *node_id_pairs;
- for (count=0; count < odbp->iodb_size; count++) {
- if (odbp->iodb_active[count] == NULL)
+ for (ix = 0, ex = -1; ix < odbp->iodb_size; ix++) {
+ if (odbp->iodb_active[ix] == NULL)
continue;
- *((int *)buf) = odbp->iodb_active[count]->svr_node;
- buf += sizeof(int);
- *((int *)buf) = odbp->iodb_active[count]->io_id;
- buf += sizeof(int);
+ if (++ex >= *size) continue;
+ node_id_pairs[ex].node_num = odbp->iodb_active[ix]->svr_node;
+ node_id_pairs[ex].ipc_id = odbp->iodb_active[ix]->io_id;
}
-done:
#ifdef NSC_IPC_RWLOCK_DOWNGRADE
NSC_IPC_RDUNLOCK(odbp);
- *size = idx;
+ *size = ex + 1;
#else
- *size = idx;
+ *size = ex + 1;
NSC_IPC_WRUNLOCK(odbp);
#endif
#ifdef TEST_IPC
Index: kernel/cluster/ssi/ipc/namesvr_svr.c
===================================================================
RCS file: /usr/local/lib/cvs-repo/sourceforge-openssi/kernel/cluster/ssi/ipc/namesvr_svr.c,v
retrieving revision 1.5
diff -u -r1.5 namesvr_svr.c
--- kernel/cluster/ssi/ipc/namesvr_svr.c 7 Aug 2007 03:06:09 -0000 1.5
+++ kernel/cluster/ssi/ipc/namesvr_svr.c 26 Mar 2009 15:44:23 -0000
@@ -36,7 +36,7 @@
int ipcname_failover_flag = 0;
extern int ipcname_getid(int, key_t, int, u_long *, clusternode_t*, int, int*);
-extern int ipcname_gettotal(int service, char **node_id_pairs, int *size);
+extern int ipcname_gettotal(int service, struct ssi_nodeid_pair *node_id_pairs, int *size);
extern int ipcname_rmid(int, long, int);
extern int ipcname_dumpinfo (int, int, int, int, int*, int*, int*, time_t*);
extern int ipcname_findid(int, u_long, key_t *, clusternode_t*, int *, int *, int *);
@@ -82,11 +82,11 @@
}
/*
- * This function trys to calculate the total number of given ipc structures
+ * This function tries to calculate the total number of given ipc structures
* in the name server.
*/
void
-ripc_ipcname_gettotal(clusternode_t *node, int *rval, int service, ssi_procstate_t *pstate, char **node_id_pairs, int *len, int *sz)
+ripc_ipcname_gettotal(clusternode_t *node, int *rval, int service, ssi_procstate_t *pstate, struct ssi_nodeid_pair **node_id_pairs, int *len, int *sz)
{
ssi_procstate_t save_pstate;
int count = *sz;
@@ -99,9 +99,20 @@
*len = 0;
ssi_procstate_get(&save_pstate);
ssi_procstate_set(pstate);
- *rval = ipcname_gettotal(service, node_id_pairs, sz);
- if (count > 0)
- *len = (*sz) * sizeof(int);
+ if (count > 0) {
+ /* Where will this be freed? */
+ *node_id_pairs = kmalloc (count * sizeof **node_id_pairs, GFP_KERNEL);
+ if (*node_id_pairs == NULL) {
+ *sz = 0;
+ goto done;
+ }
+ }
+ *rval = ipcname_gettotal(service, *node_id_pairs, sz);
+ if (count > 0) {
+ if (count > *sz) count = *sz;
+ *len = count * sizeof **node_id_pairs;
+ }
+ done:
ssi_procstate_set(&save_pstate);
}
/*
Index: kernel/include/cluster/gen/ipc.svc
===================================================================
RCS file: /usr/local/lib/cvs-repo/sourceforge-openssi/kernel/include/cluster/gen/ipc.svc,v
retrieving revision 1.2
diff -u -r1.2 ipc.svc
--- kernel/include/cluster/gen/ipc.svc 5 Mar 2005 02:41:14 -0000 1.2
+++ kernel/include/cluster/gen/ipc.svc 26 Mar 2009 10:55:18 -0000
@@ -95,7 +95,7 @@
operation ripc_ipcname_gettotal {
param IN int service
param IN ssi_procstate_t *pstate
- param OUT:OOL:VAR char **node_id_pairs
+ param OUT:OOL:VAR struct ssi_nodeid_pair **node_id_pairs
param INOUT int *sz
}
Index: kernel/include/linux/ipc.h
===================================================================
RCS file: /usr/local/lib/cvs-repo/sourceforge-openssi/kernel/include/linux/ipc.h,v
retrieving revision 1.2
diff -u -r1.2 ipc.h
--- kernel/include/linux/ipc.h 20 Oct 2004 03:23:36 -0000 1.2
+++ kernel/include/linux/ipc.h 26 Mar 2009 10:53:42 -0000
@@ -71,6 +71,15 @@
void *security;
};
+#ifdef CONFIG_SSI
+/* Return from ipcname_gettotal - mapping between ipc id and server node */
+struct ssi_nodeid_pair
+{
+ int node_num; /* clusternode_t */
+ key_t ipc_id;
+};
+#endif
+
#endif /* __KERNEL__ */
#endif /* _LINUX_IPC_H */
Index: kernel/ipc/msg.c
===================================================================
RCS file: /usr/local/lib/cvs-repo/sourceforge-openssi/kernel/ipc/msg.c,v
retrieving revision 1.12.2.1
diff -u -r1.12.2.1 msg.c
--- kernel/ipc/msg.c 15 Oct 2008 14:34:44 -0000 1.12.2.1
+++ kernel/ipc/msg.c 26 Mar 2009 13:41:52 -0000
@@ -40,7 +40,7 @@
extern int cli_ipcname_findid(int, int, key_t *, clusternode_t *, int *,
int *, int *);
-extern int cli_ipcname_gettotal(int objtype, char **, int *);
+extern int cli_ipcname_gettotal(int objtype, struct ssi_nodeid_pair **, int *);
extern int cli_ipcname_rmid(int service, global_id_t glid);
extern int cli_ripc_msgctl(clusternode_t, int *, int, int, ssi_procstate_t *,
long *);
@@ -1475,26 +1475,27 @@
#ifdef CONFIG_SSI
int local_view = ssi_get_localview();
char viewstr[10];
+ int id_count;
+ struct ssi_nodeid_pair *node_id_pairs=NULL;
+ int size, node_num=0;
#endif
down(&msg_ids.sem);
PRINT_HEADER;
#ifdef CONFIG_SSI
- int id_count=0, ipc_id=0;
- char *node_id_pairs=NULL;
- char *tmp_pairs=NULL;
- int size=0, node_num=0;
bzero(viewstr, 10);
if (!local_view)
{
- size = -1;
- cli_ipcname_gettotal(NAME_SERVICE_MSG, &node_id_pairs, &size);
- node_id_pairs = (char *)kmalloc(size * sizeof(int), GFP_KERNEL);
- if (node_id_pairs == NULL)
- goto done;
- memset(node_id_pairs, 0, size * sizeof(int));
- cli_ipcname_gettotal(NAME_SERVICE_MSG, &node_id_pairs, &size);
- tmp_pairs = (char *)node_id_pairs;
+ size = 30; /* Random guess */
+ for (;;) {
+ int allocated = size;
+ node_id_pairs = kmalloc(size * sizeof *node_id_pairs, GFP_KERNEL);
+ if (node_id_pairs == NULL)
+ goto done;
+ cli_ipcname_gettotal(NAME_SERVICE_MSG, &node_id_pairs, &size);
+ if (allocated >= size) break;
+ kfree (node_id_pairs);
+ }
}
id_count = MSG_MAX_ID;
@@ -1504,15 +1505,13 @@
#endif
struct msg_queue * msq;
#ifdef CONFIG_SSI
+ key_t ipc_id = i;
msq = NULL;
if (!local_view)
{
- node_num = *((int *)tmp_pairs);
- tmp_pairs += sizeof(int);
- ipc_id = *((key_t *)tmp_pairs);
- tmp_pairs += sizeof(key_t);
- i++;
+ ipc_id = node_id_pairs[i].ipc_id;
+ node_num = node_id_pairs[i].node_num;
if (node_num == CLUSTERNODE_INVAL)
node_num = -1;
else if (node_num == this_node)
@@ -1581,6 +1580,9 @@
*eof = 1;
done:
up(&msg_ids.sem);
+#ifdef CONFIG_SSI
+ kfree (node_id_pairs);
+#endif
*start = buffer + (offset - begin);
len -= (offset - begin);
if(len > length)
Index: kernel/ipc/sem.c
===================================================================
RCS file: /usr/local/lib/cvs-repo/sourceforge-openssi/kernel/ipc/sem.c,v
retrieving revision 1.26.2.2
diff -u -r1.26.2.2 sem.c
--- kernel/ipc/sem.c 3 Feb 2009 22:10:38 -0000 1.26.2.2
+++ kernel/ipc/sem.c 26 Mar 2009 13:34:17 -0000
@@ -147,7 +147,7 @@
ssi_procstate_t *, ics_userbuf_t *, ics_userbuf_t *);
extern int cli_ripc_semexit(clusternode_t, int *, ssi_procstate_t *, int,
pid_t);
-extern int cli_ipcname_gettotal(int, char **, int *);
+extern int cli_ipcname_gettotal(int, struct ssi_nodeid_pair **, int *);
extern int ssi_sem_get_sem_array(clusternode_t, int, char **);
extern clusternode_t name_server_node;
@@ -2124,54 +2124,50 @@
#ifdef CONFIG_SSI
int local_view = ssi_get_localview();
char viewstr[10];
+ int id_count;
+ struct ssi_nodeid_pair *node_id_pairs=NULL;
+ int size;
#endif /* CONFIG_SSI */
PRINT_HEADER;
down(&sem_ids.sem);
#ifdef CONFIG_SSI
- int id_count=0, ipc_id=0;
- char *node_id_pairs=NULL;
- char *tmp_pairs=NULL;
- int size=0, node_num=0;
- char *tsma=NULL;
bzero(viewstr, 10);
if (!local_view)
{
- size = -1;
- cli_ipcname_gettotal(NAME_SERVICE_SEM, &node_id_pairs, &size);
- node_id_pairs = (char *)kmalloc(size * sizeof(int), GFP_KERNEL);
- if (node_id_pairs == NULL)
- goto done;
- memset(node_id_pairs, 0, size * sizeof(int));
- cli_ipcname_gettotal(NAME_SERVICE_SEM, &node_id_pairs, &size);
- tmp_pairs = (char *)node_id_pairs;
+ size = 30; /* Random guess */
+ for (;;) {
+ int allocated = size;
+ node_id_pairs = kmalloc(size * sizeof *node_id_pairs, GFP_KERNEL);
+ if (node_id_pairs == NULL)
+ goto done;
+ cli_ipcname_gettotal(NAME_SERVICE_SEM, &node_id_pairs, &size);
+ if (size <= allocated) break;
+ kfree (node_id_pairs);
+ }
}
id_count = SEM_MAX_ID;
for(i = 0; i <= id_count; i++) {
struct sem_array *sma;
- sma = NULL;
+ int node_num;
+ key_t ipc_id;
if (!local_view)
{
- node_num = *((int *)tmp_pairs);
- tmp_pairs += sizeof(int);
- ipc_id = *((key_t *)tmp_pairs);
- tmp_pairs += sizeof(key_t);
- i++;
+ ipc_id = node_id_pairs[i].ipc_id;
+ node_num = node_id_pairs[i].node_num;
if (node_num == CLUSTERNODE_INVAL)
node_num = -1;
else if (node_num == this_node)
sma = sem_lock(ipc_id);
else
{
- tsma = (char *)kmalloc(sizeof(struct sem_array), GFP_KERNEL);
- if (tsma == NULL) break;
- memset(tsma, 0, sizeof(struct sem_array));
- ssi_sem_get_sem_array(node_num, ipc_id, &tsma);
- sma = (struct sem_array *)tsma;
+ sma = kmalloc(sizeof(struct sem_array), GFP_KERNEL);
+ if (sma == NULL) break;
+ ssi_sem_get_sem_array(node_num, ipc_id, (char **) &sma);
}
}
else
@@ -2213,6 +2209,7 @@
goto done;
}
}
+
#else
for(i = 0; i <= sem_ids.max_id; i++) {
struct sem_array *sma;
@@ -2243,6 +2240,9 @@
*eof = 1;
done:
up(&sem_ids.sem);
+#ifdef CONFIG_SSI
+ kfree (node_id_pairs);
+#endif
*start = buffer + (offset - begin);
len -= (offset - begin);
if(len > length)
Index: kernel/ipc/shm.c
===================================================================
RCS file: /usr/local/lib/cvs-repo/sourceforge-openssi/kernel/ipc/shm.c,v
retrieving revision 1.20.2.2
diff -u -r1.20.2.2 shm.c
--- kernel/ipc/shm.c 9 Mar 2009 11:33:23 -0000 1.20.2.2
+++ kernel/ipc/shm.c 26 Mar 2009 13:39:43 -0000
@@ -45,7 +45,7 @@
#ifdef CONFIG_SSI
extern int cli_ipcname_findid(int, int, key_t *, clusternode_t *, int *, int *, int *);
-extern int cli_ipcname_gettotal(int objtype, char **, int *);
+extern int cli_ipcname_gettotal(int objtype, struct ssi_nodeid_pair **, int *);
extern int cli_ipcname_rmid(int, global_id_t);
extern int ssi_shmctl(clusternode_t, int, int, struct shmid_ds *);
extern int ssi_shm_get_shmid_kernel(clusternode_t, int, char **, int *, int *);
@@ -1609,29 +1609,27 @@
#ifdef CONFIG_SSI
int local_view = ssi_get_localview();
char viewstr[10];
+ int id_count=0;
+ struct ssi_nodeid_pair *node_id_pairs = NULL;
+ int size=0, node_num=0;
#endif /* CONFIG_SSI */
down(&shm_ids.sem);
PRINT_HEADER;
#ifdef CONFIG_SSI
- int id_count=0, ipc_id=0;
- int segsize=0, cprid=0;
- char *node_id_pairs=NULL;
- char *tmp_pairs=NULL;
- int size=0, node_num=0;
-
bzero(viewstr, 10);
if (!local_view) {
- size = -1;
- cli_ipcname_gettotal(NAME_SERVICE_SHM, &node_id_pairs, &size);
- if (size <= 0) goto done;
- node_id_pairs = (char *)kmalloc(size * sizeof(int), GFP_KERNEL);
- if (node_id_pairs == NULL)
- goto done;
- memset(node_id_pairs, 0, size * sizeof(int));
- cli_ipcname_gettotal(NAME_SERVICE_SHM, &node_id_pairs, &size);
- tmp_pairs = (char *)node_id_pairs;
+ size = 30; /* Random guess */
+
+ for (;;) { /* Maybe limit tries? */
+ int allocated = size;
+ node_id_pairs = kmalloc(size * sizeof *node_id_pairs, GFP_KERNEL);
+ if (!node_id_pairs) goto done;
+ cli_ipcname_gettotal(NAME_SERVICE_SHM, &node_id_pairs, &size);
+ if (size <= allocated) break;
+ kfree (node_id_pairs);
+ }
}
id_count = SHM_MAX_ID;
@@ -1641,13 +1639,12 @@
#endif /* CONFIG_SSI */
struct shmid_kernel* shp;
#ifdef CONFIG_SSI
+ key_t ipc_id = i;
+ int segsize=0, cprid=0;
shp = NULL;
if (!local_view) {
- node_num = *((int *)tmp_pairs);
- tmp_pairs += sizeof(int);
- ipc_id = *((key_t *)tmp_pairs);
- tmp_pairs += sizeof(key_t);
- i++;
+ ipc_id = node_id_pairs[i].ipc_id;
+ node_num = node_id_pairs[i].node_num;
if (node_num == CLUSTERNODE_INVAL)
node_num = -1;
else if (node_num == this_node)
@@ -1751,16 +1748,15 @@
*eof = 1;
done:
up(&shm_ids.sem);
+#ifdef CONFIG_SSI
+ kfree(node_id_pairs);
+#endif /* CONFIG_SSI */
*start = buffer + (offset - begin);
len -= (offset - begin);
if(len > length)
len = length;
if(len < 0)
len = 0;
-#ifdef CONFIG_SSI
- if ((!local_view) && (size > 0))
- kfree(node_id_pairs);
-#endif /* CONFIG_SSI */
return len;
}
#endif