[PATCH rdma-core 0/6] Sparse code changes

[PATCH rdma-core 0/6] Sparse code changes

[Jason Gunthorpe]

This is the rest of the code change stuff from the core libraries that sparse
did not like.

There is one bug fix in here, the missing swap on INADDR_LOOPBACK.

Otherwise it is just small changes to make sparse happier.

The full series is here:

https://github.com/linux-rdma/rdma-core/pull/100

Which is enough hackery to make sparse run automatically from travis without
triggering any sparse warnings, at least over the core libraries.

I think interested parties should look at the entire series on github. It is
so large it probably won't fit on the list.

Bart and I were having an interesting discussion on github on this
approach. It isn't clear to me if a different version of the 'sparse-include'
setup could be designed.. But if someone else can figure out how to get the
same result more robustly that would be great.

For now this actually works, and can run inside travis, which is much better
than where we were..

Someone else(s) will have to sparse cleanup the providers and ibacm, they
scare me.

Jason Gunthorpe (6):
  Use NULL instead of 0 to silence sparse
  INADDR_* needs to be byteswapped before being used
  rstream: Use waitpid instead of wait
  rdmacm: Use C11 stdatomic for all atomics
  rsocket: Fix byte swapping when constructing path records
  verbs: Consistently apply __attribute_const

 iwpmd/iwarp_pm_common.c             |  2 +-
 iwpmd/iwarp_pm_helper.c             |  2 +-
 libibverbs/examples/rc_pingpong.c   |  2 +-
 libibverbs/examples/srq_pingpong.c  |  2 +-
 libibverbs/examples/uc_pingpong.c   |  2 +-
 libibverbs/examples/ud_pingpong.c   |  2 +-
 libibverbs/examples/xsrq_pingpong.c |  2 +-
 libibverbs/verbs.c                  |  8 +++---
 libibverbs/verbs.h                  |  8 +++---
 librdmacm/cma.h                     | 49 ++++---------------------------------
 librdmacm/examples/cmatose.c        |  4 +--
 librdmacm/examples/mckey.c          |  2 +-
 librdmacm/examples/rcopy.c          |  2 +-
 librdmacm/examples/riostream.c      |  2 +-
 librdmacm/examples/rstream.c        |  6 ++---
 librdmacm/examples/udaddy.c         |  2 +-
 librdmacm/preload.c                 | 14 +++++------
 librdmacm/rsocket.c                 | 17 ++++++-------
 rdma-ndd/rdma-ndd.c                 |  6 ++---
 19 files changed, 46 insertions(+), 88 deletions(-)

-- 
2.7.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

[PATCH rdma-core 1/6] Use NULL instead of 0 to silence sparse

[Jason Gunthorpe]

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libibverbs/examples/rc_pingpong.c   | 2 +-
 libibverbs/examples/srq_pingpong.c  | 2 +-
 libibverbs/examples/uc_pingpong.c   | 2 +-
 libibverbs/examples/ud_pingpong.c   | 2 +-
 libibverbs/examples/xsrq_pingpong.c | 2 +-
 librdmacm/examples/cmatose.c        | 4 ++--
 librdmacm/examples/mckey.c          | 2 +-
 librdmacm/examples/rcopy.c          | 2 +-
 librdmacm/examples/riostream.c      | 2 +-
 librdmacm/examples/rstream.c        | 2 +-
 librdmacm/examples/udaddy.c         | 2 +-
 librdmacm/rsocket.c                 | 2 +-
 rdma-ndd/rdma-ndd.c                 | 6 +++---
 13 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/libibverbs/examples/rc_pingpong.c b/libibverbs/examples/rc_pingpong.c
index 7c70457183d986..7400ec8c80f4b7 100644
--- a/libibverbs/examples/rc_pingpong.c
+++ b/libibverbs/examples/rc_pingpong.c
@@ -274,7 +274,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
 	}
 
 	listen(sockfd, 1);
-	connfd = accept(sockfd, NULL, 0);
+	connfd = accept(sockfd, NULL, NULL);
 	close(sockfd);
 	if (connfd < 0) {
 		fprintf(stderr, "accept() failed\n");
diff --git a/libibverbs/examples/srq_pingpong.c b/libibverbs/examples/srq_pingpong.c
index 30d81f0e4f316c..59ce0b2e729b8e 100644
--- a/libibverbs/examples/srq_pingpong.c
+++ b/libibverbs/examples/srq_pingpong.c
@@ -284,7 +284,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
 	}
 
 	listen(sockfd, 1);
-	connfd = accept(sockfd, NULL, 0);
+	connfd = accept(sockfd, NULL, NULL);
 	close(sockfd);
 	if (connfd < 0) {
 		fprintf(stderr, "accept() failed\n");
diff --git a/libibverbs/examples/uc_pingpong.c b/libibverbs/examples/uc_pingpong.c
index 3220cccd633b66..2720ab337de2c4 100644
--- a/libibverbs/examples/uc_pingpong.c
+++ b/libibverbs/examples/uc_pingpong.c
@@ -248,7 +248,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
 	}
 
 	listen(sockfd, 1);
