Linux RDMA and InfiniBand development
 help / color / mirror / Atom feed
* [PATCH 05/13] ibacm: remove unused helper beginthread
From: Christoph Hellwig @ 2016-10-17 19:11 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476731482-26491-1-git-send-email-hch-jcswGhMUV9g@public.gmane.org>

Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
---
 ibacm/linux/osd.h | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/ibacm/linux/osd.h b/ibacm/linux/osd.h
index 7ce52e6..00a5084 100644
--- a/ibacm/linux/osd.h
+++ b/ibacm/linux/osd.h
@@ -130,10 +130,4 @@ static inline uint64_t time_stamp_us(void)
 #define time_stamp_sec() (time_stamp_ms() / (uint64_t) 1000)
 #define time_stamp_min() (time_stamp_sec() / (uint64_t) 60)
 
-static inline int beginthread(void (*func)(void *), void *arg)
-{
-	pthread_t thread;
-	return pthread_create(&thread, NULL, (void *(*)(void*)) func, arg);
-}
-
 #endif /* OSD_H */
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 04/13] ibacm: use the unix socket API directly
From: Christoph Hellwig @ 2016-10-17 19:11 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476731482-26491-1-git-send-email-hch-jcswGhMUV9g@public.gmane.org>

Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
---
 ibacm/linux/osd.h  |  6 -----
 ibacm/src/acm.c    | 66 +++++++++++++++++++++++++++---------------------------
 ibacm/src/libacm.c | 16 ++++++-------
 3 files changed, 41 insertions(+), 47 deletions(-)

diff --git a/ibacm/linux/osd.h b/ibacm/linux/osd.h
index 31a7044..7ce52e6 100644
--- a/ibacm/linux/osd.h
+++ b/ibacm/linux/osd.h
@@ -118,12 +118,6 @@ static inline int event_wait(event_t *e, int timeout)
 #define osd_init()  0
 #define osd_close()
 
