[ofa-general] [PATCH] libibmad: remove functions which use pthread

Sasha Khapyorsky sashak at voltaire.com
Wed Dec 31 09:04:13 PST 2008


I looked at implementation of safe_*() functions (safe_smp_query,
safe_smp_set and safe_ca_call) and found that they are not actually
"safe" as declared by its names. The only thread-unsafe thing which
is used there is static 'mad_portid' structure (from rpc.c), but
modification of this structure is not protected by same mutex (actually
not protected at all).

As far as I know nothing uses those safe_*() primitives right now outside
libibmad, so I think it is better to remove this confused functions from
API (with changing library version, etc.).

The primitives madrpc_lock() and madrpc_unlock() are just wrappers to
hidden static pthread mutex which is not controlled by caller
application. I think that it will be more robust for multithreaded
application to use its own synchronization methods (pthread mutex or any
other) for better control. So let's remove madrpc_lock/unlock() too.

Signed-off-by: Sasha Khapyorsky <sashak at voltaire.com>
---
 libibmad/include/infiniband/mad.h |   41 -------------------------------------
 libibmad/libibmad.ver             |    2 +-
 libibmad/src/libibmad.map         |    2 -
 libibmad/src/rpc.c                |   15 -------------
 libibmad/src/sa.c                 |    5 ++-
 5 files changed, 4 insertions(+), 61 deletions(-)

diff --git a/libibmad/include/infiniband/mad.h b/libibmad/include/infiniband/mad.h
index eff6738..89b4be5 100644
--- a/libibmad/include/infiniband/mad.h
+++ b/libibmad/include/infiniband/mad.h
@@ -703,8 +703,6 @@ void *  madrpc_rmpp(ib_rpc_t *rpc, ib_portid_t *dport, ib_rmpp_hdr_t *rmpp,
 void	madrpc_init(char *dev_name, int dev_port, int *mgmt_classes,
 		    int num_classes);
 void	madrpc_save_mad(void *madbuf, int len);
-void	madrpc_lock(void);
-void	madrpc_unlock(void);
 void	madrpc_show_errors(int set);
 
 void *	mad_rpc_open_port(char *dev_name, int dev_port, int *mgmt_classes,
@@ -725,32 +723,6 @@ uint8_t * smp_query_via(void *buf, ib_portid_t *id, unsigned attrid,
 uint8_t * smp_set_via(void *buf, ib_portid_t *id, unsigned attrid, unsigned mod,
 		      unsigned timeout, const void *srcport);
 
-inline static uint8_t *
-safe_smp_query(void *rcvbuf, ib_portid_t *portid, unsigned attrid, unsigned mod,
-	       unsigned timeout)
-{
-	uint8_t *p;
-
-	madrpc_lock();
-	p = smp_query(rcvbuf, portid, attrid, mod, timeout);
-	madrpc_unlock();
-
-	return p;
-}
-
-inline static uint8_t *
-safe_smp_set(void *rcvbuf, ib_portid_t *portid, unsigned attrid, unsigned mod,
-	     unsigned timeout)
-{
-	uint8_t *p;
-
-	madrpc_lock();
-	p = smp_set(rcvbuf, portid, attrid, mod, timeout);
-	madrpc_unlock();
-
-	return p;
-}
-
 /* sa.c */
 uint8_t * sa_call(void *rcvbuf, ib_portid_t *portid, ib_sa_call_t *sa,
 		  unsigned timeout);
@@ -761,19 +733,6 @@ int	ib_path_query(ibmad_gid_t srcgid, ibmad_gid_t destgid, ib_portid_t *sm_id,
 int	ib_path_query_via(const void *srcport, ibmad_gid_t srcgid,
 			  ibmad_gid_t destgid, ib_portid_t *sm_id, void *buf);
 
-inline static uint8_t *
-safe_sa_call(void *rcvbuf, ib_portid_t *portid, ib_sa_call_t *sa,
-	     unsigned timeout)
-{
-	uint8_t *p;
-
-	madrpc_lock();
-	p = sa_call(rcvbuf, portid, sa, timeout);
-	madrpc_unlock();
-
-	return p;
-}
-
 /* resolve.c */
 int	ib_resolve_smlid(ib_portid_t *sm_id, int timeout);
 int	ib_resolve_guid(ib_portid_t *portid, uint64_t *guid,
diff --git a/libibmad/libibmad.ver b/libibmad/libibmad.ver
index 7e93c16..23d2dc2 100644
--- a/libibmad/libibmad.ver
+++ b/libibmad/libibmad.ver
@@ -6,4 +6,4 @@
 # API_REV - advance on any added API
 # RUNNING_REV - advance any change to the vendor files
 # AGE - number of backward versions the API still supports
-LIBVERSION=5:0:4
+LIBVERSION=2:0:0
diff --git a/libibmad/src/libibmad.map b/libibmad/src/libibmad.map
index 927e51c..f944d86 100644
--- a/libibmad/src/libibmad.map
+++ b/libibmad/src/libibmad.map
@@ -72,14 +72,12 @@ IBMAD_1.3 {
 		madrpc;
 		madrpc_def_timeout;
 		madrpc_init;
-		madrpc_lock;
 		madrpc_portid;
 		madrpc_rmpp;
 		madrpc_save_mad;
 		madrpc_set_retries;
 		madrpc_set_timeout;
 		madrpc_show_errors;
-		madrpc_unlock;
 		ib_path_query;
 		sa_call;
 		sa_rpc_call;
diff --git a/libibmad/src/rpc.c b/libibmad/src/rpc.c
index 5226540..670a936 100644
--- a/libibmad/src/rpc.c
+++ b/libibmad/src/rpc.c
@@ -38,7 +38,6 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <unistd.h>
-#include <pthread.h>
 #include <string.h>
 #include <errno.h>
 
@@ -286,20 +285,6 @@ madrpc_rmpp(ib_rpc_t *rpc, ib_portid_t *dport, ib_rmpp_hdr_t *rmpp, void *data)
 	return mad_rpc_rmpp(&port, rpc, dport, rmpp, data);
 }
 
-static pthread_mutex_t rpclock = PTHREAD_MUTEX_INITIALIZER;
-
-void
-madrpc_lock(void)
-{
-	pthread_mutex_lock(&rpclock);
-}
-
-void
-madrpc_unlock(void)
-{
-	pthread_mutex_unlock(&rpclock);
-}
-
 void
 madrpc_init(char *dev_name, int dev_port, int *mgmt_classes, int num_classes)
 {
diff --git a/libibmad/src/sa.c b/libibmad/src/sa.c
index 27b9d52..c601254 100644
--- a/libibmad/src/sa.c
+++ b/libibmad/src/sa.c
@@ -132,7 +132,7 @@ ib_path_query_via(const void *srcport, ibmad_gid_t srcgid, ibmad_gid_t destgid,
 	if (srcport) {
 		p = sa_rpc_call (srcport, buf, sm_id, &sa, 0);
 	} else {
-		p = safe_sa_call(buf, sm_id, &sa, 0);
+		p = sa_call(buf, sm_id, &sa, 0);
 	}
 	if (!p) {
 		IBWARN("sa call path_query failed");
@@ -142,8 +142,9 @@ ib_path_query_via(const void *srcport, ibmad_gid_t srcgid, ibmad_gid_t destgid,
 	mad_decode_field(p, IB_SA_PR_DLID_F, &dlid);
 	return dlid;
 }
+
 int
 ib_path_query(ibmad_gid_t srcgid, ibmad_gid_t destgid, ib_portid_t *sm_id, void *buf)
 {
-	return ib_path_query_via (NULL, srcgid, destgid, sm_id, buf);
+	return ib_path_query_via(NULL, srcgid, destgid, sm_id, buf);
 }
-- 
1.6.0.4.766.g6fc4a




More information about the general mailing list