-	connfd = accept(sockfd, NULL, 0);
+	connfd = accept(sockfd, NULL, NULL);
 	close(sockfd);
 	if (connfd < 0) {
 		fprintf(stderr, "accept() failed\n");
diff --git a/libibverbs/examples/ud_pingpong.c b/libibverbs/examples/ud_pingpong.c
index db783c22375e3a..d6c46f7eda4d53 100644
--- a/libibverbs/examples/ud_pingpong.c
+++ b/libibverbs/examples/ud_pingpong.c
@@ -246,7 +246,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
 	}
 
 	listen(sockfd, 1);
-	connfd = accept(sockfd, NULL, 0);
+	connfd = accept(sockfd, NULL, NULL);
 	close(sockfd);
 	if (connfd < 0) {
 		fprintf(stderr, "accept() failed\n");
diff --git a/libibverbs/examples/xsrq_pingpong.c b/libibverbs/examples/xsrq_pingpong.c
index 31a86f7c8360e2..46f59a6856a4d5 100644
--- a/libibverbs/examples/xsrq_pingpong.c
+++ b/libibverbs/examples/xsrq_pingpong.c
@@ -633,7 +633,7 @@ static int pp_server_connect(int port)
 	listen(sockfd, ctx.num_clients);
 
 	for (i = 0; i < ctx.num_clients; i++) {
-		connfd = accept(sockfd, NULL, 0);
+		connfd = accept(sockfd, NULL, NULL);
 		if (connfd < 0) {
 			fprintf(stderr, "accept() failed for client %d\n", i);
 			return 1;
diff --git a/librdmacm/examples/cmatose.c b/librdmacm/examples/cmatose.c
index c913c48adbbe05..b1c9dd13e4b15e 100644
--- a/librdmacm/examples/cmatose.c
+++ b/librdmacm/examples/cmatose.c
@@ -119,8 +119,8 @@ static int init_node(struct cmatest_node *node)
 	}
 
 	cqe = message_count ? message_count : 1;
-	node->cq[SEND_CQ_INDEX] = ibv_create_cq(node->cma_id->verbs, cqe, node, 0, 0);
-	node->cq[RECV_CQ_INDEX] = ibv_create_cq(node->cma_id->verbs, cqe, node, 0, 0);
+	node->cq[SEND_CQ_INDEX] = ibv_create_cq(node->cma_id->verbs, cqe, node, NULL, 0);
+	node->cq[RECV_CQ_INDEX] = ibv_create_cq(node->cma_id->verbs, cqe, node, NULL, 0);
 	if (!node->cq[SEND_CQ_INDEX] || !node->cq[RECV_CQ_INDEX]) {
 		ret = -ENOMEM;
 		printf("cmatose: unable to create CQ\n");
diff --git a/librdmacm/examples/mckey.c b/librdmacm/examples/mckey.c
index 7d1b53b025e6e0..60cf8a2408a447 100644
--- a/librdmacm/examples/mckey.c
+++ b/librdmacm/examples/mckey.c
@@ -140,7 +140,7 @@ static int init_node(struct cmatest_node *node)
 	}
 
 	cqe = message_count ? message_count * 2 : 2;
-	node->cq = ibv_create_cq(node->cma_id->verbs, cqe, node, 0, 0);
+	node->cq = ibv_create_cq(node->cma_id->verbs, cqe, node, NULL, 0);
 	if (!node->cq) {
 		ret = -ENOMEM;
 		printf("mckey: unable to create CQ\n");
diff --git a/librdmacm/examples/rcopy.c b/librdmacm/examples/rcopy.c
index 9ff7ade29261a3..c85ce8ee9348b4 100644
--- a/librdmacm/examples/rcopy.c
+++ b/librdmacm/examples/rcopy.c
@@ -278,7 +278,7 @@ static void server_close(int rs, struct msg_hdr *msg)
 
 	if (file_addr) {
 		munmap(file_addr, bytes);
-		file_addr = 0;
+		file_addr = NULL;
 	}
 
 	if (fd > 0) {
diff --git a/librdmacm/examples/riostream.c b/librdmacm/examples/riostream.c
index ac625702ae3342..bdf4fd34edfed6 100644
--- a/librdmacm/examples/riostream.c
+++ b/librdmacm/examples/riostream.c
@@ -422,7 +422,7 @@ static int server_connect(void)
 			}
 		}
 
-		rs = raccept(lrs, NULL, 0);
+		rs = raccept(lrs, NULL, NULL);
 	} while (rs < 0 && (errno == EAGAIN || errno == EWOULDBLOCK));
 	if (rs < 0) {
 		perror("raccept");
diff --git a/librdmacm/examples/rstream.c b/librdmacm/examples/rstream.c
index 4f76005a2651aa..70b714613e91af 100644
--- a/librdmacm/examples/rstream.c
+++ b/librdmacm/examples/rstream.c
@@ -386,7 +386,7 @@ static int server_connect(void)
 			}
 		}
 
-		rs = rs_accept(lrs, NULL, 0);
+		rs = rs_accept(lrs, NULL, NULL);
 	} while (rs < 0 && (errno == EAGAIN || errno == EWOULDBLOCK));
 	if (rs < 0) {
 		perror("raccept");
diff --git a/librdmacm/examples/udaddy.c b/librdmacm/examples/udaddy.c
index 5c984ff615336e..b1ac90c38f3e27 100644
--- a/librdmacm/examples/udaddy.c
+++ b/librdmacm/examples/udaddy.c
@@ -135,7 +135,7 @@ static int init_node(struct cmatest_node *node)
 	}
 
 	cqe = message_count ? message_count * 2 : 2;
-	node->cq = ibv_create_cq(node->cma_id->verbs, cqe, node, 0, 0);
+	node->cq = ibv_create_cq(node->cma_id->verbs, cqe, node, NULL, 0);
 	if (!node->cq) {
 		ret = -ENOMEM;
 		printf("udaddy: unable to create CQ\n");
diff --git a/librdmacm/rsocket.c b/librdmacm/rsocket.c
index db06a9c3b1c5ee..04b91a57b9a42e 100644
--- a/librdmacm/rsocket.c
+++ b/librdmacm/rsocket.c
@@ -2420,7 +2420,7 @@ ssize_t rrecv(int socket, void *buf, size_t len, int flags)
 	rs = idm_at(&idm, socket);
 	if (rs->type == SOCK_DGRAM) {
 		fastlock_acquire(&rs->rlock);
-		ret = ds_recvfrom(rs, buf, len, flags, NULL, 0);
+		ret = ds_recvfrom(rs, buf, len, flags, NULL, NULL);
 		fastlock_release(&rs->rlock);
 		return ret;
 	}
diff --git a/rdma-ndd/rdma-ndd.c b/rdma-ndd/rdma-ndd.c
index 9ac4b8ddf588e9..c598525535b7dc 100644
--- a/rdma-ndd/rdma-ndd.c
+++ b/rdma-ndd/rdma-ndd.c
@@ -291,9 +291,9 @@ int main(int argc, char *argv[])
 	while (1) {
 		int opt_idx = 0;
 		static struct option long_opts[] = {
-			{"foreground",   0, 0, 'f' },
-			{"help",         0, 0, 'h' },
-			{"debug",         0, 0, 'd' },
+			{"foreground",   0, NULL, 'f' },
+			{"help",         0, NULL, 'h' },
+			{"debug",         0, NULL, 'd' },
 			{ }
 		};
 
-- 
2.7.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

[PATCH rdma-core 2/6] INADDR_* needs to be byteswapped before being used

[Jason Gunthorpe]

The constants are in host order. INADDR_ANY is 0 so the swap has no change,
but the INADDR_LOOPBACK test in rsocket never worked.

Found by sparse

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 iwpmd/iwarp_pm_common.c | 2 +-
 iwpmd/iwarp_pm_helper.c | 2 +-
 librdmacm/rsocket.c     | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/iwpmd/iwarp_pm_common.c b/iwpmd/iwarp_pm_common.c
index e73f1714d1312d..8b4432042329c1 100644
--- a/iwpmd/iwarp_pm_common.c
+++ b/iwpmd/iwarp_pm_common.c
@@ -110,7 +110,7 @@ int create_iwpm_socket_v4(__u16 bind_port)
 	memset(&bind_addr, 0, sizeof(bind_addr));
 	bind_in4 = &bind_addr.v4_sockaddr;
 	bind_in4->sin_family = AF_INET;
-	bind_in4->sin_addr.s_addr = INADDR_ANY;
+	bind_in4->sin_addr.s_addr = htobe32(INADDR_ANY);
 	bind_in4->sin_port = htobe16(bind_port);
 
 	if (bind(pm_sock, &bind_addr.sock_addr, sizeof(struct sockaddr_in))) {
diff --git a/iwpmd/iwarp_pm_helper.c b/iwpmd/iwarp_pm_helper.c
index ddc1deb48215a6..fe69f5f166904a 100644
--- a/iwpmd/iwarp_pm_helper.c
+++ b/iwpmd/iwarp_pm_helper.c
@@ -279,7 +279,7 @@ static int get_iwpm_tcp_port(__u16 addr_family, __be16 requested_port,
 		mapped_port = &((struct sockaddr_in *)mapped_addr)->sin_port;
 		bind_in4 = &bind_addr.v4_sockaddr;
 		bind_in4->sin_family = addr_family;
-		bind_in4->sin_addr.s_addr = INADDR_ANY;
+		bind_in4->sin_addr.s_addr = htobe32(INADDR_ANY);
 		if (requested_port)
 			requested_port = *mapped_port;
 		bind_in4->sin_port = requested_port;
diff --git a/librdmacm/rsocket.c b/librdmacm/rsocket.c
index 04b91a57b9a42e..69ceab6de745d0 100644
--- a/librdmacm/rsocket.c
+++ b/librdmacm/rsocket.c
@@ -1402,8 +1402,8 @@ connected:
 static int rs_any_addr(const union socket_addr *addr)
 {
 	if (addr->sa.sa_family == AF_INET) {
-		return (addr->sin.sin_addr.s_addr == INADDR_ANY ||
-			addr->sin.sin_addr.s_addr == INADDR_LOOPBACK);
+		return (addr->sin.sin_addr.s_addr == htobe32(INADDR_ANY) ||
+			addr->sin.sin_addr.s_addr == htobe32(INADDR_LOOPBACK));
 	} else {
 		return (!memcmp(&addr->sin6.sin6_addr, &in6addr_any, 16) ||
 			!memcmp(&addr->sin6.sin6_addr, &in6addr_loopback, 16));
-- 
2.7.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

[PATCH rdma-core 3/6] rstream: Use waitpid instead of wait

[Jason Gunthorpe]

This is motiviated by sparse not handling the transparent union in wait()
properly, but using waitpid is generally a better idea since we know what is
supposed to be waited for here.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 librdmacm/examples/rstream.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/librdmacm/examples/rstream.c b/librdmacm/examples/rstream.c
index 70b714613e91af..7523269cbd5cda 100644
--- a/librdmacm/examples/rstream.c
+++ b/librdmacm/examples/rstream.c
@@ -527,7 +527,7 @@ static int run(void)
 			run_test();
 		}
 		if (fork_pid)
-			wait(NULL);
+			waitpid(fork_pid, NULL, 0);
 		else
 			rs_shutdown(rs, SHUT_RDWR);
 		rs_close(rs);
@@ -555,7 +555,7 @@ static int run(void)
 	}
 
 	if (fork_pid)
-		wait(NULL);
+		waitpid(fork_pid, NULL, 0);
 	else
 		rs_shutdown(rs, SHUT_RDWR);
 	rs_close(rs);
-- 
2.7.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

[PATCH rdma-core 4/6] rdmacm: Use C11 stdatomic for all atomics

[Jason Gunthorpe]

No sense in having a private API here. stdatomic will be equivalent
to the gcc builtin.

This is motivated by sparse which does not handle the gcc atomic builtins.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 librdmacm/cma.h     | 49 +++++--------------------------------------------
 librdmacm/preload.c | 14 ++++++--------
 librdmacm/rsocket.c |  7 +++----
 3 files changed, 14 insertions(+), 56 deletions(-)

diff --git a/librdmacm/cma.h b/librdmacm/cma.h
index 8795d2894d262f..645d1e43576279 100644
--- a/librdmacm/cma.h
+++ b/librdmacm/cma.h
@@ -40,6 +40,7 @@
 #include <errno.h>
 #include <endian.h>
 #include <semaphore.h>
+#include <stdatomic.h>
 
 #include <rdma/rdma_cma.h>
 #include <infiniband/ib.h>
@@ -53,46 +54,14 @@
 /*
  * Fast synchronization for low contention locking.
  */
-#if DEFINE_ATOMICS
-#define fastlock_t pthread_mutex_t
-#define fastlock_init(lock) pthread_mutex_init(lock, NULL)
-#define fastlock_destroy(lock) pthread_mutex_destroy(lock)
-#define fastlock_acquire(lock) pthread_mutex_lock(lock)
-#define fastlock_release(lock) pthread_mutex_unlock(lock)
-
-typedef struct { pthread_mutex_t mut; int val; } atomic_t;
-static inline int atomic_inc(atomic_t *atomic)
-{
-	int v;
-
-	pthread_mutex_lock(&atomic->mut);
-	v = ++(atomic->val);
-	pthread_mutex_unlock(&atomic->mut);
-	return v;
-}
-static inline int atomic_dec(atomic_t *atomic)
-{
-	int v;
-
-	pthread_mutex_lock(&atomic->mut);
-	v = --(atomic->val);
-	pthread_mutex_unlock(&atomic->mut);
-	return v;
-}
-static inline void atomic_init(atomic_t *atomic)
-{
-	pthread_mutex_init(&atomic->mut, NULL);
-	atomic->val = 0;
-}
-#else
 typedef struct {
 	sem_t sem;
-	volatile int cnt;
+	_Atomic(int) cnt;
 } fastlock_t;
 static inline void fastlock_init(fastlock_t *lock)
 {
 	sem_init(&lock->sem, 0, 0);
-	lock->cnt = 0;
+	atomic_store(&lock->cnt, 0);
 }
 static inline void fastlock_destroy(fastlock_t *lock)
 {
@@ -100,23 +69,15 @@ static inline void fastlock_destroy(fastlock_t *lock)
 }
 static inline void fastlock_acquire(fastlock_t *lock)
 {
-	if (__sync_add_and_fetch(&lock->cnt, 1) > 1)
+	if (atomic_fetch_add(&lock->cnt, 1) > 1)
 		sem_wait(&lock->sem);
 }
 static inline void fastlock_release(fastlock_t *lock)
 {
-	if (__sync_sub_and_fetch(&lock->cnt, 1) > 0)
+	if (atomic_fetch_sub(&lock->cnt, 1) > 0)
 		sem_post(&lock->sem);
 }
 
-typedef struct { volatile int val; } atomic_t;
-#define atomic_inc(v) (__sync_add_and_fetch(&(v)->val, 1))
-#define atomic_dec(v) (__sync_sub_and_fetch(&(v)->val, 1))
-#define atomic_init(v) ((v)->val = 0)
-#endif /* DEFINE_ATOMICS */
-#define atomic_get(v) ((v)->val)
-#define atomic_set(v, s) ((v)->val = s)
-
 uint16_t ucma_get_port(struct sockaddr *addr);
 int ucma_addrlen(struct sockaddr *addr);
 void ucma_set_sid(enum rdma_port_space ps, struct sockaddr *addr,
diff --git a/librdmacm/preload.c b/librdmacm/preload.c
index 1aea3a7f0a247a..bd1bcb1d701015 100644
--- a/librdmacm/preload.c
+++ b/librdmacm/preload.c
@@ -119,7 +119,7 @@ struct fd_info {
 	enum fd_fork_state state;
 	int fd;
 	int dupfd;
-	atomic_t refcnt;
+	_Atomic(int) refcnt;
 };
 
 struct config_entry {
@@ -266,8 +266,7 @@ static int fd_open(void)
 	}
 
 	fdi->dupfd = -1;
-	atomic_init(&fdi->refcnt);
-	atomic_set(&fdi->refcnt, 1);
+	atomic_store(&fdi->refcnt, 1);
 	pthread_mutex_lock(&mut);
 	ret = idm_set(&idm, index, fdi);
 	pthread_mutex_unlock(&mut);
@@ -1013,7 +1012,7 @@ int close(int socket)
 			return ret;
 	}
 
-	if (atomic_dec(&fdi->refcnt))
+	if (atomic_fetch_sub(&fdi->refcnt, 1))
 		return 0;
 
 	idm_clear(&idm, socket);
@@ -1118,7 +1117,7 @@ int dup2(int oldfd, int newfd)
 	newfdi = idm_lookup(&idm, newfd);
 	if (newfdi) {
 		 /* newfd cannot have been dup'ed directly */
-		if (atomic_get(&newfdi->refcnt) > 1)
+		if (atomic_load(&newfdi->refcnt) > 1)
 			return ERR(EBUSY);
 		close(newfd);
 	}
@@ -1145,9 +1144,8 @@ int dup2(int oldfd, int newfd)
 	} else {
 		newfdi->dupfd = oldfd;
 	}
-	atomic_init(&newfdi->refcnt);
-	atomic_set(&newfdi->refcnt, 1);
-	atomic_inc(&oldfdi->refcnt);
+	atomic_store(&newfdi->refcnt, 1);
+	atomic_fetch_add(&oldfdi->refcnt, 1);
 	return newfd;
 }
 
diff --git a/librdmacm/rsocket.c b/librdmacm/rsocket.c
index 69ceab6de745d0..a0bc604845c118 100644
--- a/librdmacm/rsocket.c
+++ b/librdmacm/rsocket.c
@@ -190,7 +190,7 @@ struct rs_iomap_mr {
 	uint64_t offset;
 	struct ibv_mr *mr;
 	dlist_entry entry;
-	atomic_t refcnt;
+	_Atomic(int) refcnt;
 	int index;	/* -1 if mapping is local and not in iomap_list */
 };
 
@@ -898,7 +898,7 @@ static int rs_create_ep(struct rsocket *rs)
 
 static void rs_release_iomap_mr(struct rs_iomap_mr *iomr)
 {
-	if (atomic_dec(&iomr->refcnt))
+	if (atomic_fetch_sub(&iomr->refcnt, 1))
 		return;
 
 	dlist_remove(&iomr->entry);
@@ -3798,8 +3798,7 @@ off_t riomap(int socket, void *buf, size_t len, int prot, int flags, off_t offse
 	if (offset == -1)
 		offset = (uintptr_t) buf;
 	iomr->offset = offset;
-	atomic_init(&iomr->refcnt);
-	atomic_set(&iomr->refcnt, 1);
+	atomic_store(&iomr->refcnt, 1);
 
 	if (iomr->index >= 0) {
 		dlist_insert_tail(&iomr->entry, &rs->iomap_queue);
-- 
2.7.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

Re: [PATCH rdma-core 4/6] rdmacm: Use C11 stdatomic for all atomics

[Bart Van Assche]

On Wed, 2017-03-15 at 16:12 -0600, Jason Gunthorpe wrote:
> @@ -1013,7 +1012,7 @@ int close(int socket)
>  			return ret;
>  	}
>  
> -	if (atomic_dec(&fdi->refcnt))
> +	if (atomic_fetch_sub(&fdi->refcnt, 1))
>  		return 0;
>  
>  	idm_clear(&idm, socket);
> @@ -898,7 +898,7 @@ static int rs_create_ep(struct rsocket *rs)
>  
>  static void rs_release_iomap_mr(struct rs_iomap_mr *iomr)
>  {
> -	if (atomic_dec(&iomr->refcnt))
> +	if (atomic_fetch_sub(&iomr->refcnt, 1))
>  		return;
>  
>  	dlist_remove(&iomr->entry);

Hello Jason,

In the gcc documentation I read that __sync_sub_and_fetch() (used to
implement atomic_dec()) returns the new value. In the C11 standard I read
that atomic_fetch_sub() returns the old value. Do you agree with this?

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

Re: [PATCH rdma-core 4/6] rdmacm: Use C11 stdatomic for all atomics

[Jason Gunthorpe]

On Wed, Mar 15, 2017 at 10:55:52PM +0000, Bart Van Assche wrote:
> On Wed, 2017-03-15 at 16:12 -0600, Jason Gunthorpe wrote:
> > @@ -1013,7 +1012,7 @@ int close(int socket)
> >  			return ret;
> >  	}
> >  
> > -	if (atomic_dec(&fdi->refcnt))
> > +	if (atomic_fetch_sub(&fdi->refcnt, 1))
> >  		return 0;
> >  
> >  	idm_clear(&idm, socket);
> > @@ -898,7 +898,7 @@ static int rs_create_ep(struct rsocket *rs)
> >  
> >  static void rs_release_iomap_mr(struct rs_iomap_mr *iomr)
> >  {
> > -	if (atomic_dec(&iomr->refcnt))
> > +	if (atomic_fetch_sub(&iomr->refcnt, 1))
> >  		return;
> >  
> >  	dlist_remove(&iomr->entry);
> 
> Hello Jason,
> 
> In the gcc documentation I read that __sync_sub_and_fetch() (used to
> implement atomic_dec()) returns the new value. In the C11 standard I read
> that atomic_fetch_sub() returns the old value. Do you agree with this?

Indeed, you are right, I will sqush in this patch, thanks

diff --git a/librdmacm/cma.h b/librdmacm/cma.h
index 645d1e43576279..2f5f86cf498918 100644
--- a/librdmacm/cma.h
+++ b/librdmacm/cma.h
@@ -69,12 +69,12 @@ static inline void fastlock_destroy(fastlock_t *lock)
 }
 static inline void fastlock_acquire(fastlock_t *lock)
 {
-	if (atomic_fetch_add(&lock->cnt, 1) > 1)
+	if (atomic_fetch_add(&lock->cnt, 1) > 0)
 		sem_wait(&lock->sem);
 }
 static inline void fastlock_release(fastlock_t *lock)
 {
-	if (atomic_fetch_sub(&lock->cnt, 1) > 0)
+	if (atomic_fetch_sub(&lock->cnt, 1) > 1)
 		sem_post(&lock->sem);
 }
 
diff --git a/librdmacm/preload.c b/librdmacm/preload.c
index bd1bcb1d701015..82cb0c03850b05 100644
--- a/librdmacm/preload.c
+++ b/librdmacm/preload.c
@@ -1012,7 +1012,7 @@ int close(int socket)
 			return ret;
 	}
 
-	if (atomic_fetch_sub(&fdi->refcnt, 1))
+	if (atomic_fetch_sub(&fdi->refcnt, 1) != 1)
 		return 0;
 
 	idm_clear(&idm, socket);
diff --git a/librdmacm/rsocket.c b/librdmacm/rsocket.c
index 4fbe7aaa938610..9a71035fbeed8b 100644
--- a/librdmacm/rsocket.c
+++ b/librdmacm/rsocket.c
@@ -898,7 +898,7 @@ static int rs_create_ep(struct rsocket *rs)
 
 static void rs_release_iomap_mr(struct rs_iomap_mr *iomr)
 {
-	if (atomic_fetch_sub(&iomr->refcnt, 1))
+	if (atomic_fetch_sub(&iomr->refcnt, 1) != 1)
 		return;
 
 	dlist_remove(&iomr->entry);
--
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

Re: [PATCH rdma-core 4/6] rdmacm: Use C11 stdatomic for all atomics

[Bart Van Assche]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2098 bytes --]

On Wed, 2017-03-15 at 17:18 -0600, Jason Gunthorpe wrote:
> Indeed, you are right, I will sqush in this patch, thanks
> 
> diff --git a/librdmacm/cma.h b/librdmacm/cma.h
> index 645d1e43576279..2f5f86cf498918 100644
> --- a/librdmacm/cma.h
> +++ b/librdmacm/cma.h
> @@ -69,12 +69,12 @@ static inline void fastlock_destroy(fastlock_t *lock)
>  }
>  static inline void fastlock_acquire(fastlock_t *lock)
>  {
> -	if (atomic_fetch_add(&lock->cnt, 1) > 1)
> +	if (atomic_fetch_add(&lock->cnt, 1) > 0)
>  		sem_wait(&lock->sem);
>  }
>  static inline void fastlock_release(fastlock_t *lock)
>  {
> -	if (atomic_fetch_sub(&lock->cnt, 1) > 0)
> +	if (atomic_fetch_sub(&lock->cnt, 1) > 1)
>  		sem_post(&lock->sem);
>  }
>  
> diff --git a/librdmacm/preload.c b/librdmacm/preload.c
> index bd1bcb1d701015..82cb0c03850b05 100644
> --- a/librdmacm/preload.c
> +++ b/librdmacm/preload.c
> @@ -1012,7 +1012,7 @@ int close(int socket)
>  			return ret;
>  	}
>  
> -	if (atomic_fetch_sub(&fdi->refcnt, 1))
> +	if (atomic_fetch_sub(&fdi->refcnt, 1) != 1)
>  		return 0;
>  
>  	idm_clear(&idm, socket);
> diff --git a/librdmacm/rsocket.c b/librdmacm/rsocket.c
> index 4fbe7aaa938610..9a71035fbeed8b 100644
> --- a/librdmacm/rsocket.c
> +++ b/librdmacm/rsocket.c
> @@ -898,7 +898,7 @@ static int rs_create_ep(struct rsocket *rs)
>  
>  static void rs_release_iomap_mr(struct rs_iomap_mr *iomr)
>  {
> -	if (atomic_fetch_sub(&iomr->refcnt, 1))
> +	if (atomic_fetch_sub(&iomr->refcnt, 1) != 1)
>  		return;
>  
>  	dlist_remove(&iomr->entry);

Hello Jason,

Have you considered to introduce something like the function below? I think
that would make the changes smaller and hence easier to review.

static inline int atomic_add_fetch(_Atomic(int) *a, int arg)
{
	return atomic_fetch_add(a, arg) + arg;
}

static inline int atomic_sub_fetch(_Atomic(int) *a, int arg)
{
	return atomic_fetch_sub(a, arg) - arg;
}

Bart.
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš

Re: [PATCH rdma-core 4/6] rdmacm: Use C11 stdatomic for all atomics

[Jason Gunthorpe]

On Thu, Mar 16, 2017 at 01:04:49AM +0000, Bart Van Assche wrote:
> Have you considered to introduce something like the function below? I think
> that would make the changes smaller and hence easier to review.

I started like this and then just inlined the arithmatic in the few
call sites.

I think the algorithm reads as clearly either way and I generally
prefer to use the standard library directly..

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

[PATCH rdma-core 5/6] rsocket: Fix byte swapping when constructing path records

[Jason Gunthorpe]

These swaps were missed or in the wrong place

Found by sparse.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 librdmacm/rsocket.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/librdmacm/rsocket.c b/librdmacm/rsocket.c
index a0bc604845c118..4fbe7aaa938610 100644
--- a/librdmacm/rsocket.c
+++ b/librdmacm/rsocket.c
@@ -3551,11 +3551,11 @@ static void rs_convert_sa_path(struct ibv_sa_path_rec *sa_path,
 	path_data->path.dlid = sa_path->dlid;
 	path_data->path.slid = sa_path->slid;
 	fl_hop = be32toh(sa_path->flow_label) << 8;
-	path_data->path.flowlabel_hoplimit = htobe32(fl_hop) | sa_path->hop_limit;
+	path_data->path.flowlabel_hoplimit = htobe32(fl_hop | sa_path->hop_limit);
 	path_data->path.tclass = sa_path->traffic_class;
 	path_data->path.reversible_numpath = sa_path->reversible << 7 | 1;
 	path_data->path.pkey = sa_path->pkey;
-	path_data->path.qosclass_sl = sa_path->sl;
+	path_data->path.qosclass_sl = htobe16(sa_path->sl);
 	path_data->path.mtu = sa_path->mtu | 2 << 6;	/* exactly */
 	path_data->path.rate = sa_path->rate | 2 << 6;
 	path_data->path.packetlifetime = sa_path->packet_life_time | 2 << 6;
-- 
2.7.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

[PATCH rdma-core 6/6] verbs: Consistently apply __attribute_const

[Jason Gunthorpe]

Put it on the prototype and the declaration. Sparse complains if the
attribute is not in exactly the same spot, so always put it in the usual
place for declarations.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libibverbs/verbs.c | 8 ++++----
 libibverbs/verbs.h | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/libibverbs/verbs.c b/libibverbs/verbs.c
index 6a5b2a52da3c5b..8f7cb43a220b86 100644
--- a/libibverbs/verbs.c
+++ b/libibverbs/verbs.c
@@ -51,7 +51,7 @@
 #include "neigh.h"
 #endif
 
-int ibv_rate_to_mult(enum ibv_rate rate)
+int __attribute__((const)) ibv_rate_to_mult(enum ibv_rate rate)
 {
 	switch (rate) {
 	case IBV_RATE_2_5_GBPS: return  1;
@@ -67,7 +67,7 @@ int ibv_rate_to_mult(enum ibv_rate rate)
 	}
 }
 
-enum ibv_rate mult_to_ibv_rate(int mult)
+enum ibv_rate __attribute__((const)) mult_to_ibv_rate(int mult)
 {
 	switch (mult) {
 	case 1:  return IBV_RATE_2_5_GBPS;
@@ -83,7 +83,7 @@ enum ibv_rate mult_to_ibv_rate(int mult)
 	}
 }
 
-int ibv_rate_to_mbps(enum ibv_rate rate)
+int  __attribute__((const)) ibv_rate_to_mbps(enum ibv_rate rate)
 {
 	switch (rate) {
 	case IBV_RATE_2_5_GBPS: return 2500;
@@ -107,7 +107,7 @@ int ibv_rate_to_mbps(enum ibv_rate rate)
 	}
 }
 
-enum ibv_rate mbps_to_ibv_rate(int mbps)
+enum ibv_rate __attribute__((const)) mbps_to_ibv_rate(int mbps)
 {
 	switch (mbps) {
 	case 2500:   return IBV_RATE_2_5_GBPS;
diff --git a/libibverbs/verbs.h b/libibverbs/verbs.h
index 25f4ededdb6d63..ee0fac42e68a77 100644
--- a/libibverbs/verbs.h
+++ b/libibverbs/verbs.h
@@ -584,26 +584,26 @@ enum ibv_rate {
  * converted to 2, since 5 Gbit/sec is 2 * 2.5 Gbit/sec.
  * @rate: rate to convert.
  */
-int ibv_rate_to_mult(enum ibv_rate rate) __attribute_const;
+int  __attribute_const ibv_rate_to_mult(enum ibv_rate rate);
 
 /**
  * mult_to_ibv_rate - Convert a multiple of 2.5 Gbit/sec to an IB rate enum.
  * @mult: multiple to convert.
  */
-enum ibv_rate mult_to_ibv_rate(int mult) __attribute_const;
+enum ibv_rate __attribute_const mult_to_ibv_rate(int mult);
 
 /**
  * ibv_rate_to_mbps - Convert the IB rate enum to Mbit/sec.
  * For example, IBV_RATE_5_GBPS will return the value 5000.
  * @rate: rate to convert.
  */
-int ibv_rate_to_mbps(enum ibv_rate rate) __attribute_const;
+int __attribute_const ibv_rate_to_mbps(enum ibv_rate rate);
 
 /**
  * mbps_to_ibv_rate - Convert a Mbit/sec value to an IB rate enum.
  * @mbps: value to convert.
  */
-enum ibv_rate mbps_to_ibv_rate(int mbps) __attribute_const;
+enum ibv_rate __attribute_const mbps_to_ibv_rate(int mbps) __attribute_const;
 
 struct ibv_ah_attr {
 	struct ibv_global_route	grh;
-- 
2.7.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

Re: [PATCH rdma-core 0/6] Sparse code changes

[Doug Ledford]

On Wed, 2017-03-15 at 16:12 -0600, Jason Gunthorpe wrote:
> This is the rest of the code change stuff from the core libraries
> that sparse
> did not like.
> 
> There is one bug fix in here, the missing swap on INADDR_LOOPBACK.
> 
> Otherwise it is just small changes to make sparse happier.
> 
> The full series is here:
> 
> https://github.com/linux-rdma/rdma-core/pull/100
> 
> Which is enough hackery to make sparse run automatically from travis
> without
> triggering any sparse warnings, at least over the core libraries.
> 
> I think interested parties should look at the entire series on
> github. It is
> so large it probably won't fit on the list.
> 
> Bart and I were having an interesting discussion on github on this
> approach. It isn't clear to me if a different version of the 'sparse-
> include'
> setup could be designed.. But if someone else can figure out how to
> get the
> same result more robustly that would be great.
> 
> For now this actually works, and can run inside travis, which is much
> better
> than where we were..
> 
> Someone else(s) will have to sparse cleanup the providers and ibacm,
> they
> scare me.
> 
> Jason Gunthorpe (6):
>   Use NULL instead of 0 to silence sparse
>   INADDR_* needs to be byteswapped before being used
>   rstream: Use waitpid instead of wait
>   rdmacm: Use C11 stdatomic for all atomics
>   rsocket: Fix byte swapping when constructing path records
>   verbs: Consistently apply __attribute_const
> 
>  iwpmd/iwarp_pm_common.c             |  2 +-
>  iwpmd/iwarp_pm_helper.c             |  2 +-
>  libibverbs/examples/rc_pingpong.c   |  2 +-
>  libibverbs/examples/srq_pingpong.c  |  2 +-
>  libibverbs/examples/uc_pingpong.c   |  2 +-
>  libibverbs/examples/ud_pingpong.c   |  2 +-
>  libibverbs/examples/xsrq_pingpong.c |  2 +-
>  libibverbs/verbs.c                  |  8 +++---
>  libibverbs/verbs.h                  |  8 +++---
>  librdmacm/cma.h                     | 49 ++++-----------------------
> ----------
>  librdmacm/examples/cmatose.c        |  4 +--
>  librdmacm/examples/mckey.c          |  2 +-
>  librdmacm/examples/rcopy.c          |  2 +-
>  librdmacm/examples/riostream.c      |  2 +-
>  librdmacm/examples/rstream.c        |  6 ++---
>  librdmacm/examples/udaddy.c         |  2 +-
>  librdmacm/preload.c                 | 14 +++++------
>  librdmacm/rsocket.c                 | 17 ++++++-------
>  rdma-ndd/rdma-ndd.c                 |  6 ++---
>  19 files changed, 46 insertions(+), 88 deletions(-)

Thanks Jason, series merged.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

--
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