-#define SOCKET         int
-#define SOCKET_ERROR   -1
-#define INVALID_SOCKET -1
-#define socket_errno() errno
-#define closesocket    close
-
 static inline uint64_t time_stamp_us(void)
 {
 	struct timeval curtime;
diff --git a/ibacm/src/acm.c b/ibacm/src/acm.c
index 33fc044..508148a 100644
--- a/ibacm/src/acm.c
+++ b/ibacm/src/acm.c
@@ -141,7 +141,7 @@ struct acmc_ep {
 
 struct acmc_client {
 	pthread_mutex_t lock;   /* acquire ep lock first */
-	SOCKET   sock;
+	int      sock;
 	int      index;
 	atomic_t refcnt;
 };
@@ -180,8 +180,8 @@ static struct acmc_prov *def_provider = NULL;
 
 static DLIST_ENTRY dev_list;
 
-static SOCKET listen_socket;
-static SOCKET ip_mon_socket;
+static int listen_socket;
+static int ip_mon_socket;
 static struct acmc_client client_array[FD_SETSIZE - 1];
 
 static FILE *flog;
@@ -195,7 +195,7 @@ static struct acmc_ep *acm_find_ep(struct acmc_port *port, uint16_t pkey);
 static int acm_ep_insert_addr(struct acmc_ep *ep, const char *name, uint8_t *addr,
 			      size_t addr_len, uint8_t addr_type);
 static void acm_event_handler(struct acmc_device *dev);
-static int acm_nl_send(SOCKET sock, struct acm_msg *msg);
+static int acm_nl_send(int sock, struct acm_msg *msg);
 
 static struct sa_data {
 	int		timeout;
@@ -484,7 +484,7 @@ int acm_resolve_response(uint64_t id, struct acm_msg *msg)
 		atomic_inc(&counter[ACM_CNTR_ERROR]);
 
 	pthread_mutex_lock(&client->lock);
-	if (client->sock == INVALID_SOCKET) {
+	if (client->sock == -1) {
 		acm_log(0, "ERROR - connection lost\n");
 		ret = ACM_STATUS_ENOTCONN;
 		goto release;
@@ -525,7 +525,7 @@ int acm_query_response(uint64_t id, struct acm_msg *msg)
 
 	acm_log(2, "status 0x%x\n", msg->hdr.status);
 	pthread_mutex_lock(&client->lock);
-	if (client->sock == INVALID_SOCKET) {
+	if (client->sock == -1) {
 		acm_log(0, "ERROR - connection lost\n");
 		ret = ACM_STATUS_ENOTCONN;
 		goto release;
@@ -559,7 +559,7 @@ static void acm_init_server(void)
 	for (i = 0; i < FD_SETSIZE - 1; i++) {
 		pthread_mutex_init(&client_array[i].lock, NULL);
 		client_array[i].index = i;
-		client_array[i].sock = INVALID_SOCKET;
+		client_array[i].sock = -1;
 		atomic_init(&client_array[i].refcnt);
 	}
 
@@ -578,24 +578,24 @@ static int acm_listen(void)
 
 	acm_log(2, "\n");
 	listen_socket = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
-	if (listen_socket == INVALID_SOCKET) {
+	if (listen_socket == -1) {
 		acm_log(0, "ERROR - unable to allocate listen socket\n");
-		return socket_errno();
+		return errno;
 	}
 
 	memset(&addr, 0, sizeof addr);
 	addr.sin_family = AF_INET;
 	addr.sin_port = htons(server_port);
 	ret = bind(listen_socket, (struct sockaddr *) &addr, sizeof addr);
-	if (ret == SOCKET_ERROR) {
+	if (ret == -1) {
 		acm_log(0, "ERROR - unable to bind listen socket\n");
-		return socket_errno();
+		return errno;
 	}
 	
 	ret = listen(listen_socket, 0);
-	if (ret == SOCKET_ERROR) {
+	if (ret == -1) {
 		acm_log(0, "ERROR - unable to start listen\n");
-		return socket_errno();
+		return errno;
 	}
 
 	acm_log(2, "listen active\n");
@@ -606,20 +606,20 @@ static void acm_disconnect_client(struct acmc_client *client)
 {
 	pthread_mutex_lock(&client->lock);
 	shutdown(client->sock, SHUT_RDWR);
-	closesocket(client->sock);
-	client->sock = INVALID_SOCKET;
+	close(client->sock);
+	client->sock = -1;
 	pthread_mutex_unlock(&client->lock);
 	(void) atomic_dec(&client->refcnt);
 }
 
 static void acm_svr_accept(void)
 {
-	SOCKET s;
+	int s;
 	int i;
 
 	acm_log(2, "\n");
 	s = accept(listen_socket, NULL, NULL);
-	if (s == INVALID_SOCKET) {
+	if (s == -1) {
 		acm_log(0, "ERROR - failed to accept connection\n");
 		return;
 	}
@@ -633,7 +633,7 @@ static void acm_svr_accept(void)
 
 	if (i == FD_SETSIZE - 1) {
 		acm_log(0, "ERROR - all connections busy - rejecting\n");
-		closesocket(s);
+		close(s);
 		return;
 	}
 
@@ -797,7 +797,7 @@ static int acm_svr_select_src(struct acm_ep_addr_data *src, struct acm_ep_addr_d
 	union socket_addr addr;
 	socklen_t len;
 	int ret;
-	SOCKET s;
+	int s;
 
 	acm_log(2, "selecting source address\n");
 	memset(&addr, 0, sizeof addr);
@@ -818,22 +818,22 @@ static int acm_svr_select_src(struct acm_ep_addr_data *src, struct acm_ep_addr_d
 	}
 
 	s = socket(addr.sa.sa_family, SOCK_DGRAM, IPPROTO_UDP);
-	if (s == INVALID_SOCKET) {
+	if (s == -1) {
 		acm_log(0, "ERROR - unable to allocate socket\n");
-		return socket_errno();
+		return errno;
 	}
 
 	ret = connect(s, &addr.sa, len);
 	if (ret) {
 		acm_log(0, "ERROR - unable to connect socket\n");
-		ret = socket_errno();
+		ret = errno;
 		goto out;
 	}
 
 	ret = getsockname(s, &addr.sa, &len);
 	if (ret) {
 		acm_log(0, "ERROR - failed to get socket address\n");
-		ret = socket_errno();
+		ret = errno;
 		goto out;
 	}
 
@@ -1376,7 +1376,7 @@ static void acm_ipnl_handler(void)
 	}
 }
 
-static int acm_nl_send(SOCKET sock, struct acm_msg *msg)
+static int acm_nl_send(int sock, struct acm_msg *msg)
 {
 	struct sockaddr_nl dst_addr;
 	struct acm_nl_msg acmnlmsg;
@@ -1672,12 +1672,12 @@ static int acm_init_nl(void)
 {
 	struct sockaddr_nl src_addr;
 	int ret;
-	SOCKET nl_rcv_socket;
+	int nl_rcv_socket;
 
 	nl_rcv_socket = socket(PF_NETLINK, SOCK_RAW, NETLINK_RDMA);
-	if (nl_rcv_socket == INVALID_SOCKET) {
+	if (nl_rcv_socket == -1) {
 		acm_log(0, "ERROR - unable to allocate netlink recv socket\n");
-		return socket_errno();
+		return errno;
 	}
 
 	memset(&src_addr, 0, sizeof(src_addr));
@@ -1687,10 +1687,10 @@ static int acm_init_nl(void)
 
 	ret = bind(nl_rcv_socket, (struct sockaddr *)&src_addr,
 		   sizeof(src_addr));
-	if (ret == SOCKET_ERROR) {
+	if (ret == -1) {
 		acm_log(0, "ERROR - unable to bind netlink socket\n");
 		close(nl_rcv_socket);
-		return socket_errno();
+		return errno;
 	}
 
 	/* init nl client structure */
@@ -1725,7 +1725,7 @@ static void acm_server(void)
 		FD_SET(ip_mon_socket, &readfds);
 
 		for (i = 0; i < FD_SETSIZE - 1; i++) {
-			if (client_array[i].sock != INVALID_SOCKET) {
+			if (client_array[i].sock != -1) {
 				FD_SET(client_array[i].sock, &readfds);
 				n = max(n, (int) client_array[i].sock);
 			}
@@ -1739,7 +1739,7 @@ static void acm_server(void)
 		}
 
 		ret = select(n + 1, &readfds, NULL, NULL, NULL);
-		if (ret == SOCKET_ERROR) {
+		if (ret == -1) {
 			acm_log(0, "ERROR - server select error\n");
 			continue;
 		}
@@ -1751,7 +1751,7 @@ static void acm_server(void)
 			acm_ipnl_handler();
 
 		for (i = 0; i < FD_SETSIZE - 1; i++) {
-			if (client_array[i].sock != INVALID_SOCKET &&
+			if (client_array[i].sock != -1 &&
 				FD_ISSET(client_array[i].sock, &readfds)) {
 				acm_log(2, "receiving from client %d\n", i);
 				if (i == NL_CLIENT_INDEX)
@@ -3122,7 +3122,7 @@ int CDECL_FUNC main(int argc, char **argv)
 	acm_server();
 
 	acm_log(0, "shutting down\n");
-	if (client_array[NL_CLIENT_INDEX].sock != INVALID_SOCKET)
+	if (client_array[NL_CLIENT_INDEX].sock != -1)
 		close(client_array[NL_CLIENT_INDEX].sock);
 	acm_close_providers();
 	acm_stop_sa_handler();
diff --git a/ibacm/src/libacm.c b/ibacm/src/libacm.c
index 0f45469..4273dfe 100644
--- a/ibacm/src/libacm.c
+++ b/ibacm/src/libacm.c
@@ -39,7 +39,7 @@
 #include <arpa/inet.h>
 
 extern pthread_mutex_t lock;
-static SOCKET sock = INVALID_SOCKET;
+static int sock = -1;
 static short server_port = 6125;
 
 static void acm_set_server_port(void)
@@ -67,8 +67,8 @@ int ib_acm_connect(char *dest)
 		return ret;
 
 	sock = socket(res->ai_family, res->ai_socktype, res->ai_protocol);
-	if (sock == INVALID_SOCKET) {
-		ret = socket_errno();
+	if (sock == -1) {
+		ret = errno;
 		goto err1;
 	}
 
@@ -81,8 +81,8 @@ int ib_acm_connect(char *dest)
 	return 0;
 
 err2:
-	closesocket(sock);
-	sock = INVALID_SOCKET;
+	close(sock);
+	sock = -1;
 err1:
 	freeaddrinfo(res);
 	return ret;
@@ -90,10 +90,10 @@ err1:
 
 void ib_acm_disconnect(void)
 {
-	if (sock != INVALID_SOCKET) {
+	if (sock != -1) {
 		shutdown(sock, SHUT_RDWR);
-		closesocket(sock);
-		sock = INVALID_SOCKET;
+		close(sock);
+		sock = -1;
 	}
 }
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 03/13] ibacm: use __thread directly
From: Christoph Hellwig @ 2016-10-17 19:11 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476731482-26491-1-git-send-email-hch-jcswGhMUV9g@public.gmane.org>

Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
---
 ibacm/linux/osd.h          | 1 -
 ibacm/prov/acmp/src/acmp.c | 2 +-
 ibacm/src/acm.c            | 2 +-
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/ibacm/linux/osd.h b/ibacm/linux/osd.h
index 3580cc3..31a7044 100644
--- a/ibacm/linux/osd.h
+++ b/ibacm/linux/osd.h
@@ -136,7 +136,6 @@ static inline uint64_t time_stamp_us(void)
 #define time_stamp_sec() (time_stamp_ms() / (uint64_t) 1000)
 #define time_stamp_min() (time_stamp_sec() / (uint64_t) 60)
 
-#define PER_THREAD __thread
 static inline int beginthread(void (*func)(void *), void *arg)
 {
 	pthread_t thread;
diff --git a/ibacm/prov/acmp/src/acmp.c b/ibacm/prov/acmp/src/acmp.c
index 24d2b70..1f3aeef 100644
--- a/ibacm/prov/acmp/src/acmp.c
+++ b/ibacm/prov/acmp/src/acmp.c
@@ -259,7 +259,7 @@ static atomic_t wait_cnt;
 static pthread_t retry_thread_id;
 static int retry_thread_started = 0;
 
-PER_THREAD char log_data[ACM_MAX_ADDRESS];
+__thread char log_data[ACM_MAX_ADDRESS];
 
 /*
  * Service options - may be set through ibacm_opts.cfg file.
diff --git a/ibacm/src/acm.c b/ibacm/src/acm.c
index d40aa71..33fc044 100644
--- a/ibacm/src/acm.c
+++ b/ibacm/src/acm.c
@@ -186,7 +186,7 @@ static struct acmc_client client_array[FD_SETSIZE - 1];
 
 static FILE *flog;
 static pthread_mutex_t log_lock;
-PER_THREAD char log_data[ACM_MAX_ADDRESS];
+__thread char log_data[ACM_MAX_ADDRESS];
 static atomic_t counter[ACM_MAX_COUNTER];
 
 static struct acmc_device *
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 02/13] ibacm: use pthread_mutex_t directly
From: Christoph Hellwig @ 2016-10-17 19:11 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476731482-26491-1-git-send-email-hch-jcswGhMUV9g@public.gmane.org>

Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
---
 ibacm/linux/libacm_linux.c |   4 +-
 ibacm/linux/osd.h          |   5 --
 ibacm/prov/acmp/src/acmp.c | 132 ++++++++++++++++++++++-----------------------
 ibacm/src/acm.c            |  42 +++++++--------
 ibacm/src/libacm.c         |  22 ++++----
 5 files changed, 100 insertions(+), 105 deletions(-)

diff --git a/ibacm/linux/libacm_linux.c b/ibacm/linux/libacm_linux.c
index a3f6715..2b8a910 100644
--- a/ibacm/linux/libacm_linux.c
+++ b/ibacm/linux/libacm_linux.c
@@ -29,9 +29,9 @@
 
 #include <osd.h>
 
-lock_t lock;
+pthread_mutex_t lock;
 
 static void __attribute__((constructor)) lib_init(void)
 {
-	lock_init(&lock);
+	pthread_mutex_init(&lock, NULL);
 }
diff --git a/ibacm/linux/osd.h b/ibacm/linux/osd.h
index c1e7d99..3580cc3 100644
--- a/ibacm/linux/osd.h
+++ b/ibacm/linux/osd.h
@@ -115,11 +115,6 @@ static inline int event_wait(event_t *e, int timeout)
 	return ret;
 }
 
-#define lock_t       pthread_mutex_t
-#define lock_init(x) pthread_mutex_init(x, NULL)
-#define lock_acquire pthread_mutex_lock
-#define lock_release pthread_mutex_unlock
-
 #define osd_init()  0
 #define osd_close()
 
diff --git a/ibacm/prov/acmp/src/acmp.c b/ibacm/prov/acmp/src/acmp.c
index 806320f..24d2b70 100644
--- a/ibacm/prov/acmp/src/acmp.c
+++ b/ibacm/prov/acmp/src/acmp.c
@@ -115,7 +115,7 @@ struct acmp_dest {
 	uint64_t               req_id;
 	DLIST_ENTRY            req_queue;
 	uint32_t               remote_qpn;
-	lock_t                 lock;
+	pthread_mutex_t        lock;
 	enum acmp_state        state;
 	atomic_t               refcnt;
 	uint64_t	       addr_timeout;
@@ -130,7 +130,7 @@ struct acmp_port {
 	struct acmp_device  *dev;
 	const struct acm_port *port;
 	DLIST_ENTRY         ep_list;
-	lock_t              lock;
+	pthread_mutex_t     lock;
 	struct acmp_dest    sa_dest;
 	enum ibv_port_state state;
 	enum ibv_mtu        mtu;
@@ -181,7 +181,7 @@ struct acmp_ep {
 	uint16_t              pkey_index;
 	uint16_t	      pkey;
 	const struct acm_endpoint *endpoint;
-	lock_t                lock;
+	pthread_mutex_t       lock;
 	struct acmp_send_queue resolve_queue;
 	struct acmp_send_queue resp_queue;
 	DLIST_ENTRY           active_queue;
@@ -250,7 +250,7 @@ static struct acm_provider def_prov = {
 };
 
 static DLIST_ENTRY acmp_dev_list;
-static lock_t acmp_dev_lock;
+static pthread_mutex_t acmp_dev_lock;
 
 static atomic_t g_tid;
 static DLIST_ENTRY timeout_list;
@@ -304,7 +304,7 @@ acmp_init_dest(struct acmp_dest *dest, uint8_t addr_type,
 	DListInit(&dest->req_queue);
 	atomic_init(&dest->refcnt);
 	atomic_set(&dest->refcnt, 1);
-	lock_init(&dest->lock);
+	pthread_mutex_init(&dest->lock, NULL);
 	if (size)
 		acmp_set_dest_addr(dest, addr_type, addr, size);
 	dest->state = ACMP_INIT;
@@ -374,7 +374,7 @@ acmp_acquire_dest(struct acmp_ep *ep, uint8_t addr_type, const uint8_t *addr)
 	acm_format_name(2, log_data, sizeof log_data,
 			addr_type, addr, ACM_MAX_ADDRESS);
 	acm_log(2, "%s\n", log_data);
-	lock_acquire(&ep->lock);
+	pthread_mutex_lock(&ep->lock);
 	dest = acmp_get_dest(ep, addr_type, addr);
 	if (dest && dest->state == ACMP_READY &&
 	    dest->addr_timeout != (uint64_t)~0ULL) {
@@ -396,7 +396,7 @@ acmp_acquire_dest(struct acmp_ep *ep, uint8_t addr_type, const uint8_t *addr)
 			(void) atomic_inc(&dest->refcnt);
 		}
 	}
-	lock_release(&ep->lock);
+	pthread_mutex_unlock(&ep->lock);
 	return dest;
 }
 
@@ -504,7 +504,7 @@ static void acmp_post_send(struct acmp_send_queue *queue, struct acmp_send_msg *
 	struct ibv_send_wr *bad_wr;
 
 	msg->req_queue = queue;
-	lock_acquire(&ep->lock);
+	pthread_mutex_lock(&ep->lock);
 	if (queue->credits) {
 		acm_log(2, "posting send to QP\n");
 		queue->credits--;
@@ -514,7 +514,7 @@ static void acmp_post_send(struct acmp_send_queue *queue, struct acmp_send_msg *
 		acm_log(2, "no sends available, queuing message\n");
 		DListInsertTail(&msg->entry, &queue->pending);
 	}
-	lock_release(&ep->lock);
+	pthread_mutex_unlock(&ep->lock);
 }
 
 static void acmp_post_recv(struct acmp_ep *ep, uint64_t address)
@@ -557,7 +557,7 @@ static void acmp_complete_send(struct acmp_send_msg *msg)
 {
 	struct acmp_ep *ep = msg->ep;
 
-	lock_acquire(&ep->lock);
+	pthread_mutex_lock(&ep->lock);
 	DListRemove(&msg->entry);
 	if (msg->tries) {
 		acm_log(2, "waiting for response\n");
@@ -570,7 +570,7 @@ static void acmp_complete_send(struct acmp_send_msg *msg)
 		acmp_send_available(ep, msg->req_queue);
 		acmp_free_send(msg);
 	}
-	lock_release(&ep->lock);
+	pthread_mutex_unlock(&ep->lock);
 }
 
 static struct acmp_send_msg *acmp_get_request(struct acmp_ep *ep, uint64_t tid, int *free)
@@ -580,7 +580,7 @@ static struct acmp_send_msg *acmp_get_request(struct acmp_ep *ep, uint64_t tid,
 	DLIST_ENTRY *entry, *next;
 
 	acm_log(2, "\n");
-	lock_acquire(&ep->lock);
+	pthread_mutex_lock(&ep->lock);
 	for (entry = ep->wait_queue.Next; entry != &ep->wait_queue; entry = next) {
 		next = entry->Next;
 		msg = container_of(entry, struct acmp_send_msg, entry);
@@ -608,7 +608,7 @@ static struct acmp_send_msg *acmp_get_request(struct acmp_ep *ep, uint64_t tid,
 		}
 	}
 unlock:
-	lock_release(&ep->lock);
+	pthread_mutex_unlock(&ep->lock);
 	return req;
 }
 
@@ -688,13 +688,13 @@ acmp_init_path_av(struct acmp_port *port, struct acmp_dest *dest)
 	flow_hop = ntohl(dest->path.flowlabel_hoplimit);
 	dest->av.is_global = 1;
 	dest->av.grh.flow_label = (flow_hop >> 8) & 0xFFFFF;
-	lock_acquire(&port->lock);
+	pthread_mutex_lock(&port->lock);
 	if (port->port) 
 		dest->av.grh.sgid_index = acm_gid_index(
 		   (struct acm_port *) port->port, &dest->path.sgid);
 	else
 		dest->av.grh.sgid_index = 0;
-	lock_release(&port->lock);
+	pthread_mutex_unlock(&port->lock);
 	dest->av.grh.hop_limit = (uint8_t) flow_hop;
 	dest->av.grh.traffic_class = dest->path.tclass;
 }
@@ -710,7 +710,7 @@ static void acmp_process_join_resp(struct acm_sa_mad *sa_mad)
 	mad = (struct ib_sa_mad *) &sa_mad->sa_mad;
 	acm_log(1, "response status: 0x%x, mad status: 0x%x\n",
 		sa_mad->umad.status, mad->status);
-	lock_acquire(&ep->lock);
+	pthread_mutex_lock(&ep->lock);
 	if (sa_mad->umad.status) {
 		acm_log(0, "ERROR - send join failed 0x%x\n", sa_mad->umad.status);
 		goto out;
@@ -753,7 +753,7 @@ static void acmp_process_join_resp(struct acm_sa_mad *sa_mad)
 	acm_log(1, "join successful\n");
 out:
 	acm_free_sa_mad(sa_mad);
-	lock_release(&ep->lock);
+	pthread_mutex_unlock(&ep->lock);
 }
 
 static uint8_t
@@ -975,20 +975,20 @@ acmp_complete_queued_req(struct acmp_dest *dest, uint8_t status)
 	DLIST_ENTRY *entry;
 
 	acm_log(2, "status %d\n", status);
-	lock_acquire(&dest->lock);
+	pthread_mutex_lock(&dest->lock);
 	while (!DListEmpty(&dest->req_queue)) {
 		entry = dest->req_queue.Next;
 		DListRemove(entry);
 		req = container_of(entry, struct acmp_request, entry);
-		lock_release(&dest->lock);
+		pthread_mutex_unlock(&dest->lock);
 
 		acm_log(2, "completing request, client %" PRIu64 "\n", req->id);
 		acmp_resolve_response(req->id, &req->msg, dest, status);
 		acmp_free_req(req);
 
-		lock_acquire(&dest->lock);
+		pthread_mutex_lock(&dest->lock);
 	}
-	lock_release(&dest->lock);
+	pthread_mutex_unlock(&dest->lock);
 }
 
 static void
@@ -1005,10 +1005,10 @@ acmp_dest_sa_resp(struct acm_sa_mad *mad)
 	}
 	acm_log(2, "%s status=0x%x\n", dest->name, status);
 
-	lock_acquire(&dest->lock);
+	pthread_mutex_lock(&dest->lock);
 	if (dest->state != ACMP_QUERY_ROUTE) {
 		acm_log(1, "notice - discarding SA response\n");
-		lock_release(&dest->lock);
+		pthread_mutex_unlock(&dest->lock);
 		goto out;
 	}
 
@@ -1023,7 +1023,7 @@ acmp_dest_sa_resp(struct acm_sa_mad *mad)
 	} else {
 		dest->state = ACMP_INIT;
 	}
-	lock_release(&dest->lock);
+	pthread_mutex_unlock(&dest->lock);
 
 	acmp_complete_queued_req(dest, status);
 out:
@@ -1039,9 +1039,9 @@ acmp_resolve_sa_resp(struct acm_sa_mad *mad)
 	acm_log(2, "\n");
 	acmp_dest_sa_resp(mad);
 
-	lock_acquire(&dest->lock);
+	pthread_mutex_lock(&dest->lock);
 	send_resp = (dest->state == ACMP_READY);
-	lock_release(&dest->lock);
+	pthread_mutex_unlock(&dest->lock);
 
 	if (send_resp)
 		acmp_send_addr_resp(dest->ep, dest);
@@ -1093,7 +1093,7 @@ acmp_process_addr_req(struct acmp_ep *ep, struct ibv_wc *wc, struct acm_mad *mad
 	if (addr)
 		dest->req_id = mad->tid;
 
-	lock_acquire(&dest->lock);
+	pthread_mutex_lock(&dest->lock);
 	acm_log(2, "dest state %d\n", dest->state);
 	switch (dest->state) {
 	case ACMP_READY:
@@ -1120,11 +1120,11 @@ acmp_process_addr_req(struct acmp_ep *ep, struct ibv_wc *wc, struct acm_mad *mad
 		}
 		/* fall through */
 	default:
-		lock_release(&dest->lock);
+		pthread_mutex_unlock(&dest->lock);
 		acmp_put_dest(dest);
 		return;
 	}
-	lock_release(&dest->lock);
+	pthread_mutex_unlock(&dest->lock);
 	acmp_complete_queued_req(dest, status);
 
 	if (addr && !status) {
@@ -1149,9 +1149,9 @@ acmp_process_addr_resp(struct acmp_send_msg *msg, struct ibv_wc *wc, struct acm_
 	}
 	acm_log(2, "resp status 0x%x\n", status);
 
-	lock_acquire(&dest->lock);
+	pthread_mutex_lock(&dest->lock);
 	if (dest->state != ACMP_QUERY_ADDR) {
-		lock_release(&dest->lock);
+		pthread_mutex_unlock(&dest->lock);
 		goto put;
 	}
 
@@ -1163,7 +1163,7 @@ acmp_process_addr_resp(struct acmp_send_msg *msg, struct ibv_wc *wc, struct acm_
 			} else {
 				status = acmp_resolve_path_sa(msg->ep, dest, acmp_dest_sa_resp);
 				if (!status) {
-					lock_release(&dest->lock);
+					pthread_mutex_unlock(&dest->lock);
 					goto put;
 				}
 			}
@@ -1171,7 +1171,7 @@ acmp_process_addr_resp(struct acmp_send_msg *msg, struct ibv_wc *wc, struct acm_
 	} else {
 		dest->state = ACMP_INIT;
 	}
-	lock_release(&dest->lock);
+	pthread_mutex_unlock(&dest->lock);
 
 	acmp_complete_queued_req(dest, status);
 put:
@@ -1578,34 +1578,34 @@ static void *acmp_retry_handler(void *context)
 		}
 
 		next_expire = -1;
-		lock_acquire(&acmp_dev_lock);
+		pthread_mutex_lock(&acmp_dev_lock);
 		for (dev_entry = acmp_dev_list.Next; dev_entry != &acmp_dev_list;
 		     dev_entry = dev_entry->Next) {
 
 			dev = container_of(dev_entry, struct acmp_device, entry);
-			lock_release(&acmp_dev_lock);
+			pthread_mutex_unlock(&acmp_dev_lock);
 
 			for (i = 0; i < dev->port_cnt; i++) {
 				port = &dev->port[i];
 
-				lock_acquire(&port->lock);
+				pthread_mutex_lock(&port->lock);
 				for (ep_entry = port->ep_list.Next;
 				     ep_entry != &port->ep_list;
 				     ep_entry = ep_entry->Next) {
 
 					ep = container_of(ep_entry, struct acmp_ep, entry);
-					lock_release(&port->lock);
-					lock_acquire(&ep->lock);
+					pthread_mutex_unlock(&port->lock);
+					pthread_mutex_lock(&ep->lock);
 					if (!DListEmpty(&ep->wait_queue))
 						acmp_process_wait_queue(ep, &next_expire);
-					lock_release(&ep->lock);
-					lock_acquire(&port->lock);
+					pthread_mutex_unlock(&ep->lock);
+					pthread_mutex_lock(&port->lock);
 				}
-				lock_release(&port->lock);
+				pthread_mutex_unlock(&port->lock);
 			}
-			lock_acquire(&acmp_dev_lock);
+			pthread_mutex_lock(&acmp_dev_lock);
 		}
-		lock_release(&acmp_dev_lock);
+		pthread_mutex_unlock(&acmp_dev_lock);
 
 		acmp_process_timeouts();
 		wait = (int) (next_expire - time_stamp_ms());
@@ -1844,7 +1844,7 @@ acmp_resolve_dest(struct acmp_ep *ep, struct acm_msg *msg, uint64_t id)
 		return acmp_resolve_response(id, msg, NULL, ACM_STATUS_ENOMEM);
 	}
 
-	lock_acquire(&dest->lock);
+	pthread_mutex_lock(&dest->lock);
 test:
 	switch (dest->state) {
 	case ACMP_READY:
@@ -1885,10 +1885,10 @@ queue:
 			break;
 		}
 		ret = 0;
-		lock_release(&dest->lock);
+		pthread_mutex_unlock(&dest->lock);
 		goto put;
 	}
-	lock_release(&dest->lock);
+	pthread_mutex_unlock(&dest->lock);
 	ret = acmp_resolve_response(id, msg, dest, status);
 put:
 	acmp_put_dest(dest);
@@ -1920,7 +1920,7 @@ acmp_resolve_path(struct acmp_ep *ep, struct acm_msg *msg, uint64_t id)
 		return acmp_resolve_response(id, msg, NULL, ACM_STATUS_ENOMEM);
 	}
 
-	lock_acquire(&dest->lock);
+	pthread_mutex_lock(&dest->lock);
 test:
 	switch (dest->state) {
 	case ACMP_READY:
@@ -1953,10 +1953,10 @@ test:
 			break;
 		}
 		ret = 0;
-		lock_release(&dest->lock);
+		pthread_mutex_unlock(&dest->lock);
 		goto put;
 	}
-	lock_release(&dest->lock);
+	pthread_mutex_unlock(&dest->lock);
 	ret = acmp_resolve_response(id, msg, dest, status);
 put:
 	acmp_put_dest(dest);
@@ -2530,7 +2530,7 @@ acmp_alloc_ep(struct acmp_port *port, struct acm_endpoint *endpoint)
 	DListInit(&ep->resp_queue.pending);
 	DListInit(&ep->active_queue);
 	DListInit(&ep->wait_queue);
-	lock_init(&ep->lock);
+	pthread_mutex_init(&ep->lock, NULL);
 	sprintf(ep->id_string, "%s-%d-0x%x", port->dev->verbs->device->name,
 		port->port_num, endpoint->pkey);
 	for (i = 0; i < ACM_MAX_COUNTER; i++) 
@@ -2551,9 +2551,9 @@ static int acmp_open_endpoint(const struct acm_endpoint *endpoint,
 	ep = acmp_get_ep(port,  (struct acm_endpoint *) endpoint);
 	if (ep) {
 		acm_log(2, "endpoint for pkey 0x%x already exists\n", endpoint->pkey);
-		lock_acquire(&ep->lock);
+		pthread_mutex_lock(&ep->lock);
 		ep->endpoint =  (struct acm_endpoint *) endpoint;
-		lock_release(&ep->lock);
+		pthread_mutex_unlock(&ep->lock);
 		*ep_context = (void *) ep;
 		return 0;
 	}
@@ -2627,9 +2627,9 @@ static int acmp_open_endpoint(const struct acm_endpoint *endpoint,
 	if (ret)
 		goto err2;
 
-	lock_acquire(&port->lock);
+	pthread_mutex_lock(&port->lock);
 	DListInsertHead(&ep->entry, &port->ep_list);
-	lock_release(&port->lock);
+	pthread_mutex_unlock(&port->lock);
 	acmp_ep_preload(ep);
 	acmp_ep_join(ep);
 	*ep_context = (void *) ep;
@@ -2700,9 +2700,9 @@ static void acmp_port_up(struct acmp_port *port)
 static void acmp_port_down(struct acmp_port *port)
 {
 	acm_log(1, "%s %d\n", port->dev->verbs->device->name, port->port_num);
-	lock_acquire(&port->lock);
+	pthread_mutex_lock(&port->lock);
 	port->state = IBV_PORT_DOWN;
-	lock_release(&port->lock);
+	pthread_mutex_unlock(&port->lock);
 
 	/*
 	 * We wait for the SA destination to be released.  We could use an
@@ -2712,9 +2712,9 @@ static void acmp_port_down(struct acmp_port *port)
 	atomic_dec(&port->sa_dest.refcnt);
 	while (atomic_get(&port->sa_dest.refcnt))
 		sleep(0);
-	lock_acquire(&port->sa_dest.lock);
+	pthread_mutex_lock(&port->sa_dest.lock);
 	port->sa_dest.state = ACMP_INIT;
-	lock_release(&port->sa_dest.lock);
+	pthread_mutex_unlock(&port->sa_dest.lock);
 	acm_log(1, "%s %d is down\n", port->dev->verbs->device->name, port->port_num);
 }
 
@@ -2731,10 +2731,10 @@ static int acmp_open_port(const struct acm_port *cport, void *dev_context,
 	}
 
 	port = &dev->port[cport->port_num - 1];
-	lock_acquire(&port->lock);
+	pthread_mutex_lock(&port->lock);
 	port->port = cport;
 	port->state = IBV_PORT_DOWN;
-	lock_release(&port->lock);
+	pthread_mutex_unlock(&port->lock);
 	acmp_port_up(port);
 	*port_context = port;
 	return 0;
@@ -2745,9 +2745,9 @@ static void acmp_close_port(void *port_context)
 	struct acmp_port *port = port_context;
 
 	acmp_port_down(port);
-	lock_acquire(&port->lock);
+	pthread_mutex_lock(&port->lock);
 	port->port = NULL;
-	lock_release(&port->lock);
+	pthread_mutex_unlock(&port->lock);
 }
 
 static void acmp_init_port(struct acmp_port *port, struct acmp_device *dev, 
@@ -2756,7 +2756,7 @@ static void acmp_init_port(struct acmp_port *port, struct acmp_device *dev,
 	acm_log(1, "%s %d\n", dev->verbs->device->name, port_num);
 	port->dev = dev;
 	port->port_num = port_num;
-	lock_init(&port->lock);
+	pthread_mutex_init(&port->lock, NULL);
 	DListInit(&port->ep_list);
 	acmp_init_dest(&port->sa_dest, ACM_ADDRESS_LID, NULL, 0);
 	port->state = IBV_PORT_DOWN;
@@ -2838,9 +2838,9 @@ static int acmp_open_dev(const struct acm_device *device, void **dev_context)
 		goto err3;
 	}
 
-	lock_acquire(&acmp_dev_lock);
+	pthread_mutex_lock(&acmp_dev_lock);
 	DListInsertHead(&dev->entry, &acmp_dev_list);
-	lock_release(&acmp_dev_lock);
+	pthread_mutex_unlock(&acmp_dev_lock);
 	dev->guid = device->dev_guid;
 	*dev_context = dev;
 
@@ -2951,7 +2951,7 @@ static void __attribute__((constructor)) acmp_init(void)
 	atomic_init(&g_tid);
 	atomic_init(&wait_cnt);
 	DListInit(&acmp_dev_list);
-	lock_init(&acmp_dev_lock);
+	pthread_mutex_init(&acmp_dev_lock, NULL);
 	DListInit(&timeout_list);
 	event_init(&timeout_event);
 
diff --git a/ibacm/src/acm.c b/ibacm/src/acm.c
index 01ba4e1..d40aa71 100644
--- a/ibacm/src/acm.c
+++ b/ibacm/src/acm.c
@@ -106,7 +106,7 @@ struct acmc_port {
 	DLIST_ENTRY         sa_pending;
 	DLIST_ENTRY	    sa_wait;
 	int		    sa_credits;
-	lock_t              lock;
+	pthread_mutex_t     lock;
 	DLIST_ENTRY         ep_list;
 	enum ibv_port_state state;
 	int                 gid_cnt;
@@ -140,7 +140,7 @@ struct acmc_ep {
 };
 
 struct acmc_client {
-	lock_t   lock;   /* acquire ep lock first */
+	pthread_mutex_t lock;   /* acquire ep lock first */
 	SOCKET   sock;
 	int      index;
 	atomic_t refcnt;
@@ -185,7 +185,7 @@ static SOCKET ip_mon_socket;
 static struct acmc_client client_array[FD_SETSIZE - 1];
 
 static FILE *flog;
-static lock_t log_lock;
+static pthread_mutex_t log_lock;
 PER_THREAD char log_data[ACM_MAX_ADDRESS];
 static atomic_t counter[ACM_MAX_COUNTER];
 
@@ -230,11 +230,11 @@ void acm_write(int level, const char *format, ...)
 
 	gettimeofday(&tv, NULL);
 	va_start(args, format);
-	lock_acquire(&log_lock);
+	pthread_mutex_lock(&log_lock);
 	fprintf(flog, "%u.%03u: ", (unsigned) tv.tv_sec, (unsigned) (tv.tv_usec / 1000));
 	vfprintf(flog, format, args);
 	fflush(flog);
-	lock_release(&log_lock);
+	pthread_mutex_unlock(&log_lock);
 	va_end(args);
 }
 
@@ -483,7 +483,7 @@ int acm_resolve_response(uint64_t id, struct acm_msg *msg)
 	else if (msg->hdr.status)
 		atomic_inc(&counter[ACM_CNTR_ERROR]);
 
-	lock_acquire(&client->lock);
+	pthread_mutex_lock(&client->lock);
 	if (client->sock == INVALID_SOCKET) {
 		acm_log(0, "ERROR - connection lost\n");
 		ret = ACM_STATUS_ENOTCONN;
@@ -501,7 +501,7 @@ int acm_resolve_response(uint64_t id, struct acm_msg *msg)
 		ret = 0;
 
 release:
-	lock_release(&client->lock);
+	pthread_mutex_unlock(&client->lock);
 	(void) atomic_dec(&client->refcnt);
 	return ret;
 }
@@ -524,7 +524,7 @@ int acm_query_response(uint64_t id, struct acm_msg *msg)
 	int ret;
 
 	acm_log(2, "status 0x%x\n", msg->hdr.status);
-	lock_acquire(&client->lock);
+	pthread_mutex_lock(&client->lock);
 	if (client->sock == INVALID_SOCKET) {
 		acm_log(0, "ERROR - connection lost\n");
 		ret = ACM_STATUS_ENOTCONN;
@@ -538,7 +538,7 @@ int acm_query_response(uint64_t id, struct acm_msg *msg)
 		ret = 0;
 
 release:
-	lock_release(&client->lock);
+	pthread_mutex_unlock(&client->lock);
 	(void) atomic_dec(&client->refcnt);
 	return ret;
 }
@@ -557,7 +557,7 @@ static void acm_init_server(void)
 	int i;
 
 	for (i = 0; i < FD_SETSIZE - 1; i++) {
-		lock_init(&client_array[i].lock);
+		pthread_mutex_init(&client_array[i].lock, NULL);
 		client_array[i].index = i;
 		client_array[i].sock = INVALID_SOCKET;
 		atomic_init(&client_array[i].refcnt);
@@ -604,11 +604,11 @@ static int acm_listen(void)
 
 static void acm_disconnect_client(struct acmc_client *client)
 {
-	lock_acquire(&client->lock);
+	pthread_mutex_lock(&client->lock);
 	shutdown(client->sock, SHUT_RDWR);
 	closesocket(client->sock);
 	client->sock = INVALID_SOCKET;
-	lock_release(&client->lock);
+	pthread_mutex_unlock(&client->lock);
 	(void) atomic_dec(&client->refcnt);
 }
 
@@ -2425,7 +2425,7 @@ acm_open_port(struct acmc_port *port, struct acmc_device *dev, uint8_t port_num)
 	port->dev = dev;
 	port->port.dev = &dev->device;
 	port->port.port_num = port_num;
-	lock_init(&port->lock);
+	pthread_mutex_init(&port->lock, NULL);
 	DListInit(&port->ep_list);
 	DListInit(&port->sa_pending);
 	DListInit(&port->sa_wait);
@@ -2783,7 +2783,7 @@ int acm_send_sa_mad(struct acm_sa_mad *mad)
 	mad->umad.addr.sl = port->sa_addr.sl;
 	mad->umad.addr.pkey_index = req->ep->port->sa_pkey_index;
 
-	lock_acquire(&port->lock);
+	pthread_mutex_lock(&port->lock);
 	if (port->sa_credits && DListEmpty(&port->sa_wait)) {
 		ret = umad_send(port->mad_portid, port->mad_agentid, &mad->umad,
 				sizeof mad->sa_mad, sa.timeout, sa.retries);
@@ -2795,7 +2795,7 @@ int acm_send_sa_mad(struct acm_sa_mad *mad)
 		ret = 0;
 		DListInsertTail(&req->entry, &port->sa_wait);
 	}
-	lock_release(&port->lock);
+	pthread_mutex_unlock(&port->lock);
 	return ret;
 }
 
@@ -2804,9 +2804,9 @@ static void acmc_send_queued_req(struct acmc_port *port)
 	struct acmc_sa_req *req;
 	int ret;
 
-	lock_acquire(&port->lock);
+	pthread_mutex_lock(&port->lock);
 	if (DListEmpty(&port->sa_wait) || !port->sa_credits) {
-		lock_release(&port->lock);
+		pthread_mutex_unlock(&port->lock);
 		return;
 	}
 
@@ -2818,7 +2818,7 @@ static void acmc_send_queued_req(struct acmc_port *port)
 		port->sa_credits--;
 		DListInsertTail(&req->entry, &port->sa_pending);
 	}
-	lock_release(&port->lock);
+	pthread_mutex_unlock(&port->lock);
 
 	if (ret) {
 		req->mad.umad.status = -ret;
@@ -2847,7 +2847,7 @@ static void acmc_recv_mad(struct acmc_port *port)
 		hdr->base_version, hdr->mgmt_class, hdr->class_version,
 		hdr->method, hdr->status, hdr->tid, hdr->attr_id, hdr->attr_mod);
 	found = 0;
-	lock_acquire(&port->lock);
+	pthread_mutex_lock(&port->lock);
 	for (entry = port->sa_pending.Next; entry != &port->sa_pending;
 	     entry = entry->Next) {
 		req = container_of(entry, struct acmc_sa_req, entry);
@@ -2859,7 +2859,7 @@ static void acmc_recv_mad(struct acmc_port *port)
 			break;
 		}
 	}
-	lock_release(&port->lock);
+	pthread_mutex_unlock(&port->lock);
 
 	if (found) {
 		memcpy(&req->mad.umad, &resp.umad, sizeof(resp.umad) + len);
@@ -3083,7 +3083,7 @@ int CDECL_FUNC main(int argc, char **argv)
 	if (acm_open_lock_file())
 		return -1;
 
-	lock_init(&log_lock);
+	pthread_mutex_init(&log_lock, NULL);
 	flog = acm_open_log();
 
 	acm_log(0, "Assistant to the InfiniBand Communication Manager\n");
diff --git a/ibacm/src/libacm.c b/ibacm/src/libacm.c
index def5b2c..0f45469 100644
--- a/ibacm/src/libacm.c
+++ b/ibacm/src/libacm.c
@@ -38,7 +38,7 @@
 #include <netdb.h>
 #include <arpa/inet.h>
 
-extern lock_t lock;
+extern pthread_mutex_t lock;
 static SOCKET sock = INVALID_SOCKET;
 static short server_port = 6125;
 
@@ -212,7 +212,7 @@ static int acm_resolve(uint8_t *src, uint8_t *dest, uint8_t type,
 	struct acm_msg msg;
 	int ret, cnt = 0;
 
-	lock_acquire(&lock);
+	pthread_mutex_lock(&lock);
 	memset(&msg, 0, sizeof msg);
 	msg.hdr.version = ACM_VERSION;
 	msg.hdr.opcode = ACM_OP_RESOLVE;
@@ -246,7 +246,7 @@ static int acm_resolve(uint8_t *src, uint8_t *dest, uint8_t type,
 
 	ret = acm_format_resp(&msg, paths, count, print);
 out:
-	lock_release(&lock);
+	pthread_mutex_unlock(&lock);
 	return ret;
 }
 
@@ -275,7 +275,7 @@ int ib_acm_resolve_path(struct ibv_path_record *path, uint32_t flags)
 	struct acm_ep_addr_data *data;
 	int ret;
 
-	lock_acquire(&lock);
+	pthread_mutex_lock(&lock);
 	memset(&msg, 0, sizeof msg);
 	msg.hdr.version = ACM_VERSION;
 	msg.hdr.opcode = ACM_OP_RESOLVE;
@@ -299,7 +299,7 @@ int ib_acm_resolve_path(struct ibv_path_record *path, uint32_t flags)
 		*path = data->info.path;
 
 out:
-	lock_release(&lock);
+	pthread_mutex_unlock(&lock);
 	return ret;
 }
 
@@ -308,7 +308,7 @@ int ib_acm_query_perf(int index, uint64_t **counters, int *count)
 	struct acm_msg msg;
 	int ret, i;
 
-	lock_acquire(&lock);
+	pthread_mutex_lock(&lock);
 	memset(&msg, 0, sizeof msg);
 	msg.hdr.version = ACM_VERSION;
 	msg.hdr.opcode = ACM_OP_PERF_QUERY;
@@ -341,7 +341,7 @@ int ib_acm_query_perf(int index, uint64_t **counters, int *count)
 		(*counters)[i] = ntohll(msg.perf_data[i]);
 	ret = 0;
 out:
-	lock_release(&lock);
+	pthread_mutex_unlock(&lock);
 	return ret;
 }
 
@@ -353,7 +353,7 @@ int ib_acm_enum_ep(int index, struct acm_ep_config_data **data)
 	int cnt;
 	struct acm_ep_config_data *edata;
 
-	lock_acquire(&lock);
+	pthread_mutex_lock(&lock);
 	memset(&msg, 0, sizeof msg);
 	msg.hdr.version = ACM_VERSION;
 	msg.hdr.opcode = ACM_OP_EP_QUERY;
@@ -391,7 +391,7 @@ int ib_acm_enum_ep(int index, struct acm_ep_config_data **data)
 	*data = edata;
 	ret = 0;
 out:
-	lock_release(&lock);
+	pthread_mutex_unlock(&lock);
 	return ret;
 }
 
@@ -404,7 +404,7 @@ int ib_acm_query_perf_ep_addr(uint8_t *src, uint8_t type,
 	if (!src) 
 		return -1;
 
-	lock_acquire(&lock);
+	pthread_mutex_lock(&lock);
 	memset(&msg, 0, sizeof msg);
 	msg.hdr.version = ACM_VERSION;
 	msg.hdr.opcode = ACM_OP_PERF_QUERY;
@@ -444,7 +444,7 @@ int ib_acm_query_perf_ep_addr(uint8_t *src, uint8_t type,
 
 	ret = 0;
 out:
-	lock_release(&lock);
+	pthread_mutex_unlock(&lock);
 	return ret;
 }
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 01/13] ibacm: remove HAVE_NETLINK
From: Christoph Hellwig @ 2016-10-17 19:11 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476731482-26491-1-git-send-email-hch-jcswGhMUV9g@public.gmane.org>

We now always require netlink support

Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
---
 buildlib/config.h.in |  3 ---
 ibacm/src/acm.c      | 25 +------------------------
 2 files changed, 1 insertion(+), 27 deletions(-)

diff --git a/buildlib/config.h.in b/buildlib/config.h.in
index 2378180..e4e7b9e 100644
--- a/buildlib/config.h.in
+++ b/buildlib/config.h.in
@@ -6,9 +6,6 @@
 #define HAVE_TYPEOF 1
 #define HAVE_ISBLANK 1
 
-// FIXME: Remove this, we provide the netlink kernel headers ibacm needs
-#define HAVE_NETLINK 1
-
 // FIXME: Remove this, The cmake version hard-requires new style CLOEXEC support
 #define STREAM_CLOEXEC "e"
 
diff --git a/ibacm/src/acm.c b/ibacm/src/acm.c
index befe942..01ba4e1 100644
--- a/ibacm/src/acm.c
+++ b/ibacm/src/acm.c
@@ -44,10 +44,8 @@
 #include <infiniband/acm_prov.h>
 #include <infiniband/umad.h>
 #include <infiniband/verbs.h>
-#ifdef HAVE_NETLINK
 #include <infiniband/umad_types.h>
 #include <infiniband/umad_sa.h>
-#endif
 #include <dlist.h>
 #include <dlfcn.h> 
 #include <search.h>
@@ -57,19 +55,15 @@
 #include <netinet/in.h>
 #include <linux/netlink.h>
 #include <linux/rtnetlink.h>
-#ifdef HAVE_NETLINK
 #include <rdma/rdma_netlink.h>
 #include <rdma/ib_user_sa.h>
-#endif
 #include <poll.h>
 #include <inttypes.h>
 #include "acm_mad.h"
 #include "acm_util.h"
-#ifdef HAVE_NETLINK
 #if !defined(RDMA_NL_LS_F_ERR)
 	#include "acm_netlink.h"
 #endif
-#endif
 
 #define src_out     data[0]
 #define src_index   data[1]
@@ -78,9 +72,7 @@
 #define MAX_EP_ADDR 4
 #define NL_MSG_BUF_SIZE 4096
 #define ACM_PROV_NAME_SIZE 64
-#ifdef HAVE_NETLINK
 #define NL_CLIENT_INDEX 0
-#endif
 
 struct acmc_subnet {
 	DLIST_ENTRY            entry;
@@ -167,7 +159,6 @@ struct acmc_sa_req {
 	struct acm_sa_mad	mad;
 };
 
-#ifdef HAVE_NETLINK
 struct acm_nl_path {
 	struct nlattr			attr_hdr;
 	struct ib_path_rec_data		rec;
@@ -182,7 +173,6 @@ struct acm_nl_msg {
 		struct acm_nl_path		path[0];
 	};
 };
-#endif
 
 static char def_prov_name[ACM_PROV_NAME_SIZE] = "ibacmp";
 static DLIST_ENTRY provider_list;
@@ -205,9 +195,7 @@ static struct acmc_ep *acm_find_ep(struct acmc_port *port, uint16_t pkey);
 static int acm_ep_insert_addr(struct acmc_ep *ep, const char *name, uint8_t *addr,
 			      size_t addr_len, uint8_t addr_type);
 static void acm_event_handler(struct acmc_device *dev);
-#ifdef HAVE_NETLINK
 static int acm_nl_send(SOCKET sock, struct acm_msg *msg);
-#endif
 
 static struct sa_data {
 	int		timeout;
@@ -502,11 +490,9 @@ int acm_resolve_response(uint64_t id, struct acm_msg *msg)
 		goto release;
 	}
 
-#ifdef HAVE_NETLINK
 	if (id == NL_CLIENT_INDEX)
 		ret = acm_nl_send(client->sock, msg);
 	else
-#endif
 		ret = send(client->sock, (char *) msg, msg->hdr.length, 0);
 
 	if (ret != msg->hdr.length)
@@ -639,10 +625,8 @@ static void acm_svr_accept(void)
 	}
 
 	for (i = 0; i < FD_SETSIZE - 1; i++) {
-	#ifdef HAVE_NETLINK
 		if (i == NL_CLIENT_INDEX)
 			continue;
-	#endif
 		if (!atomic_get(&client_array[i].refcnt))
 			break;
 	}
@@ -1392,7 +1376,6 @@ static void acm_ipnl_handler(void)
 	}
 }
 
-#ifdef HAVE_NETLINK
 static int acm_nl_send(SOCKET sock, struct acm_msg *msg)
 {
 	struct sockaddr_nl dst_addr;
@@ -1714,7 +1697,6 @@ static int acm_init_nl(void)
 	client_array[NL_CLIENT_INDEX].sock = nl_rcv_socket;
 	return 0;
 }
-#endif
 
 static void acm_server(void)
 {
@@ -1730,11 +1712,10 @@ static void acm_server(void)
 		acm_log(0, "ERROR - server listen failed\n");
 		return;
 	}
-#ifdef HAVE_NETLINK
+
 	ret = acm_init_nl();
 	if (ret)
 		acm_log(1, "Warn - Netlink init failed\n");
-#endif
 
 	while (1) {
 		n = (int) listen_socket;
@@ -1773,11 +1754,9 @@ static void acm_server(void)
 			if (client_array[i].sock != INVALID_SOCKET &&
 				FD_ISSET(client_array[i].sock, &readfds)) {
 				acm_log(2, "receiving from client %d\n", i);
-			#ifdef HAVE_NETLINK
 				if (i == NL_CLIENT_INDEX)
 					acm_nl_receive(&client_array[i]);
 				else
-			#endif
 					acm_svr_receive(&client_array[i]);
 			}
 		}
@@ -3143,10 +3122,8 @@ int CDECL_FUNC main(int argc, char **argv)
 	acm_server();
 
 	acm_log(0, "shutting down\n");
-#ifdef HAVE_NETLINK
 	if (client_array[NL_CLIENT_INDEX].sock != INVALID_SOCKET)
 		close(client_array[NL_CLIENT_INDEX].sock);
-#endif
 	acm_close_providers();
 	acm_stop_sa_handler();
 	umad_done();
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* ibacm cleanups
From: Christoph Hellwig @ 2016-10-17 19:11 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA

This series fixed various sparse issues in ibacm, makes it use
the common list helpers and removes various cruft.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* RE: [PATCH rdma-core 2/4] glue/redhat: add udev/systemd/etc infrastructure bits
From: Weiny, Ira @ 2016-10-17 19:10 UTC (permalink / raw)
  To: Jason Gunthorpe, Jarod Wilson
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Doug Ledford,
	Hefty, Sean
In-Reply-To: <20161017185657.GA8122-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

 
> > > rdma-ndd dynamically sets the NodeDescription to the hostname in the
> > > adaptor for the subnet manager/tools to ready. I guess this hunk is
> > > setting the NodeDescription one-shot at boot..
> >
> > Rather than having this janky udev rule, what if we simply made
> > rdma-ndd part of what's installed with rdma-core, rather than
> > something found in yet another infiniband package? (Looks like it's in
> > infiniband-diags, wasn't even aware rdma-ndd existed until looking at this
> here).
> 
> Yep, very good idea.

Yep, very good idea...

> 
> Ira? What do you think?

Yes, I've been trying to find time to make a patch which adds rdma-ndd into the rdma-core.  I absolutely agree it should be part of rdma-core as it is much better than the 1 time shot of the start up scripts.

What I have been worried about is conflicts between infiniband-diags and the new rdma-core.  RH made a separate package out of rdma-ndd so that would be easy but I don't think other distros have.

So how do you obsolete "part" of a package?

Ira

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH rdma-core 4/4] glue/redhat/spec: build split rpm packages
From: Jason Gunthorpe @ 2016-10-17 19:07 UTC (permalink / raw)
  To: Jarod Wilson; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161017184506.GL14983-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Mon, Oct 17, 2016 at 02:45:06PM -0400, Jarod Wilson wrote:

> > > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libcxgb3-11-1.el7.x86_64.rpm
> > > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libcxgb4-11-1.el7.x86_64.rpm
> > > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libhfi1-11-1.el7.x86_64.rpm
> > > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libi40iw-11-1.el7.x86_64.rpm
> > > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libipathverbs-11-1.el7.x86_64.rpm
> > > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libmlx4-11-1.el7.x86_64.rpm
> > > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libmlx5-11-1.el7.x86_64.rpm
> > > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libmthca-11-1.el7.x86_64.rpm
> > > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libnes-11-1.el7.x86_64.rpm
> > > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libocrdma-11-1.el7.x86_64.rpm
> > > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/librxe-11-1.el7.x86_64.rpm
> > 
> > Does this mean RH is not going to do a single package for the
> > providers? You know another 5 or so are coming..
> 
> Putting all the providers in a single sub-package is certainly something
> we could do. With rpm virtual Provides and Obsoletes, we could make the
> upgrade path transparent to at least yum/dnf/rpm, but it might still
> confuse people who go looking for the same package they've always had
> installed, so it could be something we do in Fedora and the next RHEL
> major release, rather than in a current RHEL minor update.

I would strongly encoruage this - the current scheme is silly, you
need to know to look for the right provider to get things working
(which is very much unlike the way the kernel modules work).

Suggest to dump the providers in libiverbs or add a ibverbs-providers
package..

Or at least start adding new providers (Eg rxe,hfi) to a -providers package.

> > > +install -D -m0644 Documentation/{ibacm,ibsrpdm,libibcm,libibverbs,librdmacm,rxe}.md %{buildroot}/%{_docdir}/%{name}-%{version}/
> > > +install -D -m0644 README.md %{buildroot}/%{_docdir}/%{name}-%{version}/
> > 
> > I guess README should go in that patch I sent you..
> 
> D'oh, yeah, I'll mix that in here locally.

I think you can just add ../README.md to the
Documentation/CMakeFile.txt

> > > +%{buildroot}/%{_bindir}/ib_acme -D . -O
> > > +# Fixup a multilib conflict in ibacm_opts.cfg:
> > > +sed -i -e '/^# Specifies the directory of the provider libraries$/ a\
> > > +# Use /usr/lib64/ibacm on 64bit, /usr/lib/libacm on 32bit.
> > > +' -e 's%^\(# provider_lib_path
> > > /usr/\)lib\(64\)\?/ibacm$%\1lib64/ibacm%' ibacm_opts.cfg
> > 
> > Hum? I'm pretty sure this is basically fixed in the code now, was done here
> > 5eebdb9baaaae420a4bb16e586a96807823916a0
> > 
> > Adjusting the comment like that doesn't really make sense, the acm
> > daemon has a fixed endianness and looks in a single place to load the
> > plugin. If someone wants to use 32 bit plugins they have to install
> > the 32 bit acm daemon, which would have the 32 bit path in the sample
> > conf file...
> 
> This was just copy and paste from our existing libacm spec, might well be
> something that was fixed and I just missed it.

It looks OK as-is to me, I'd drop it now.

> They weren't getting installed, so I threw that hack in, meant to
> actually say something about that, got lost in the shuffle. I'll
> double-check if they're still not getting installed... Okay, they're
> getting installed now. May have just been a transient error with an
> earlier tarball, or something else I did wrong. I'll drop that bit.

How strange.. FWIW, the tarball should be built with git-archive ..

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: iscsi_trx going into D state
From: Robert LeBlanc @ 2016-10-17 19:03 UTC (permalink / raw)
  To: Zhu Lingshan
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAANLjFobXiBO2tXxTBB-8BQjM8FC0wmxdxQvEd6Rp=1LZkrvpA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

In addition, on the client we see:

----------------
Robert LeBlanc
PGP Fingerprint 79A2 9CA4 6CC4 45DD A904  C70E E654 3BB2 FA62 B9F1


On Mon, Oct 17, 2016 at 10:32 AM, Robert LeBlanc <robert-4JaGZRWAfWbajFs6igw21g@public.gmane.org> wrote:
> Some more info as we hit this this morning. We have volumes mirrored
> between two targets and we had one target on the kernel with the three
> patches mentioned in this thread [0][1][2] and the other was on a
> kernel without the patches. We decided that after a week and a half we
> wanted to get both targets on the same kernel so we rebooted the
> non-patched target. Within an hour we saw iSCSI in D state with the
> same stack trace so it seems that we are not hitting any of the
> WARN_ON lines. We are getting both iscsi_trx and iscsi_np both in D
> state, this time we have two iscsi_trx processes in D state. I don't
> know if stale sessions on the clients could be contributing to this
> issue (the target trying to close non-existent sessions??). This is on
> 4.4.23. Any more debug info we can throw at this problem to help?
>
> Thank you,
> Robert LeBlanc
>
> # ps aux | grep D | grep iscsi
> root     16525  0.0  0.0      0     0 ?        D    08:50   0:00 [iscsi_np]
> root     16614  0.0  0.0      0     0 ?        D    08:50   0:00 [iscsi_trx]
> root     16674  0.0  0.0      0     0 ?        D    08:50   0:00 [iscsi_trx]
>
> # for i in 16525 16614 16674; do echo $i; cat /proc/$i/stack; done
> 16525
> [<ffffffff814f0d5f>] iscsit_stop_session+0x19f/0x1d0
> [<ffffffff814e2516>] iscsi_check_for_session_reinstatement+0x1e6/0x270
> [<ffffffff814e4ed0>] iscsi_target_check_for_existing_instances+0x30/0x40
> [<ffffffff814e5020>] iscsi_target_do_login+0x140/0x640
> [<ffffffff814e63bc>] iscsi_target_start_negotiation+0x1c/0xb0
> [<ffffffff814e410b>] iscsi_target_login_thread+0xa9b/0xfc0
> [<ffffffff8109c748>] kthread+0xd8/0xf0
> [<ffffffff8172018f>] ret_from_fork+0x3f/0x70
> [<ffffffffffffffff>] 0xffffffffffffffff
> 16614
> [<ffffffff814cca79>] target_wait_for_sess_cmds+0x49/0x1a0
> [<ffffffffa064692b>] isert_wait_conn+0x1ab/0x2f0 [ib_isert]
> [<ffffffff814f0ef2>] iscsit_close_connection+0x162/0x870
> [<ffffffff814df9bf>] iscsit_take_action_for_connection_exit+0x7f/0x100
> [<ffffffff814f00a0>] iscsi_target_rx_thread+0x5a0/0xe80
> [<ffffffff8109c748>] kthread+0xd8/0xf0
> [<ffffffff8172018f>] ret_from_fork+0x3f/0x70
> [<ffffffffffffffff>] 0xffffffffffffffff
> 16674
> [<ffffffff814cca79>] target_wait_for_sess_cmds+0x49/0x1a0
> [<ffffffffa064692b>] isert_wait_conn+0x1ab/0x2f0 [ib_isert]
> [<ffffffff814f0ef2>] iscsit_close_connection+0x162/0x870
> [<ffffffff814df9bf>] iscsit_take_action_for_connection_exit+0x7f/0x100
> [<ffffffff814f00a0>] iscsi_target_rx_thread+0x5a0/0xe80
> [<ffffffff8109c748>] kthread+0xd8/0xf0
> [<ffffffff8172018f>] ret_from_fork+0x3f/0x70
> [<ffffffffffffffff>] 0xffffffffffffffff
>
>
> [0] https://www.spinics.net/lists/target-devel/msg13463.html
> [1] http://marc.info/?l=linux-scsi&m=147282568910535&w=2
> [2] http://www.spinics.net/lists/linux-scsi/msg100221.html
> ----------------
> Robert LeBlanc
> PGP Fingerprint 79A2 9CA4 6CC4 45DD A904  C70E E654 3BB2 FA62 B9F1
>
>
> On Fri, Oct 7, 2016 at 8:59 PM, Zhu Lingshan <lszhu-IBi9RG/b67k@public.gmane.org> wrote:
>> Hi Robert,
>>
>> I also see this issue, but this is not the only code path can trigger this
>> problem, I think you may also see iscsi_np in D status. I fixed one code
>> path whitch still not merged to mainline. I will forward you my patch later.
>> Note: my patch only fixed one code path, you may see other call statck with
>> D status.
>>
>> Thanks,
>> BR
>> Zhu Lingshan
>>
>>
>> 在 2016/10/1 1:14, Robert LeBlanc 写道:
>>>
>>> We are having a reoccurring problem where iscsi_trx is going into D
>>> state. It seems like it is waiting for a session tear down to happen
>>> or something, but keeps waiting. We have to reboot these targets on
>>> occasion. This is running the 4.4.12 kernel and we have seen it on
>>> several previous 4.4.x and 4.2.x kernels. There is no message in dmesg
>>> or /var/log/messages. This seems to happen with increased frequency
>>> when we have a disruption in our Infiniband fabric, but can happen
>>> without any changes to the fabric (other than hosts rebooting).
>>>
>>> # ps aux | grep iscsi | grep D
>>> root      4185  0.0  0.0      0     0 ?        D    Sep29   0:00
>>> [iscsi_trx]
>>> root     18505  0.0  0.0      0     0 ?        D    Sep29   0:00
>>> [iscsi_np]
>>>
>>> # cat /proc/4185/stack
>>> [<ffffffff814cc999>] target_wait_for_sess_cmds+0x49/0x1a0
>>> [<ffffffffa087292b>] isert_wait_conn+0x1ab/0x2f0 [ib_isert]
>>> [<ffffffff814f0de2>] iscsit_close_connection+0x162/0x840
>>> [<ffffffff814df8df>] iscsit_take_action_for_connection_exit+0x7f/0x100
>>> [<ffffffff814effc0>] iscsi_target_rx_thread+0x5a0/0xe80
>>> [<ffffffff8109c6f8>] kthread+0xd8/0xf0
>>> [<ffffffff8172004f>] ret_from_fork+0x3f/0x70
>>> [<ffffffffffffffff>] 0xffffffffffffffff
>>>
>>> # cat /proc/18505/stack
>>> [<ffffffff814f0c71>] iscsit_stop_session+0x1b1/0x1c0
>>> [<ffffffff814e2436>] iscsi_check_for_session_reinstatement+0x1e6/0x270
>>> [<ffffffff814e4df0>] iscsi_target_check_for_existing_instances+0x30/0x40
>>> [<ffffffff814e4f40>] iscsi_target_do_login+0x140/0x640
>>> [<ffffffff814e62dc>] iscsi_target_start_negotiation+0x1c/0xb0
>>> [<ffffffff814e402b>] iscsi_target_login_thread+0xa9b/0xfc0
>>> [<ffffffff8109c6f8>] kthread+0xd8/0xf0
>>> [<ffffffff8172004f>] ret_from_fork+0x3f/0x70
>>> [<ffffffffffffffff>] 0xffffffffffffffff
>>>
>>> What can we do to help get this resolved?
>>>
>>> Thanks,
>>>
>>> ----------------
>>> Robert LeBlanc
>>> PGP Fingerprint 79A2 9CA4 6CC4 45DD A904  C70E E654 3BB2 FA62 B9F1
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH rdma-core 2/4] glue/redhat: add udev/systemd/etc infrastructure bits
From: Jason Gunthorpe @ 2016-10-17 18:56 UTC (permalink / raw)
  To: Jarod Wilson
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Doug Ledford, Weiny, Ira,
	Hefty, Sean
In-Reply-To: <20161017182037.GK14983-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Mon, Oct 17, 2016 at 02:20:37PM -0400, Jarod Wilson wrote:

> > > > > diff --git a/glue/redhat/ibacm.service b/glue/redhat/ibacm.service
> > > > > new file mode 100644
> > > > > index 0000000..1cd031a
> > > > > +++ b/glue/redhat/ibacm.service
> > > > 
> > > > Can we just put this in ibacm/ ?
> > > 
> > > Probably.
> > 
> > Okay, the only thing I really don't like being upstream is the
> > opensm.service..
> > 
> > Do you know why acm needs that?
> 
> I think Doug already attempted to address this elsewhere in the thread,
> and he'd know better than me.

srp_daemon not being able to handle a change in prefix makes sense.

But that doesn't explain what problem ibacm has..

To my mind, depending on something like opensm indicates the daemon
has a bug - eg it cannot handle dynamic subnet changes. So lets at
least be clear on what the bugs we are working around are, ask if they
have been fixed, etc.

Sean, do you know why ibacm would need to be started after opensm?

> > Okay. The trick will be to standardize the systemd_wants name ..
> 
> Perhaps rdma-core should go with rdma-core.service? We were shipping a
> package called 'rdma' that carried that.

Maybe, but I'd like to have an overall systemd plan..

If we can't have a .target then sure, this is probably the best
way..

> > rdma-ndd dynamically sets the NodeDescription to the hostname in the
> > adaptor for the subnet manager/tools to ready. I guess this hunk is
> > setting the NodeDescription one-shot at boot..
> 
> Rather than having this janky udev rule, what if we simply made rdma-ndd
> part of what's installed with rdma-core, rather than something found in
> yet another infiniband package? (Looks like it's in infiniband-diags,
> wasn't even aware rdma-ndd existed until looking at this here).

Yep, very good idea.

Ira? What do you think?

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH rdma-core 2/4] glue/redhat: add udev/systemd/etc infrastructure bits
From: Doug Ledford @ 2016-10-17 18:56 UTC (permalink / raw)
  To: Jarod Wilson, Jason Gunthorpe; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161017182037.GK14983-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 4362 bytes --]

On 10/17/2016 2:20 PM, Jarod Wilson wrote:
> On Mon, Oct 17, 2016 at 11:46:11AM -0600, Jason Gunthorpe wrote:
>> On Mon, Oct 17, 2016 at 12:22:21PM -0400, Jarod Wilson wrote:

>>>>> diff --git a/glue/redhat/ibacm.service b/glue/redhat/ibacm.service
>>>>> new file mode 100644
>>>>> index 0000000..1cd031a
>>>>> +++ b/glue/redhat/ibacm.service
>>>>
>>>> Can we just put this in ibacm/ ?
>>>
>>> Probably.
>>
>> Okay, the only thing I really don't like being upstream is the
>> opensm.service..
>>
>> Do you know why acm needs that?
> 
> I think Doug already attempted to address this elsewhere in the thread,
> and he'd know better than me.

Since it got snipped, I'm pretty sure it just lists opensm in the Wants=
tag.  With systemd, that's a soft dependency.  Only if opensm is
installed and configured to start anyway does systemd then order this
item after opensm.  Since you need opensm for links to come up anyway,
it makes sense for the order of startup for RDMA related items to be:

rdma-core
\-opensm
  \-Everything else

Because of that, I think I had opensm listed as a want in pretty much
everything, but as already stated, it's a soft dependency and systemd
ignores it if you don't have opensm configured to run on that machine.


>>>>> +SUBSYSTEM=="module", KERNEL=="cxgb*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
>>>>> +SUBSYSTEM=="module", KERNEL=="ib_*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
>>>>> +SUBSYSTEM=="module", KERNEL=="mlx*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
>>>>> +SUBSYSTEM=="module", KERNEL=="iw_*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
>>>>> +SUBSYSTEM=="module", KERNEL=="be2net", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
>>>>> +SUBSYSTEM=="module", KERNEL=="enic", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
>>>>
>>>> Also cross distro
>>>
>>> Yeah, these are definitely prime candidates for being cross-distro. And
>>> really, I was thinking maybe these should be part of the core upstream
>>> udev/systemd rules set, rather than something we ship here.
>>
>> Okay. The trick will be to standardize the systemd_wants name ..
> 
> Perhaps rdma-core should go with rdma-core.service? We were shipping a
> package called 'rdma' that carried that.

Whatever the service file is, that's what the name is.  I'm partial to
just leaving it as rdma.service.  The -core suffix doesn't add anything
of value IMO, and we really are initializing the entire rdma stack minus
just those upper layer protocols that have their own setup.

>>>>> +# When we detect a new verbs device is added to the system, set the node
>>>>> +# description on that device
>>>>> +# If rdma-ndd is installed, defer the setting of the node description to it.
>>>>> +SUBSYSTEM=="infiniband", KERNEL=="*", ACTION=="add", TEST!="/usr/sbin/rdma-ndd", RUN+="/bin/bash -c 'sleep 1; echo -n `hostname -s` %k > /sys/class/infiniband/%k/node_desc'"
>>>>
>>>> Shouldn't this udev drop-in by in the rdma-ndd package?
>>>
>>> Honestly don't even have a clue what rdma-ndd is. :) Don't remember what
>>> Doug said about this one...
>>
>> Guess I didn't read closely enough, this is used if rdma-ndd is not
>> installed..

A bit of a chicken and egg issue here isn't there?  Wasn't rdma-ndd part
of ibutils?  And since ibutils isn't in rdma-core, how do we know if
it's installed?

>> So something like that should be upstream, but the 'sleep 1' is ugly -
>> this should probably be a systemd service that runs after
>> network-online.target not as a script run from udev?
>>
>> rdma-ndd dynamically sets the NodeDescription to the hostname in the
>> adaptor for the subnet manager/tools to ready. I guess this hunk is
>> setting the NodeDescription one-shot at boot..
> 
> Rather than having this janky udev rule, what if we simply made rdma-ndd
> part of what's installed with rdma-core, rather than something found in
> yet another infiniband package? (Looks like it's in infiniband-diags,
> wasn't even aware rdma-ndd existed until looking at this here).

It's probably a good candidate to be pulled back to rdma-core.  Ira?

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG Key ID: 0E572FDD


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

^ permalink raw reply

* Re: [PATCH rdma-core 4/4] glue/redhat/spec: build split rpm packages
From: Jarod Wilson @ 2016-10-17 18:45 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161014233904.GD16509-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

On Fri, Oct 14, 2016 at 05:39:04PM -0600, Jason Gunthorpe wrote:
> On Fri, Oct 14, 2016 at 03:21:36PM -0400, Jarod Wilson wrote:
> 
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/iwpmd-11-1.el7.x86_64.rpm
> 
> FC used to call this libiwpm for some reason..

Ah. I may need to add an Obsoletes: for that particular case then.

> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libibcm-11-1.el7.x86_64.rpm
> 
> Do you want to try and do as I did with debian and put the SONAME data
> into the package version for the libraries? eg
> 
> libibcm1_1.0.11-1_amd64.deb

No, that's a debian-ism that Red Hat has never really followed. The soname
data is already captured in the rpm metadata.

> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libcxgb3-11-1.el7.x86_64.rpm
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libcxgb4-11-1.el7.x86_64.rpm
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libhfi1-11-1.el7.x86_64.rpm
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libi40iw-11-1.el7.x86_64.rpm
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libipathverbs-11-1.el7.x86_64.rpm
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libmlx4-11-1.el7.x86_64.rpm
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libmlx5-11-1.el7.x86_64.rpm
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libmthca-11-1.el7.x86_64.rpm
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libnes-11-1.el7.x86_64.rpm
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libocrdma-11-1.el7.x86_64.rpm
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/librxe-11-1.el7.x86_64.rpm
> 
> Does this mean RH is not going to do a single package for the
> providers? You know another 5 or so are coming..

Putting all the providers in a single sub-package is certainly something
we could do. With rpm virtual Provides and Obsoletes, we could make the
upgrade path transparent to at least yum/dnf/rpm, but it might still
confuse people who go looking for the same package they've always had
installed, so it could be something we do in Fedora and the next RHEL
major release, rather than in a current RHEL minor update.

> Any feedback if things build on your supported arches? I'd be
> interested to see compiler log if it isn't silent.
> 
> > +install -D -m0644 Documentation/{ibacm,ibsrpdm,libibcm,libibverbs,librdmacm,rxe}.md %{buildroot}/%{_docdir}/%{name}-%{version}/
> > +install -D -m0644 README.md %{buildroot}/%{_docdir}/%{name}-%{version}/
> 
> I guess README should go in that patch I sent you..

D'oh, yeah, I'll mix that in here locally.

> > +%{buildroot}/%{_bindir}/ib_acme -D . -O
> > +# Fixup a multilib conflict in ibacm_opts.cfg:
> > +sed -i -e '/^# Specifies the directory of the provider libraries$/ a\
> > +# Use /usr/lib64/ibacm on 64bit, /usr/lib/libacm on 32bit.
> > +' -e 's%^\(# provider_lib_path
> > /usr/\)lib\(64\)\?/ibacm$%\1lib64/ibacm%' ibacm_opts.cfg
> 
> Hum? I'm pretty sure this is basically fixed in the code now, was done here
> 5eebdb9baaaae420a4bb16e586a96807823916a0
> 
> Adjusting the comment like that doesn't really make sense, the acm
> daemon has a fixed endianness and looks in a single place to load the
> plugin. If someone wants to use 32 bit plugins they have to install
> the 32 bit acm daemon, which would have the 32 bit path in the sample
> conf file...

This was just copy and paste from our existing libacm spec, might well be
something that was fixed and I just missed it.

> > +install -D -m0644 ibacm/man/*.1 %{buildroot}/%{_mandir}/man1/
> > +install -D -m0644 ibacm/man/*.7 %{buildroot}/%{_mandir}/man7/
> > +sed -e 's|@ACM_PROVIDER_DIR@|%{libdir}/%{name}/|g' -e 's|@CMAKE_INSTALL_FULL_LOCALSTATEDIR@|%{_localstatedir}|g' ibacm/man/ibacm_prov.7.in > %{buildroot}/%{_mandir}/man7/ibacm_prov.7
> 
> ?? cmake is supposed to install these.. Looks like it works. What did
> you need done here to open code it?

They weren't getting installed, so I threw that hack in, meant to
actually say something about that, got lost in the shuffle. I'll
double-check if they're still not getting installed... Okay, they're
getting installed now. May have just been a transient error with an
earlier tarball, or something else I did wrong. I'll drop that bit.

-- 
Jarod Wilson
jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* RE: RQ overflow seen running isert traffic
From: Steve Wise @ 2016-10-17 18:29 UTC (permalink / raw)
  To: 'Potnuri Bharat Teja', 'Sagi Grimberg'
  Cc: target-devel, nab, linux-rdma
In-Reply-To: <20161017111655.GA21245@chelsio.com>

> On Wednesday, October 10/05/16, 2016 at 11:44:12 +0530, Sagi Grimberg wrote:
> >
> > > Hi Sagi,
> >
> > Hey Baharat,
> >
> > Sorry for the late response, its the holiday
> > season in Israel...
> >
> > > I've been trying to understand the isert functionality with respect to
> > > RDMA Receive Queue sizing and Queue full handling. Here is the problem
> > > is see with iw_cxgb4:
> > >
> > > After running few minutes of iSER traffic with iw_cxgb4, I am seeing
> > > post receive failures due to receive queue full returning -ENOMEM.
> > > In case of iw_cxgb4 the RQ size is 130 with qp attribute max_recv_wr =
129,
> > > passed down by isert to iw_cxgb4.isert decides on max_recv_wr as 129 based
> > > on (ISERT_QP_MAX_RECV_DTOS = ISCSI_DEF_XMIT_CMDS_MAX = 128) + 1.
> >
> > That's correct.
> 
> Hi Sagi,
> Sorry for the late reply, I had to recheck my findings before I reply
> you back.
> 
> My interpretation of the queue full issue was not complete, got carried
> away by the recieve queue and missed the SQ failure due to full among
> the debug logs.
> 
> Here is what was happening:
> It was the SQ full first and fails to get posted with ENOMEM, due to this
> the command is queued to queue full list this will schedule it to post at
> later instance and this repeated try for posting will cause the rq to get
> full. For everyfurther try sq post will any way fail and an extra rq wr is
> posted as a part of datain() leading it to be full too.
> 
> This happened a bit earlier in my case, since I corrected the the
> iser_put_datain() to return error to LIO.
> 
> Here is the failure log on a fresh 4.8 kernel:
> isert: isert_rdma_rw_ctx_post: Cmd: ffff882ec8c96e60 failed to post RDMA
> res <===here is the post send failure due to ENOMEM.
> ABORT_TASK: Found referenced iSCSI task_tag: 33
> ABORT_TASK: Sending TMR_FUNCTION_COMPLETE for ref_tag: 33
> isert: isert_post_recv: ib_post_recv() failed with ret: -22
> isert: isert_post_response: ib_post_recv failed with -22
> isert: isert_post_recv: ib_post_recv() failed with ret: -22
> isert: isert_post_response: ib_post_recv failed with -22
> isert: isert_post_recv: ib_post_recv() failed with ret: -22
> isert: isert_post_response: ib_post_recv failed with -22
> isert: isert_post_recv: ib_post_recv() failed with ret: -22
> isert: isert_post_response: ib_post_recv failed with -22
> isert: isert_post_recv: ib_post_recv() failed with ret: -22
> isert: isert_post_response: ib_post_recv failed with -22
> cxgb4 0000:84:00.4: AE qpid 1026 opcode 10 status 0x1 type 1 len 0x0
> wrid.hi 0x0 wrid.lo 0x4c01
> isert: isert_qp_event_callback: QP access error (3): conn
> ffff8817ddc46000
> iSCSI Login timeout on Network Portal 10.40.40.198:3260
> INFO: task iscsi_np:14744 blocked for more than 120 seconds.
> 
> Here are the QP and CQ stats before my assert for queue full:
> qhp->attr.sq_num_entries 523
> qhp->attr.rq_num_entries 129
> qhp->wq.sq.qid 1026
> qhp->wq.rq.qid 1027
> qhp->wq.sq.in_use 523 <=== SQ to the brim
> qhp->wq.sq.size 524 <=== SQ size
> qhp->wq.sq.cidx 391
> qhp->wq.sq.pidx 390
> qhp->wq.sq.wq_pidx 202
> qhp->wq.sq.wq_pidx_inc 0
> qhp->wq.sq.flush_cidx 391
> qhp->wq.rq.in_use 128
> qhp->wq.rq.size 130
> qhp->wq.rq.cidx 112
> qhp->wq.rq.pidx 110
> qhp->wq.rq.wq_pidx 240
> qhp->wq.rq.wq_pidx_inc 0
> qhp->wq.flushed 0
> chp->cq.cqid 1024
> chp->cq.size 6335
> chp->cq.cidx 4978
> chp->cq.sw_cidx 4126
> chp->cq.sw_pidx 4126
> chp->cq.sw_in_use 0
> chp->cq.cidx_inc 0
> 
> As an experiment I tried increasing the ISCSI_DEF_XMIT_CMDS_MAX to 256
> instead of 128, which incase of iwarp creates SQ with size 1548, the
> issue is not seen.
> I doubt if the SQ is not sized properly incase of iWARP Or factored
> incorrectly in the rdma api for iser IOP.
> 
> I am digging for root cause.
> 
> Thanks for your time,
> Bharat.
> 

Hey Sagi, I'm looking at isert_create_qp() and it appears to not be correctly
sizing the SQ:

...
#define ISERT_QP_MAX_REQ_DTOS   (ISCSI_DEF_XMIT_CMDS_MAX +    \
                                ISERT_MAX_TX_MISC_PDUS  + \
                                ISERT_MAX_RX_MISC_PDUS)
...
        attr.cap.max_send_wr = ISERT_QP_MAX_REQ_DTOS + 1;
        attr.cap.max_recv_wr = ISERT_QP_MAX_RECV_DTOS + 1;
...

I think above snipit assumes a DTO consumes exactly one WR/WQE in the SQ.  But
the DTO can be broken into multiple WRs to handle REG_MRs, multiple WRITE or
READ WRs due to limits on local sge depths target sge depths, etc.  Yes?  Or am
I all wet?  Or perhaps isert doesn't require the SQ to be the max possible
because it flow controls the DTO submissions? 

Stevo

^ permalink raw reply

* Re: [PATCH rdma-core 2/4] glue/redhat: add udev/systemd/etc infrastructure bits
From: Jarod Wilson @ 2016-10-17 18:20 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Doug Ledford
In-Reply-To: <20161017174611.GB6430-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

On Mon, Oct 17, 2016 at 11:46:11AM -0600, Jason Gunthorpe wrote:
> On Mon, Oct 17, 2016 at 12:22:21PM -0400, Jarod Wilson wrote:
> > Worksforme. As mentioned in reply to Leon, I really don't care what the
> > directory is called, just that we get this stuff out there, instead of
> > being in our own custom glue package. I do think maybe Leon's suggestion
> > of having a directory to put a bunch of distro-specific directories under
> > would be good for cleanliness, but I also seem to recall that debian
> > actually looks for a directory in the root of the tarball, so it may need
> > to stay like it is.
> 
> Okay, let us just do a very minimal upstreaming and tackle things from
> there..

Sounds like a plan.

> > > Common stuff should be installed via cmake
> > > 
> > > > diff --git a/glue/redhat/ibacm.service b/glue/redhat/ibacm.service
> > > > new file mode 100644
> > > > index 0000000..1cd031a
> > > > +++ b/glue/redhat/ibacm.service
> > > 
> > > Can we just put this in ibacm/ ?
> > 
> > Probably.
> 
> Okay, the only thing I really don't like being upstream is the
> opensm.service..
> 
> Do you know why acm needs that?

I think Doug already attempted to address this elsewhere in the thread,
and he'd know better than me.

> > > > +++ b/glue/redhat/iwpmd.service
> > > > @@ -0,0 +1,12 @@
> > > > +[Unit]
> > > > +Description=Starts the IWPMD daemon
> > > > +Documentation=file:///usr/share/doc/iwpmd/README
> > > 
> > > File does not exit? There is a man page now
> > > 
> > > We already have a iwpmd/iwpmd.service that is almost identical, can you
> > > you just update it and drop this version?
> > 
> > Bah, that's a carryover from our individually packaged iwpmd, didn't look
> > closely enough. Can we merge a bit from ours into the 'stock' one? The
> > main relevant difference I see is we have ours set to load after
> > syslog.target as well as network.target.
> 
> syslog.target is obsolete now, just drop it from all your unit files
> unless you need to support systemd <= 35. systemd no longer even
> documents this special target exists.
> 
> commit e8f2b5c11e9db0bab2654e75c2558955effb82fe
> Author: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> Date:   Mon Sep 12 15:53:10 2016 -0600
> 
>     iwpmd: Remove syslog.target from service file
>     
>     Debian's Lintian remarks:
>     
>     W: rdma-plumbing: systemd-service-file-refers-to-obsolete-target lib/systemd/system/iwpmd.service syslog.target
>     
>     Apparently systemd stopped recommending this in version 35, socket activation
>     eliminates the need.
>     
>     Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

Ah, okay. So not needed for any distro that is even remotely modern.

> > > > +SUBSYSTEM=="module", KERNEL=="cxgb*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > > > +SUBSYSTEM=="module", KERNEL=="ib_*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > > > +SUBSYSTEM=="module", KERNEL=="mlx*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > > > +SUBSYSTEM=="module", KERNEL=="iw_*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > > > +SUBSYSTEM=="module", KERNEL=="be2net", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > > > +SUBSYSTEM=="module", KERNEL=="enic", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > > 
> > > Also cross distro
> > 
> > Yeah, these are definitely prime candidates for being cross-distro. And
> > really, I was thinking maybe these should be part of the core upstream
> > udev/systemd rules set, rather than something we ship here.
> 
> Okay. The trick will be to standardize the systemd_wants name ..

Perhaps rdma-core should go with rdma-core.service? We were shipping a
package called 'rdma' that carried that.

> > > > +# When we detect a new verbs device is added to the system, set the node
> > > > +# description on that device
> > > > +# If rdma-ndd is installed, defer the setting of the node description to it.
> > > > +SUBSYSTEM=="infiniband", KERNEL=="*", ACTION=="add", TEST!="/usr/sbin/rdma-ndd", RUN+="/bin/bash -c 'sleep 1; echo -n `hostname -s` %k > /sys/class/infiniband/%k/node_desc'"
> > > 
> > > Shouldn't this udev drop-in by in the rdma-ndd package?
> > 
> > Honestly don't even have a clue what rdma-ndd is. :) Don't remember what
> > Doug said about this one...
> 
> Guess I didn't read closely enough, this is used if rdma-ndd is not
> installed..
> 
> So something like that should be upstream, but the 'sleep 1' is ugly -
> this should probably be a systemd service that runs after
> network-online.target not as a script run from udev?
> 
> rdma-ndd dynamically sets the NodeDescription to the hostname in the
> adaptor for the subnet manager/tools to ready. I guess this hunk is
> setting the NodeDescription one-shot at boot..

Rather than having this janky udev rule, what if we simply made rdma-ndd
part of what's installed with rdma-core, rather than something found in
yet another infiniband package? (Looks like it's in infiniband-diags,
wasn't even aware rdma-ndd existed until looking at this here).

-- 
Jarod Wilson
jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH rdma-core 2/4] glue/redhat: add udev/systemd/etc infrastructure bits
From: Jason Gunthorpe @ 2016-10-17 17:46 UTC (permalink / raw)
  To: Jarod Wilson; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Doug Ledford
In-Reply-To: <20161017162221.GI14983-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Mon, Oct 17, 2016 at 12:22:21PM -0400, Jarod Wilson wrote:
> Worksforme. As mentioned in reply to Leon, I really don't care what the
> directory is called, just that we get this stuff out there, instead of
> being in our own custom glue package. I do think maybe Leon's suggestion
> of having a directory to put a bunch of distro-specific directories under
> would be good for cleanliness, but I also seem to recall that debian
> actually looks for a directory in the root of the tarball, so it may need
> to stay like it is.

Okay, let us just do a very minimal upstreaming and tackle things from
there..

> > Common stuff should be installed via cmake
> > 
> > > diff --git a/glue/redhat/ibacm.service b/glue/redhat/ibacm.service
> > > new file mode 100644
> > > index 0000000..1cd031a
> > > +++ b/glue/redhat/ibacm.service
> > 
> > Can we just put this in ibacm/ ?
> 
> Probably.

Okay, the only thing I really don't like being upstream is the
opensm.service..

Do you know why acm needs that?

> > > +++ b/glue/redhat/iwpmd.service
> > > @@ -0,0 +1,12 @@
> > > +[Unit]
> > > +Description=Starts the IWPMD daemon
> > > +Documentation=file:///usr/share/doc/iwpmd/README
> > 
> > File does not exit? There is a man page now
> > 
> > We already have a iwpmd/iwpmd.service that is almost identical, can you
> > you just update it and drop this version?
> 
> Bah, that's a carryover from our individually packaged iwpmd, didn't look
> closely enough. Can we merge a bit from ours into the 'stock' one? The
> main relevant difference I see is we have ours set to load after
> syslog.target as well as network.target.

syslog.target is obsolete now, just drop it from all your unit files
unless you need to support systemd <= 35. systemd no longer even
documents this special target exists.

commit e8f2b5c11e9db0bab2654e75c2558955effb82fe
Author: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Date:   Mon Sep 12 15:53:10 2016 -0600

    iwpmd: Remove syslog.target from service file
    
    Debian's Lintian remarks:
    
    W: rdma-plumbing: systemd-service-file-refers-to-obsolete-target lib/systemd/system/iwpmd.service syslog.target
    
    Apparently systemd stopped recommending this in version 35, socket activation
    eliminates the need.
    
    Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

> > > +SUBSYSTEM=="module", KERNEL=="cxgb*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > > +SUBSYSTEM=="module", KERNEL=="ib_*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > > +SUBSYSTEM=="module", KERNEL=="mlx*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > > +SUBSYSTEM=="module", KERNEL=="iw_*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > > +SUBSYSTEM=="module", KERNEL=="be2net", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > > +SUBSYSTEM=="module", KERNEL=="enic", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > 
> > Also cross distro
> 
> Yeah, these are definitely prime candidates for being cross-distro. And
> really, I was thinking maybe these should be part of the core upstream
> udev/systemd rules set, rather than something we ship here.

Okay. The trick will be to standardize the systemd_wants name ..

> > > +# When we detect a new verbs device is added to the system, set the node
> > > +# description on that device
> > > +# If rdma-ndd is installed, defer the setting of the node description to it.
> > > +SUBSYSTEM=="infiniband", KERNEL=="*", ACTION=="add", TEST!="/usr/sbin/rdma-ndd", RUN+="/bin/bash -c 'sleep 1; echo -n `hostname -s` %k > /sys/class/infiniband/%k/node_desc'"
> > 
> > Shouldn't this udev drop-in by in the rdma-ndd package?
> 
> Honestly don't even have a clue what rdma-ndd is. :) Don't remember what
> Doug said about this one...

Guess I didn't read closely enough, this is used if rdma-ndd is not
installed..

So something like that should be upstream, but the 'sleep 1' is ugly -
this should probably be a systemd service that runs after
network-online.target not as a script run from udev?

rdma-ndd dynamically sets the NodeDescription to the hostname in the
adaptor for the subnet manager/tools to ready. I guess this hunk is
setting the NodeDescription one-shot at boot..

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH rdma-core 2/4] glue/redhat: add udev/systemd/etc infrastructure bits
From: Jason Gunthorpe @ 2016-10-17 17:31 UTC (permalink / raw)
  To: Doug Ledford; +Cc: Jarod Wilson, linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1e03ceea-5584-6bbb-6fbe-e11d9d251649-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Sun, Oct 16, 2016 at 10:40:27AM -0400, Doug Ledford wrote:
> >> +Requires=rdma.service
> >> +After=rdma.service opensm.service
> > 
> > This is the only RH specific thiing I see.. Could we standardize on
> > something here and use it on all distros? rdma-available.target?
> 
> You can't, unless you rename the rdma.service unit file to something
> else.  They are tied in that way.

Well, I don't really care about names too much, rdma-whatever.target
is fine...

> >> +++ b/glue/redhat/rdma.cxgb4.sys.modprobe
> >> @@ -0,0 +1 @@
> >> +install cxgb4 /sbin/modprobe --ignore-install cxgb4 $CMDLINE_OPTS && /sbin/modprobe iw_cxgb4
> > 
> > What are these for? Should they be cross distro? Why are only a few
> > drivers this special?
> 
> We have one of these for every two (or more) part driver.  They aren't
> special, it's just the multipart drivers that are.

So, should we move them into the provider directories? Or patch some
kind of request_module into the kernel?

> > I wonder if this could be split into a generic 'load the modules' part
> > and a distro specific part? Every distro needs systemd to load the
> > extra modules because out auto-loading is broken - IMHO, and that is
> > pretty complex unfortunately.
> 
> Yes, this probably could be broken out.

So, I think the 'systemd way' would be a rdma-load-modules.service
onshot and a rdma-whatever.target

This way a distro can add their other stuff with additional drops ins,
eg rdma-bios-fixup.service (after load-modules, before
rdma-whatever.target)

> >> +[Unit]
> >> +Description=Initialize the iWARP/InfiniBand/RDMA stack in the kernel
> >> +Documentation=file:/etc/rdma/rdma.conf
> >> +RefuseManualStop=true
> >> +DefaultDependencies=false
> >> +Conflicts=emergency.target emergency.service
> >> +Before=network.target remote-fs-pre.target
> > 
> > This is an area we really need to cross-distro standardize - we really
> > need a set of rdma-*.targets.
> > 
> > eg
> >  rdma-available.target
> >    - RDMA hardware is available and all prep is done
> >      opensm (if installed) is started, etc
> >      Use in place of rdma.service
> >   rdma-detected.target
> >    - udev detected rdma hardware
> 
> It's not that easy, unfortunately.  Creating a target is a big deal.

Okay, do you mean big deal in the sense we need to get approval from
systemd folks or something? We are a big grown up subsystem now, and
good systemd integration is very important to a good user experience
these days.

I think we are in a better place now, because the target(s) *really*
needs to be cross distro and maintained 'upstream' - rdma-core is the
natural place to do that.

> I could be wrong).  I would have thought it means "Start this unit
> before starting the target listed in the Before= line", instead it
> means "Start this unit and make sure it finishes before the target
> in the Before= line is considered complete".  It can be started
> after the listed target is started, but the listed target won't be
> considered complete until it is also complete.

I'm not sure I follow the issue?

Your description matches how I understand systemd - a .target will not
become ready until all the prerequisits reach a 'ready' state (eg a
oneshot script completes). As the target does not become 'ready' until
its prerequisites are all 'ready', and dependents never start until
the parent is 'ready', this provides a reliable ordering sequence
point in the startup.

The order of starting is simply that target prerequisites are started
before the target becomes ready.

When systemd enabling anything it is important to keep in mind the
distinction between 'started' and 'ready' - and broadly speaking, our
daemons do not do this correctly today :/.

So the design goal is to make a target(s) that indicates enough of the
RDMA core systems is 'ready' so that we can begin to start things that
use rdmacm, etc.

We have problems with our daemons not properly interacting with
systemd to indicate 'ready', and that will cause bugs, but the overall
idea should be sound.

So this is a sketch of what I am thinking about.

rdma-fix-bios.service:
 [Unit]
 Type=oneshot
 Before=rdma-available.target, rdma-load-modules.service
rdma-load-modules.service:
 [Unit]
 Type=oneshot
 Before=rdma-available.target
iwpmd.service:
 [Unit]
 After=rdma-load-modules.service
 Before=rdma-available.target
opensm.service:
 [Unit]
 After=rdma-load-modules.service
 Before=rdma-available.target

rdma-available.target:
 [Unit]
 Description=Target indicating that the RDMA kernel stack is setup for user use.

srp_daemon.service:
 [Unit]
 After=rdma-available.target
 Before=remote-fs-pre.target

'Type=oneshot' will prevent anything past rdma-available.target from
starting until the scripts complete.

Internal ordering in the 'before' section has stuff like opensm and
iwpmd taken care of, and all 'user' daemons have a clear single
.target to depend on that works no matter what the distro or
underlying RDMA protocol.

To be clear, I'm proposing something like this as a goal, there will
certainly be some needed work on the C daemons to get there:
 - iwpmd forks in the wrong place, it needs to fork after it sets up
   netlink, or stop forking and use sd_notify. (or even better, we
   should figure out how to use ListenNetlink !!)
 - ibacmd needs to use socket activation/sd_notify/fork order to ensure
   acm is started before rdma cm users start
 - srp_daemon needs to respond to dynamic prefix changes and probably
   use sd_notify/fork order to indicate that it is OK to move on to
   mounting FS.

Why is this more important now?
 1) There are more SM's than opensm, it makes those peoples lives
    very hard if 'opensm' is hardcoded into all the service files for
    correctness, hard to swap out opensm with something else. Eg hfi
    does not use opensm.
 2) iwarp is involved in all of this too, and we need to start iwpmd
    before moving on to other services that might need rdmacm. Ditto
    for ibacm
 3) Things like rxe could use additional 'before' service plugins to
    enable rxe mode on interfaces.

So, I think this is a subject worth tackling.. (over the long term,
let us not block Jarod's stuff)

The goal would be to standardize the .target names and be able to use
upstream .service files for many of the things, and allow
distros/users/other to reliably 'drop in' additional stuff (eg the
bios-fixup) at various well defined sequence points.

> Fortunately, the targets listed in the unit files are pretty standard
> (they are part of the systemd upstream), and so I think they can be
> cross distro just as they are.

Sure, the pre-existing targets are, it is stuff like opensm.service
that seems off to me.

> >> +Description=Start or stop the daemon that attaches to SRP devices
> >> +Documentation=file:///etc/rdma/rdma.conf file:///etc/srp_daemon.conf
> >> +DefaultDependencies=false
> >> +Conflicts=emergency.target emergency.service
> >> +Requires=rdma.service
> >> +Wants=opensm.service
> >> +After=rdma.service opensm.service
> >> +After=network.target
> >> +Before=remote-fs-pre.target
> > 
> > Also should be common, why does it reference opensm.service?
> 
> Because if opensm is running on this host, then it must be up before the
> configured srp targets are valid any time there is a non-default subnet
> prefix.

Well, that kinda sounds like a srp_daemon bug - how does it work
race-free with an external SM?

Even with an on-node opensm, how does this work without a race?  Is
After=opensm.service enough to assert that opensm has completed a
sweep and assigned the subnet prefix?

If we can have srp_daemon respond to dynamic changes in the subnet
prefix can we drop this from the unit file?

> >> +[Service]
> >> +Type=simple
> >> +ExecStart=/usr/sbin/srp_daemon.sh
> > 
> > Hurm, someday we have to make better systemd integration for these
> > daemons..
> 
> There really isn't any better integration to get with our complex
> daemons unless we update the daemons themselves to get rid of their
> shell script starters...

Exactly, update the daemons.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 0/8] infiniband: Remove semaphores
From: Bart Van Assche @ 2016-10-17 16:57 UTC (permalink / raw)
  To: Binoy Jayan, Doug Ledford, Sean Hefty, Hal Rosenstock
  Cc: Arnd Bergmann, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476721862-7070-1-git-send-email-binoy.jayan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On 10/17/2016 09:30 AM, Binoy Jayan wrote:
> These are a set of patches which removes semaphores from infiniband.
> These are part of a bigger effort to eliminate all semaphores from the
> linux kernel.

Hello Binoy,

Why do you think it would be a good idea to eliminate all semaphores 
from the Linux kernel? I don't know anyone who doesn't consider 
semaphores a useful abstraction.

Thanks,

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 8/8] IB/mlx5: Replace counting semaphore sem with wait condition
From: Christoph Hellwig @ 2016-10-17 16:40 UTC (permalink / raw)
  To: Binoy Jayan
  Cc: Doug Ledford, Sean Hefty, Hal Rosenstock, Arnd Bergmann,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476721862-7070-9-git-send-email-binoy.jayan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On Mon, Oct 17, 2016 at 10:01:02PM +0530, Binoy Jayan wrote:
> Counting semaphores are going away in the future, so replace the semaphore
> umr_common::sem with an open-coded implementation.

And NAK again..
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 7/8] IB/mthca: Replace counting semaphore event_sem with wait condition
From: Christoph Hellwig @ 2016-10-17 16:39 UTC (permalink / raw)
  To: Binoy Jayan
  Cc: Doug Ledford, Sean Hefty, Hal Rosenstock, Arnd Bergmann,
	linux-rdma, linux-kernel
In-Reply-To: <1476721862-7070-8-git-send-email-binoy.jayan@linaro.org>

Same NAK as for the last patch.

^ permalink raw reply

* Re: [PATCH 6/8] IB/hns: Replace counting semaphore event_sem with wait condition
From: Christoph Hellwig @ 2016-10-17 16:39 UTC (permalink / raw)
  To: Binoy Jayan
  Cc: Doug Ledford, Sean Hefty, Hal Rosenstock, Arnd Bergmann,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476721862-7070-7-git-send-email-binoy.jayan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On Mon, Oct 17, 2016 at 10:01:00PM +0530, Binoy Jayan wrote:
> Counting semaphores are going away in the future, so replace the semaphore
> hns_roce_cmdq::event_sem with an open-coded implementation.

Sorry, but no.  Using a proper semaphore abstraction is always
better than open coding it.  While most semaphore users should move
to better primitives, those that actually are counting semaphore
should stick to the existing primitive.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: iscsi_trx going into D state
From: Robert LeBlanc @ 2016-10-17 16:32 UTC (permalink / raw)
  To: Zhu Lingshan; +Cc: linux-rdma, linux-scsi
In-Reply-To: <880e374c-cf8b-f276-e930-57b09fe1a686@suse.com>

Some more info as we hit this this morning. We have volumes mirrored
between two targets and we had one target on the kernel with the three
patches mentioned in this thread [0][1][2] and the other was on a
kernel without the patches. We decided that after a week and a half we
wanted to get both targets on the same kernel so we rebooted the
non-patched target. Within an hour we saw iSCSI in D state with the
same stack trace so it seems that we are not hitting any of the
WARN_ON lines. We are getting both iscsi_trx and iscsi_np both in D
state, this time we have two iscsi_trx processes in D state. I don't
know if stale sessions on the clients could be contributing to this
issue (the target trying to close non-existent sessions??). This is on
4.4.23. Any more debug info we can throw at this problem to help?

Thank you,
Robert LeBlanc

# ps aux | grep D | grep iscsi
root     16525  0.0  0.0      0     0 ?        D    08:50   0:00 [iscsi_np]
root     16614  0.0  0.0      0     0 ?        D    08:50   0:00 [iscsi_trx]
root     16674  0.0  0.0      0     0 ?        D    08:50   0:00 [iscsi_trx]

# for i in 16525 16614 16674; do echo $i; cat /proc/$i/stack; done
16525
[<ffffffff814f0d5f>] iscsit_stop_session+0x19f/0x1d0
[<ffffffff814e2516>] iscsi_check_for_session_reinstatement+0x1e6/0x270
[<ffffffff814e4ed0>] iscsi_target_check_for_existing_instances+0x30/0x40
[<ffffffff814e5020>] iscsi_target_do_login+0x140/0x640
[<ffffffff814e63bc>] iscsi_target_start_negotiation+0x1c/0xb0
[<ffffffff814e410b>] iscsi_target_login_thread+0xa9b/0xfc0
[<ffffffff8109c748>] kthread+0xd8/0xf0
[<ffffffff8172018f>] ret_from_fork+0x3f/0x70
[<ffffffffffffffff>] 0xffffffffffffffff
16614
[<ffffffff814cca79>] target_wait_for_sess_cmds+0x49/0x1a0
[<ffffffffa064692b>] isert_wait_conn+0x1ab/0x2f0 [ib_isert]
[<ffffffff814f0ef2>] iscsit_close_connection+0x162/0x870
[<ffffffff814df9bf>] iscsit_take_action_for_connection_exit+0x7f/0x100
[<ffffffff814f00a0>] iscsi_target_rx_thread+0x5a0/0xe80
[<ffffffff8109c748>] kthread+0xd8/0xf0
[<ffffffff8172018f>] ret_from_fork+0x3f/0x70
[<ffffffffffffffff>] 0xffffffffffffffff
16674
[<ffffffff814cca79>] target_wait_for_sess_cmds+0x49/0x1a0
[<ffffffffa064692b>] isert_wait_conn+0x1ab/0x2f0 [ib_isert]
[<ffffffff814f0ef2>] iscsit_close_connection+0x162/0x870
[<ffffffff814df9bf>] iscsit_take_action_for_connection_exit+0x7f/0x100
[<ffffffff814f00a0>] iscsi_target_rx_thread+0x5a0/0xe80
[<ffffffff8109c748>] kthread+0xd8/0xf0
[<ffffffff8172018f>] ret_from_fork+0x3f/0x70
[<ffffffffffffffff>] 0xffffffffffffffff


[0] https://www.spinics.net/lists/target-devel/msg13463.html
[1] http://marc.info/?l=linux-scsi&m=147282568910535&w=2
[2] http://www.spinics.net/lists/linux-scsi/msg100221.html
----------------
Robert LeBlanc
PGP Fingerprint 79A2 9CA4 6CC4 45DD A904  C70E E654 3BB2 FA62 B9F1


On Fri, Oct 7, 2016 at 8:59 PM, Zhu Lingshan <lszhu@suse.com> wrote:
> Hi Robert,
>
> I also see this issue, but this is not the only code path can trigger this
> problem, I think you may also see iscsi_np in D status. I fixed one code
> path whitch still not merged to mainline. I will forward you my patch later.
> Note: my patch only fixed one code path, you may see other call statck with
> D status.
>
> Thanks,
> BR
> Zhu Lingshan
>
>
> 在 2016/10/1 1:14, Robert LeBlanc 写道:
>>
>> We are having a reoccurring problem where iscsi_trx is going into D
>> state. It seems like it is waiting for a session tear down to happen
>> or something, but keeps waiting. We have to reboot these targets on
>> occasion. This is running the 4.4.12 kernel and we have seen it on
>> several previous 4.4.x and 4.2.x kernels. There is no message in dmesg
>> or /var/log/messages. This seems to happen with increased frequency
>> when we have a disruption in our Infiniband fabric, but can happen
>> without any changes to the fabric (other than hosts rebooting).
>>
>> # ps aux | grep iscsi | grep D
>> root      4185  0.0  0.0      0     0 ?        D    Sep29   0:00
>> [iscsi_trx]
>> root     18505  0.0  0.0      0     0 ?        D    Sep29   0:00
>> [iscsi_np]
>>
>> # cat /proc/4185/stack
>> [<ffffffff814cc999>] target_wait_for_sess_cmds+0x49/0x1a0
>> [<ffffffffa087292b>] isert_wait_conn+0x1ab/0x2f0 [ib_isert]
>> [<ffffffff814f0de2>] iscsit_close_connection+0x162/0x840
>> [<ffffffff814df8df>] iscsit_take_action_for_connection_exit+0x7f/0x100
>> [<ffffffff814effc0>] iscsi_target_rx_thread+0x5a0/0xe80
>> [<ffffffff8109c6f8>] kthread+0xd8/0xf0
>> [<ffffffff8172004f>] ret_from_fork+0x3f/0x70
>> [<ffffffffffffffff>] 0xffffffffffffffff
>>
>> # cat /proc/18505/stack
>> [<ffffffff814f0c71>] iscsit_stop_session+0x1b1/0x1c0
>> [<ffffffff814e2436>] iscsi_check_for_session_reinstatement+0x1e6/0x270
>> [<ffffffff814e4df0>] iscsi_target_check_for_existing_instances+0x30/0x40
>> [<ffffffff814e4f40>] iscsi_target_do_login+0x140/0x640
>> [<ffffffff814e62dc>] iscsi_target_start_negotiation+0x1c/0xb0
>> [<ffffffff814e402b>] iscsi_target_login_thread+0xa9b/0xfc0
>> [<ffffffff8109c6f8>] kthread+0xd8/0xf0
>> [<ffffffff8172004f>] ret_from_fork+0x3f/0x70
>> [<ffffffffffffffff>] 0xffffffffffffffff
>>
>> What can we do to help get this resolved?
>>
>> Thanks,
>>
>> ----------------
>> Robert LeBlanc
>> PGP Fingerprint 79A2 9CA4 6CC4 45DD A904  C70E E654 3BB2 FA62 B9F1
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>

^ permalink raw reply

* [PATCH 8/8] IB/mlx5: Replace counting semaphore sem with wait condition
From: Binoy Jayan @ 2016-10-17 16:31 UTC (permalink / raw)
  To: Doug Ledford, Sean Hefty, Hal Rosenstock
  Cc: Arnd Bergmann, linux-rdma, linux-kernel, Binoy Jayan
In-Reply-To: <1476721862-7070-1-git-send-email-binoy.jayan@linaro.org>

Counting semaphores are going away in the future, so replace the semaphore
umr_common::sem with an open-coded implementation.

Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
 drivers/infiniband/hw/mlx5/main.c    |  3 ++-
 drivers/infiniband/hw/mlx5/mlx5_ib.h |  3 ++-
 drivers/infiniband/hw/mlx5/mr.c      | 28 +++++++++++++++++++---------
 3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 2217477..5667ea8 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -2520,7 +2520,8 @@ static int create_umr_res(struct mlx5_ib_dev *dev)
 	dev->umrc.cq = cq;
 	dev->umrc.pd = pd;
 
-	sema_init(&dev->umrc.sem, MAX_UMR_WR);
+	init_waitqueue_head(&dev->umrc.sem.wq);
+	atomic_set(&dev->umrc.sem.count, MAX_UMR_WR);
 	ret = mlx5_mr_cache_init(dev);
 	if (ret) {
 		mlx5_ib_warn(dev, "mr cache init failed %d\n", ret);
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index dcdcd19..60e2d29 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -45,6 +45,7 @@
 #include <linux/mlx5/transobj.h>
 #include <rdma/ib_user_verbs.h>
 #include <rdma/mlx5-abi.h>
+#include <rdma/ib_sa.h>
 
 #define mlx5_ib_dbg(dev, format, arg...)				\
 pr_debug("%s:%s:%d:(pid %d): " format, (dev)->ib_dev.name, __func__,	\
@@ -533,7 +534,7 @@ struct umr_common {
 	struct ib_qp	*qp;
 	/* control access to UMR QP
 	 */
-	struct semaphore	sem;
+	struct ib_semaphore	sem;
 };
 
 enum {
diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
index d4ad672..7c2af26 100644
--- a/drivers/infiniband/hw/mlx5/mr.c
+++ b/drivers/infiniband/hw/mlx5/mr.c
@@ -900,7 +900,9 @@ static struct mlx5_ib_mr *reg_umr(struct ib_pd *pd, struct ib_umem *umem,
 	prep_umr_reg_wqe(pd, &umrwr.wr, &sg, dma, npages, mr->mmkey.key,
 			 page_shift, virt_addr, len, access_flags);
 
-	down(&umrc->sem);
+	wait_event(umrc->sem.wq,
+		   atomic_add_unless(&umrc->sem.count, -1, 0));
+
 	err = ib_post_send(umrc->qp, &umrwr.wr, &bad);
 	if (err) {
 		mlx5_ib_warn(dev, "post send failed, err %d\n", err);
@@ -920,7 +922,8 @@ static struct mlx5_ib_mr *reg_umr(struct ib_pd *pd, struct ib_umem *umem,
 	mr->live = 1;
 
 unmap_dma:
-	up(&umrc->sem);
+	if (atomic_inc_return(&umrc->sem.count) == 1)
+		wake_up(&umrc->sem.wq);
 	dma_unmap_single(ddev, dma, size, DMA_TO_DEVICE);
 
 	kfree(mr_pas);
@@ -1031,7 +1034,8 @@ int mlx5_ib_update_mtt(struct mlx5_ib_mr *mr, u64 start_page_index, int npages,
 		wr.mkey = mr->mmkey.key;
 		wr.target.offset = start_page_index;
 
-		down(&umrc->sem);
+		wait_event(umrc->sem.wq,
+			   atomic_add_unless(&umrc->sem.count, -1, 0));
 		err = ib_post_send(umrc->qp, &wr.wr, &bad);
 		if (err) {
 			mlx5_ib_err(dev, "UMR post send failed, err %d\n", err);
@@ -1043,7 +1047,8 @@ int mlx5_ib_update_mtt(struct mlx5_ib_mr *mr, u64 start_page_index, int npages,
 				err = -EFAULT;
 			}
 		}
-		up(&umrc->sem);
+		if (atomic_inc_return(&umrc->sem.count) == 1)
+			wake_up(&umrc->sem.wq);
 	}
 	dma_unmap_single(ddev, dma, size, DMA_TO_DEVICE);
 
@@ -1224,15 +1229,18 @@ static int unreg_umr(struct mlx5_ib_dev *dev, struct mlx5_ib_mr *mr)
 	umrwr.wr.wr_cqe = &umr_context.cqe;
 	prep_umr_unreg_wqe(dev, &umrwr.wr, mr->mmkey.key);
 
-	down(&umrc->sem);
+	wait_event(umrc->sem.wq,
+		   atomic_add_unless(&umrc->sem.count, -1, 0));
 	err = ib_post_send(umrc->qp, &umrwr.wr, &bad);
 	if (err) {
-		up(&umrc->sem);
+		if (atomic_inc_return(&umrc->sem.count) == 1)
+			wake_up(&umrc->sem.wq);
 		mlx5_ib_dbg(dev, "err %d\n", err);
 		goto error;
 	} else {
 		wait_for_completion(&umr_context.done);
-		up(&umrc->sem);
+		if (atomic_inc_return(&umrc->sem.count) == 1)
+			wake_up(&umrc->sem.wq);
 	}
 	if (umr_context.status != IB_WC_SUCCESS) {
 		mlx5_ib_warn(dev, "unreg umr failed\n");
@@ -1291,7 +1299,8 @@ static int rereg_umr(struct ib_pd *pd, struct mlx5_ib_mr *mr, u64 virt_addr,
 	}
 
 	/* post send request to UMR QP */
-	down(&umrc->sem);
+	wait_event(umrc->sem.wq,
+		   atomic_add_unless(&umrc->sem.count, -1, 0));
 	err = ib_post_send(umrc->qp, &umrwr.wr, &bad);
 
 	if (err) {
@@ -1305,7 +1314,8 @@ static int rereg_umr(struct ib_pd *pd, struct mlx5_ib_mr *mr, u64 virt_addr,
 		}
 	}
 
-	up(&umrc->sem);
+	if (atomic_inc_return(&umrc->sem.count) == 1)
+		wake_up(&umrc->sem.wq);
 	if (flags & IB_MR_REREG_TRANS) {
 		dma_unmap_single(ddev, dma, size, DMA_TO_DEVICE);
 		kfree(mr_pas);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related

* [PATCH 7/8] IB/mthca: Replace counting semaphore event_sem with wait condition
From: Binoy Jayan @ 2016-10-17 16:31 UTC (permalink / raw)
  To: Doug Ledford, Sean Hefty, Hal Rosenstock
  Cc: Arnd Bergmann, linux-rdma, linux-kernel, Binoy Jayan
In-Reply-To: <1476721862-7070-1-git-send-email-binoy.jayan@linaro.org>

Counting semaphores are going away in the future, so replace the semaphore
mthca_cmd::event_sem with an open-coded implementation.

Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
 drivers/infiniband/hw/mthca/mthca_cmd.c | 12 ++++++++----
 drivers/infiniband/hw/mthca/mthca_dev.h |  3 ++-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/hw/mthca/mthca_cmd.c b/drivers/infiniband/hw/mthca/mthca_cmd.c
index 0cb58ea..5b8d5ea 100644
--- a/drivers/infiniband/hw/mthca/mthca_cmd.c
+++ b/drivers/infiniband/hw/mthca/mthca_cmd.c
@@ -417,7 +417,8 @@ static int mthca_cmd_wait(struct mthca_dev *dev,
 	int err = 0;
 	struct mthca_cmd_context *context;
 
-	down(&dev->cmd.event_sem);
+	wait_event(dev->cmd.event_sem.wq,
+		   atomic_add_unless(&dev->cmd.event_sem.count, -1, 0));
 
 	spin_lock(&dev->cmd.context_lock);
 	BUG_ON(dev->cmd.free_head < 0);
@@ -459,7 +460,8 @@ static int mthca_cmd_wait(struct mthca_dev *dev,
 	dev->cmd.free_head = context - dev->cmd.context;
 	spin_unlock(&dev->cmd.context_lock);
 
-	up(&dev->cmd.event_sem);
+	if (atomic_inc_return(&dev->cmd.event_sem.count) == 1)
+		wake_up(&dev->cmd.event_sem.wq);
 	return err;
 }
 
@@ -571,7 +573,8 @@ int mthca_cmd_use_events(struct mthca_dev *dev)
 	dev->cmd.context[dev->cmd.max_cmds - 1].next = -1;
 	dev->cmd.free_head = 0;
 
-	sema_init(&dev->cmd.event_sem, dev->cmd.max_cmds);
+	init_waitqueue_head(&dev->cmd.event_sem.wq);
+	atomic_set(&dev->cmd.event_sem.count, dev->cmd.max_cmds);
 	spin_lock_init(&dev->cmd.context_lock);
 
 	for (dev->cmd.token_mask = 1;
@@ -597,7 +600,8 @@ void mthca_cmd_use_polling(struct mthca_dev *dev)
 	dev->cmd.flags &= ~MTHCA_CMD_USE_EVENTS;
 
 	for (i = 0; i < dev->cmd.max_cmds; ++i)
-		down(&dev->cmd.event_sem);
+		wait_event(dev->cmd.event_sem.wq,
+			   atomic_add_unless(&dev->cmd.event_sem.count, -1, 0));
 
 	kfree(dev->cmd.context);
 
diff --git a/drivers/infiniband/hw/mthca/mthca_dev.h b/drivers/infiniband/hw/mthca/mthca_dev.h
index 87ab964..1f88835c 100644
--- a/drivers/infiniband/hw/mthca/mthca_dev.h
+++ b/drivers/infiniband/hw/mthca/mthca_dev.h
@@ -46,6 +46,7 @@
 #include <linux/list.h>
 #include <linux/semaphore.h>
 
+#include <rdma/ib_sa.h>
 #include "mthca_provider.h"
 #include "mthca_doorbell.h"
 
@@ -121,7 +122,7 @@ struct mthca_cmd {
 	struct pci_pool          *pool;
 	struct mutex              hcr_mutex;
 	struct mutex		  poll_mutex;
-	struct semaphore 	  event_sem;
+	struct ib_semaphore 	  event_sem;
 	int              	  max_cmds;
 	spinlock_t                context_lock;
 	int                       free_head;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related

* [PATCH 6/8] IB/hns: Replace counting semaphore event_sem with wait condition
From: Binoy Jayan @ 2016-10-17 16:31 UTC (permalink / raw)
  To: Doug Ledford, Sean Hefty, Hal Rosenstock
  Cc: Arnd Bergmann, linux-rdma, linux-kernel, Binoy Jayan
In-Reply-To: <1476721862-7070-1-git-send-email-binoy.jayan@linaro.org>

Counting semaphores are going away in the future, so replace the semaphore
hns_roce_cmdq::event_sem with an open-coded implementation.

Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
 drivers/infiniband/hw/hns/hns_roce_cmd.c    | 16 ++++++++++++----
 drivers/infiniband/hw/hns/hns_roce_device.h |  3 ++-
 include/rdma/ib_sa.h                        |  5 +++++
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_cmd.c b/drivers/infiniband/hw/hns/hns_roce_cmd.c
index 1421fdb..3e76717 100644
--- a/drivers/infiniband/hw/hns/hns_roce_cmd.c
+++ b/drivers/infiniband/hw/hns/hns_roce_cmd.c
@@ -248,10 +248,14 @@ static int hns_roce_cmd_mbox_wait(struct hns_roce_dev *hr_dev, u64 in_param,
 {
 	int ret = 0;
 
-	down(&hr_dev->cmd.event_sem);
+	wait_event(hr_dev->cmd.event_sem.wq,
+		   atomic_add_unless(&hr_dev->cmd.event_sem.count, -1, 0));
+
 	ret = __hns_roce_cmd_mbox_wait(hr_dev, in_param, out_param,
 				       in_modifier, op_modifier, op, timeout);
-	up(&hr_dev->cmd.event_sem);
+
+	if (atomic_inc_return(&hr_dev->cmd.event_sem.count) == 1)
+		wake_up(&hr_dev->cmd.event_sem.wq);
 
 	return ret;
 }
@@ -313,7 +317,9 @@ int hns_roce_cmd_use_events(struct hns_roce_dev *hr_dev)
 	hr_cmd->context[hr_cmd->max_cmds - 1].next = -1;
 	hr_cmd->free_head = 0;
 
-	sema_init(&hr_cmd->event_sem, hr_cmd->max_cmds);
+	init_waitqueue_head(&hr_cmd->event_sem.wq);
+	atomic_set(&hr_cmd->event_sem.count, hr_cmd->max_cmds);
+
 	spin_lock_init(&hr_cmd->context_lock);
 
 	hr_cmd->token_mask = CMD_TOKEN_MASK;
@@ -332,7 +338,9 @@ void hns_roce_cmd_use_polling(struct hns_roce_dev *hr_dev)
 	hr_cmd->use_events = 0;
 
 	for (i = 0; i < hr_cmd->max_cmds; ++i)
-		down(&hr_cmd->event_sem);
+		wait_event(hr_cmd->event_sem.wq,
+			   atomic_add_unless(
+			   &hr_dev->cmd.event_sem.count, -1, 0));
 
 	kfree(hr_cmd->context);
 	mutex_unlock(&hr_cmd->poll_mutex);
diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index 2afe075..6aed04a 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -34,6 +34,7 @@
 #define _HNS_ROCE_DEVICE_H
 
 #include <rdma/ib_verbs.h>
+#include <rdma/ib_sa.h>
 #include <linux/mutex.h>
 
 #define DRV_NAME "hns_roce"
@@ -364,7 +365,7 @@ struct hns_roce_cmdq {
 	* Event mode: cmd register mutex protection,
 	* ensure to not exceed max_cmds and user use limit region
 	*/
-	struct semaphore	event_sem;
+	struct ib_semaphore	event_sem;
 	int			max_cmds;
 	spinlock_t		context_lock;
 	int			free_head;
diff --git a/include/rdma/ib_sa.h b/include/rdma/ib_sa.h
index 5ee7aab..1901042 100644
--- a/include/rdma/ib_sa.h
+++ b/include/rdma/ib_sa.h
@@ -291,6 +291,11 @@ struct ib_sa_service_rec {
 #define IB_SA_GUIDINFO_REC_GID6		IB_SA_COMP_MASK(10)
 #define IB_SA_GUIDINFO_REC_GID7		IB_SA_COMP_MASK(11)
 
+struct ib_semaphore {
+	wait_queue_head_t wq;
+	atomic_t count;
+};
+
 struct ib_sa_guidinfo_rec {
 	__be16	lid;
 	u8	block_num;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related

* [PATCH 5/8] IB/isert: Replace semaphore sem with completion
From: Binoy Jayan @ 2016-10-17 16:30 UTC (permalink / raw)
  To: Doug Ledford, Sean Hefty, Hal Rosenstock
  Cc: Arnd Bergmann, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Binoy Jayan
In-Reply-To: <1476721862-7070-1-git-send-email-binoy.jayan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

The semaphore 'sem' in isert_device is used as completion, so convert
it to struct completion. Semaphores are going away in the future.

Signed-off-by: Binoy Jayan <binoy.jayan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/ulp/isert/ib_isert.c | 6 +++---
 drivers/infiniband/ulp/isert/ib_isert.h | 3 ++-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index 6dd43f6..de80f56 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -619,7 +619,7 @@
 	mutex_unlock(&isert_np->mutex);
 
 	isert_info("np %p: Allow accept_np to continue\n", isert_np);
-	up(&isert_np->sem);
+	complete(&isert_np->comp);
 }
 
 static void
@@ -2311,7 +2311,7 @@ struct rdma_cm_id *
 		isert_err("Unable to allocate struct isert_np\n");
 		return -ENOMEM;
 	}
-	sema_init(&isert_np->sem, 0);
+	init_completion(&isert_np->comp);
 	mutex_init(&isert_np->mutex);
 	INIT_LIST_HEAD(&isert_np->accepted);
 	INIT_LIST_HEAD(&isert_np->pending);
@@ -2427,7 +2427,7 @@ struct rdma_cm_id *
 	int ret;
 
 accept_wait:
-	ret = down_interruptible(&isert_np->sem);
+	ret = wait_for_completion_interruptible(&isert_np->comp);
 	if (ret)
 		return -ENODEV;
 
diff --git a/drivers/infiniband/ulp/isert/ib_isert.h b/drivers/infiniband/ulp/isert/ib_isert.h
index c02ada5..a1277c0 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.h
+++ b/drivers/infiniband/ulp/isert/ib_isert.h
@@ -3,6 +3,7 @@
 #include <linux/in6.h>
 #include <rdma/ib_verbs.h>
 #include <rdma/rdma_cm.h>
+#include <linux/completion.h>
 #include <rdma/rw.h>
 #include <scsi/iser.h>
 
@@ -190,7 +191,7 @@ struct isert_device {
 
 struct isert_np {
 	struct iscsi_np         *np;
-	struct semaphore	sem;
+	struct completion	comp;
 	struct rdma_cm_id	*cm_id;
 	struct mutex		mutex;
 	struct list_head	accepted;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox