[PATCH 00/28] Patches for userspace

[PATCH 00/28] Patches for userspace

[Jason Gunthorpe]

Hello Everyone,

I have audited rdma-plumbing for correct compilation, compiler compatability
and warnings. This has resulted in a number of patches to basically
everything, I am presenting them here seperately from rdma-plumbing to get
them picked up and reviewed.

If you missed my earlier emails, read about rdma-plumbing here:
  http://www.spinics.net/lists/linux-rdma/msg39026.html
  http://www.spinics.net/lists/linux-rdma/msg39328.html
  https://github.com/jgunthorpe/rdma-plumbing

If you are maintaining one of the 17 packages being absorbed into
rdma-plumbing please do one of the following:

1) Take your patches into your git tree and send me a note. These patches
   will need to be applied with patch -p2, and sometimes a bit more
   fussing as they are against the initial merge in rdma-plubming.
   It is much too much work for me to create patches against each of
   the 17 repos individually.
2) Send a Acked-by/Reviewed-by and the patch will be carried in
   rdma-plumbing.
3) Do nothing. I will mark you as retired in the rdma-plumbing
   MAINTAINERS file.

Within rdma-plumbing, the net result of all these patches (and more) is clean
compilation on gcc 6.1/clang 3.8 with a high warning level (-Wall -Wextra
-Wno-sign-compare -Wno-unused-parameter), which is highly desired.

Jason Gunthorpe (28):
  Fix bogus executable file permissions
  Include pthreads in the provider libraries
  Be explicit about _GNU_SOURCE
  cxgb3/4: Display correct version number in error message
  hfi/ipath: Use the name of the provider for the .driver file
  i40iw: Avoid gcc warning -Wint-to-pointer-cast
  iwpm: Add AM_INIT_AUTOMAKE([subdir-objects])
  ibcm: Actually use the version script when linking
  mlx5: Fix gcc 6.4 uninitialized variable warning
  nes: Fix clang 3.6 warning
    -Wtautological-constant-out-of-range-compare
  ocrdma: Fix incorrect type of ibwc_status
  ocrdma: Fix incorrect enum constant
  rdmacm: Use correct format specifier for size_t
  rdmacm: Control symbol export from librspreload
  umad: Include umad.h in the canonical way
  umad: Fix incorrect arguments to umad_register2 in tests
  verbs: Fix incorrect type of len
  verbs: Fix clang 3.6 warning -Wtautological-compare
  verbs: Avoid gcc 6.1 warning -Wunused-variable
  verbs: Use inttypes.h format string macros
  mlx5: Avoid gcc 5.4 warning -Wempty-body
  nes: Avoid gcc 6.1 warning -Wmisleading-indentation
  rxe: Avoid gcc 5.4 warning -Wswitch
  Fix gcc 5.4, clang 3.6 warnings about unused objects
  Avoid gcc 5.4 warning -Wtype-limits
  Avoid gcc 5.4 warning -Wunused-result
  Avoid clang 3.6 warning -Wmissing-field-initializers
  Avoid gcc warning -Wpointer-to-int-cast

 iwpm/Makefile.am                                  |  2 +-
 iwpm/configure.ac                                 |  2 +-
 iwpm/src/iwarp_pm_helper.c                        |  2 +-
 libcxgb3/Makefile.am                              |  2 +-
 libcxgb3/configure.in                             |  2 +
 libcxgb3/src/iwch.c                               |  4 +-
 libcxgb3/src/qp.c                                 | 41 ----------------
 libcxgb4/Makefile.am                              |  2 +-
 libcxgb4/configure.in                             |  2 +
 libcxgb4/src/dev.c                                |  6 +--
 libhfi1verbs/Makefile.am                          |  6 +--
 libhfi1verbs/configure.in                         |  2 +
 libhfi1verbs/{hfi1.driver => hfi1verbs.driver}    |  0
 libhfi1verbs/src/verbs.c                          |  2 +-
 libi40iw/Makefile.am                              |  2 +-
 libi40iw/configure.ac                             |  2 +
 libi40iw/src/i40iw_uverbs.c                       |  6 +--
 libibcm/Makefile.am                               |  6 +--
 libibcm/src/cm.c                                  |  2 +-
 libibumad/src/sysfs.c                             |  3 --
 libibumad/src/umad.c                              |  2 +-
 libibumad/tests/umad_reg2_compat.c                |  5 +-
 libibumad/tests/umad_register2.c                  |  8 +--
 libibverbs/configure.ac                           |  1 -
 libibverbs/examples/asyncwatch.c                  |  4 +-
 libibverbs/examples/devinfo.c                     |  4 +-
 libibverbs/examples/pingpong.c                    |  2 +-
 libibverbs/examples/rc_pingpong.c                 | 21 ++++----
 libibverbs/examples/srq_pingpong.c                | 19 ++++---
 libibverbs/examples/uc_pingpong.c                 | 21 ++++----
 libibverbs/examples/ud_pingpong.c                 | 23 ++++-----
 libibverbs/examples/xsrq_pingpong.c               |  6 +--
 libibverbs/src/device.c                           |  2 +-
 libibverbs/src/init.c                             |  2 +-
 libibverbs/src/neigh.c                            | 21 ++++----
 libibverbs/src/sysfs.c                            |  2 +-
 libipathverbs/Makefile.am                         |  6 +--
 libipathverbs/configure.in                        |  2 +
 libipathverbs/{ipath.driver => ipathverbs.driver} |  0
 libipathverbs/src/verbs.c                         |  2 +-
 libipathverbs/truescale-serdes.cmds               |  0
 libmlx4/Makefile.am                               |  2 +-
 libmlx4/configure.ac                              |  2 +
 libmlx4/src/dbrec.c                               |  2 +-
 libmlx5/Makefile.am                               |  2 +-
 libmlx5/configure.ac                              |  3 ++
 libmlx5/src/cq.c                                  |  3 ++
 libmlx5/src/dbrec.c                               |  3 +-
 libmlx5/src/mlx5.c                                |  3 +-
 libmlx5/src/mlx5.h                                |  6 ++-
 libmlx5/src/qp.c                                  |  7 ---
 libmlx5/src/verbs.c                               | 14 +-----
 libmthca/Makefile.am                              |  2 +-
 libmthca/configure.in                             |  2 +
 libmthca/src/memfree.c                            |  2 +-
 libnes/Makefile.am                                |  2 +-
 libnes/configure.in                               |  2 +
 libnes/src/nes_uverbs.c                           | 29 ++++++-----
 libocrdma/Makefile.am                             |  2 +-
 libocrdma/configure.in                            |  2 +
 libocrdma/src/ocrdma_verbs.c                      |  7 +--
 librdmacm/Makefile.am                             |  7 ++-
 librdmacm/examples/cmtime.c                       | 16 ------
 librdmacm/examples/rping.c                        |  4 +-
 librdmacm/examples/udpong.c                       |  2 +-
 librdmacm/src/acm.c                               |  3 +-
 librdmacm/src/librspreload.map                    | 33 +++++++++++++
 librdmacm/src/preload.c                           |  6 +--
 librdmacm/src/rsocket.c                           | 60 ++++++++++++++++-------
 librxe/Makefile.am                                |  2 +-
 librxe/configure.in                               |  2 +
 librxe/src/rxe.c                                  |  6 +++
 72 files changed, 256 insertions(+), 231 deletions(-)
 rename libhfi1verbs/{hfi1.driver => hfi1verbs.driver} (100%)
 mode change 100755 => 100644 libibcm/src/cm.c
 rename libipathverbs/{ipath.driver => ipathverbs.driver} (100%)
 mode change 100644 => 100755 libipathverbs/truescale-serdes.cmds
 create mode 100644 librdmacm/src/librspreload.map

-- 
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 01/28] Fix bogus executable file permissions

[Jason Gunthorpe]

- cm.c is a source file and should not be executable
- truescale-serdes.cmds is a script and should be executable

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libibcm/src/cm.c                    | 0
 libipathverbs/truescale-serdes.cmds | 0
 2 files changed, 0 insertions(+), 0 deletions(-)
 mode change 100755 => 100644 libibcm/src/cm.c
 mode change 100644 => 100755 libipathverbs/truescale-serdes.cmds

diff --git a/libibcm/src/cm.c b/libibcm/src/cm.c
old mode 100755
new mode 100644
diff --git a/libipathverbs/truescale-serdes.cmds b/libipathverbs/truescale-serdes.cmds
old mode 100644
new mode 100755
-- 
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 02/28] Include pthreads in the provider libraries

[Jason Gunthorpe]

It is a mistake to not explicitly link to the libraries required.
Not linking causes the symbols to drop the symbol version which could
cause runtime problems down the road if pthreads ever goes through
another symbol version change.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libcxgb3/configure.in      | 2 ++
 libcxgb4/configure.in      | 2 ++
 libhfi1verbs/configure.in  | 2 ++
 libi40iw/configure.ac      | 2 ++
 libipathverbs/configure.in | 2 ++
 libmlx4/configure.ac       | 2 ++
 libmlx5/configure.ac       | 3 +++
 libmthca/configure.in      | 2 ++
 libnes/configure.in        | 2 ++
 libocrdma/configure.in     | 2 ++
 librxe/configure.in        | 2 ++
 11 files changed, 23 insertions(+)

diff --git a/libcxgb3/configure.in b/libcxgb3/configure.in
index 9efc82d34f33..db9177230cae 100644
--- a/libcxgb3/configure.in
+++ b/libcxgb3/configure.in
@@ -24,6 +24,8 @@ then
 AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
     AC_MSG_ERROR([ibv_get_device_list() not found.  libcxgb3 requires libibverbs.]))
 fi
+AC_CHECK_LIB(pthread, pthread_mutex_init, [],
+    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires libpthread.]))
 
 dnl Checks for header files.
 AC_CHECK_HEADERS(sysfs/libsysfs.h)
diff --git a/libcxgb4/configure.in b/libcxgb4/configure.in
index 4a74596ce8c2..336ab1c57daf 100644
--- a/libcxgb4/configure.in
+++ b/libcxgb4/configure.in
@@ -24,6 +24,8 @@ then
 AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
     AC_MSG_ERROR([ibv_get_device_list() not found.  libcxgb4 requires libibverbs.]))
 fi
+AC_CHECK_LIB(pthread, pthread_mutex_init, [],
+    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires libpthread.]))
 
 dnl Checks for header files.
 AC_CHECK_HEADERS(sysfs/libsysfs.h)
diff --git a/libhfi1verbs/configure.in b/libhfi1verbs/configure.in
index 7611a55bae20..3f6e6779ce74 100644
--- a/libhfi1verbs/configure.in
+++ b/libhfi1verbs/configure.in
@@ -81,6 +81,8 @@ AC_PROG_CC
 dnl Checks for libraries
 AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
     AC_MSG_ERROR([ibv_get_device_list() not found.  libhfi1verbs requires libibverbs.]))
+AC_CHECK_LIB(pthread, pthread_mutex_init, [],
+    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires libpthread.]))
 
 dnl Checks for header files.
 AC_CHECK_HEADER(infiniband/driver.h, [],
diff --git a/libi40iw/configure.ac b/libi40iw/configure.ac
index 04e246d75366..e3849b03561b 100644
--- a/libi40iw/configure.ac
+++ b/libi40iw/configure.ac
@@ -23,6 +23,8 @@ then
 AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
     AC_MSG_ERROR([ibv_get_device_list() not found.  libi40iw requires libibverbs.]))
 fi
+AC_CHECK_LIB(pthread, pthread_mutex_init, [],
+    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires libpthread.]))
 
 dnl Checks for header files.
 AC_CHECK_HEADERS(sysfs/libsysfs.h)
diff --git a/libipathverbs/configure.in b/libipathverbs/configure.in
index 70093c3a8acb..f2cf2e3f90ee 100644
--- a/libipathverbs/configure.in
+++ b/libipathverbs/configure.in
@@ -81,6 +81,8 @@ AC_CHECK_SIZEOF(long)
 dnl Checks for library functions
 AC_CHECK_FUNCS(ibv_read_sysfs_file ibv_dontfork_range ibv_dofork_range \
     ibv_register_driver)
+AC_CHECK_LIB(pthread, pthread_mutex_init, [],
+    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires libpthread.]))
 
 dnl Now check if for libibverbs 1.0 vs 1.1
 dummy=if$$
diff --git a/libmlx4/configure.ac b/libmlx4/configure.ac
index a50faa2e7353..01bf7cbcf163 100644
--- a/libmlx4/configure.ac
+++ b/libmlx4/configure.ac
@@ -29,6 +29,8 @@ AC_LANG([C])
 dnl Checks for libraries
 AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
     AC_MSG_ERROR([ibv_get_device_list() not found.  libmlx4 requires libibverbs.]))
+AC_CHECK_LIB(pthread, pthread_mutex_init, [],
+    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires libpthread.]))
 
 dnl Checks for header files.
 AC_CHECK_HEADER(infiniband/driver.h, [],
diff --git a/libmlx5/configure.ac b/libmlx5/configure.ac
index c7591268a137..cd235474245e 100644
--- a/libmlx5/configure.ac
+++ b/libmlx5/configure.ac
@@ -51,6 +51,9 @@ AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
 AC_CHECK_LIB(ibverbs, ibv_register_driver_ext,
     AC_DEFINE(HAVE_IBV_EXT, 1, [adding verbs extension support]))
 
+AC_CHECK_LIB(pthread, pthread_mutex_init, [],
+    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires libpthread.]))
+
 dnl Checks for header files.
 AC_CHECK_HEADER(infiniband/driver.h, [],
     AC_MSG_ERROR([<infiniband/driver.h> not found.  libmlx5 requires libibverbs.]))
diff --git a/libmthca/configure.in b/libmthca/configure.in
index ffd5db6962f6..fb665ed2dd40 100644
--- a/libmthca/configure.in
+++ b/libmthca/configure.in
@@ -30,6 +30,8 @@ AC_PROG_CC
 dnl Checks for libraries
 AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
     AC_MSG_ERROR([ibv_get_device_list() not found.  libmthca requires libibverbs.]))
+AC_CHECK_LIB(pthread, pthread_mutex_init, [],
+    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires libpthread.]))
 
 dnl Checks for header files.
 AC_CHECK_HEADER(infiniband/driver.h, [],
diff --git a/libnes/configure.in b/libnes/configure.in
index 0cde51420753..a25d129cc159 100644
--- a/libnes/configure.in
+++ b/libnes/configure.in
@@ -23,6 +23,8 @@ then
 AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
     AC_MSG_ERROR([ibv_get_device_list() not found.  libnes requires libibverbs.]))
 fi
+AC_CHECK_LIB(pthread, pthread_mutex_init, [],
+    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires libpthread.]))
 
 dnl Checks for header files.
 AC_CHECK_HEADERS(sysfs/libsysfs.h)
diff --git a/libocrdma/configure.in b/libocrdma/configure.in
index a0c8175035f9..2fd54b5ecd72 100644
--- a/libocrdma/configure.in
+++ b/libocrdma/configure.in
@@ -24,6 +24,8 @@ then
 AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
     AC_MSG_ERROR([ibv_get_device_list() not found.  libocrdma requires libibverbs.]))
 fi
+AC_CHECK_LIB(pthread, pthread_mutex_init, [],
+    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires libpthread.]))
 
 dnl Checks for header files.
 AC_CHECK_HEADERS(sysfs/libsysfs.h)
diff --git a/librxe/configure.in b/librxe/configure.in
index 4f349cc9bae7..8927cef50468 100644
--- a/librxe/configure.in
+++ b/librxe/configure.in
@@ -36,6 +36,8 @@ if test x$enable_repackage = x || test x$enable_repackage = xno; then
         AC_MSG_ERROR([<infiniband/driver.h> not found.  librxe requires libibverbs.]))
     AC_CHECK_FUNCS(ibv_read_sysfs_file ibv_dontfork_range ibv_dofork_range \
         ibv_register_driver)
+    AC_CHECK_LIB(pthread, pthread_mutex_init, [],
+    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires libpthread.]))
 
     dummy=if$$
     cat <<IBV_VERSION > $dummy.c
-- 
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 02/28] Include pthreads in the provider libraries

[Steve Wise]

> It is a mistake to not explicitly link to the libraries required.
> Not linking causes the symbols to drop the symbol version which could
> cause runtime problems down the road if pthreads ever goes through
> another symbol version change.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> ---
>  libcxgb3/configure.in      | 2 ++
>  libcxgb4/configure.in      | 2 ++
>  libhfi1verbs/configure.in  | 2 ++
>  libi40iw/configure.ac      | 2 ++
>  libipathverbs/configure.in | 2 ++
>  libmlx4/configure.ac       | 2 ++
>  libmlx5/configure.ac       | 3 +++
>  libmthca/configure.in      | 2 ++
>  libnes/configure.in        | 2 ++
>  libocrdma/configure.in     | 2 ++
>  librxe/configure.in        | 2 ++
>  11 files changed, 23 insertions(+)
> 
> diff --git a/libcxgb3/configure.in b/libcxgb3/configure.in
> index 9efc82d34f33..db9177230cae 100644
> --- a/libcxgb3/configure.in
> +++ b/libcxgb3/configure.in
> @@ -24,6 +24,8 @@ then
>  AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
>      AC_MSG_ERROR([ibv_get_device_list() not found.  libcxgb3 requires
> libibverbs.]))
>  fi
> +AC_CHECK_LIB(pthread, pthread_mutex_init, [],
> +    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires
> libpthread.]))
> 

Nit: This should be "libcxgb3 requires..."  Similar change needed for all of
this patch.


--
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 02/28] Include pthreads in the provider libraries

[Jason Gunthorpe]

On Wed, Sep 07, 2016 at 09:41:11AM -0500, Steve Wise wrote:
> > +AC_CHECK_LIB(pthread, pthread_mutex_init, [],
> > +    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires
> > libpthread.]))
> > 
> 
> Nit: This should be "libcxgb3 requires..."  Similar change needed for all of
> this patch.

Sure.

>From 44ce61764b9293468cfd8b13191c453910a6ab15 Mon Sep 17 00:00:00 2001
From: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Date: Wed, 7 Sep 2016 10:02:47 -0600
Subject: [PATCH] Include pthreads in the provider libraries

It is a mistake to not explicitly link to the libraries required.
Not linking causes the symbols to drop the symbol version which could
cause runtime problems down the road if pthreads ever goes through
another symbol version change.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libcxgb3/configure.in      | 2 ++
 libcxgb4/configure.in      | 2 ++
 libhfi1verbs/configure.in  | 2 ++
 libi40iw/configure.ac      | 2 ++
 libipathverbs/configure.in | 2 ++
 libmlx4/configure.ac       | 2 ++
 libmlx5/configure.ac       | 3 +++
 libmthca/configure.in      | 2 ++
 libnes/configure.in        | 2 ++
 libocrdma/configure.in     | 2 ++
 librxe/configure.in        | 2 ++
 11 files changed, 23 insertions(+)

diff --git a/libcxgb3/configure.in b/libcxgb3/configure.in
index 9efc82d34f33..cc254a6e5d8d 100644
--- a/libcxgb3/configure.in
+++ b/libcxgb3/configure.in
@@ -24,6 +24,8 @@ then
 AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
     AC_MSG_ERROR([ibv_get_device_list() not found.  libcxgb3 requires libibverbs.]))
 fi
+AC_CHECK_LIB(pthread, pthread_mutex_init, [],
+    AC_MSG_ERROR([pthread_mutex_init() not found.  libcxgb3 requires libpthread.]))
 
 dnl Checks for header files.
 AC_CHECK_HEADERS(sysfs/libsysfs.h)
diff --git a/libcxgb4/configure.in b/libcxgb4/configure.in
index 4a74596ce8c2..2bb930399a3e 100644
--- a/libcxgb4/configure.in
+++ b/libcxgb4/configure.in
@@ -24,6 +24,8 @@ then
 AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
     AC_MSG_ERROR([ibv_get_device_list() not found.  libcxgb4 requires libibverbs.]))
 fi
+AC_CHECK_LIB(pthread, pthread_mutex_init, [],
+    AC_MSG_ERROR([pthread_mutex_init() not found.  libcxgb4 requires libpthread.]))
 
 dnl Checks for header files.
 AC_CHECK_HEADERS(sysfs/libsysfs.h)
diff --git a/libhfi1verbs/configure.in b/libhfi1verbs/configure.in
index 7611a55bae20..bdea514c25b4 100644
--- a/libhfi1verbs/configure.in
+++ b/libhfi1verbs/configure.in
@@ -81,6 +81,8 @@ AC_PROG_CC
 dnl Checks for libraries
 AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
     AC_MSG_ERROR([ibv_get_device_list() not found.  libhfi1verbs requires libibverbs.]))
+AC_CHECK_LIB(pthread, pthread_mutex_init, [],
+    AC_MSG_ERROR([pthread_mutex_init() not found.  libhfi1verbs requires libpthread.]))
 
 dnl Checks for header files.
 AC_CHECK_HEADER(infiniband/driver.h, [],
diff --git a/libi40iw/configure.ac b/libi40iw/configure.ac
index 04e246d75366..108ffd5999b9 100644
--- a/libi40iw/configure.ac
+++ b/libi40iw/configure.ac
@@ -23,6 +23,8 @@ then
 AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
     AC_MSG_ERROR([ibv_get_device_list() not found.  libi40iw requires libibverbs.]))
 fi
+AC_CHECK_LIB(pthread, pthread_mutex_init, [],
+    AC_MSG_ERROR([pthread_mutex_init() not found.  libi40iw requires libpthread.]))
 
 dnl Checks for header files.
 AC_CHECK_HEADERS(sysfs/libsysfs.h)
diff --git a/libipathverbs/configure.in b/libipathverbs/configure.in
index 70093c3a8acb..9718d56e4411 100644
--- a/libipathverbs/configure.in
+++ b/libipathverbs/configure.in
@@ -81,6 +81,8 @@ AC_CHECK_SIZEOF(long)
 dnl Checks for library functions
 AC_CHECK_FUNCS(ibv_read_sysfs_file ibv_dontfork_range ibv_dofork_range \
     ibv_register_driver)
+AC_CHECK_LIB(pthread, pthread_mutex_init, [],
+    AC_MSG_ERROR([pthread_mutex_init() not found. libipathverbs requires libpthread.]))
 
 dnl Now check if for libibverbs 1.0 vs 1.1
 dummy=if$$
diff --git a/libmlx4/configure.ac b/libmlx4/configure.ac
index a50faa2e7353..536a07921d7e 100644
--- a/libmlx4/configure.ac
+++ b/libmlx4/configure.ac
@@ -29,6 +29,8 @@ AC_LANG([C])
 dnl Checks for libraries
 AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
     AC_MSG_ERROR([ibv_get_device_list() not found.  libmlx4 requires libibverbs.]))
+AC_CHECK_LIB(pthread, pthread_mutex_init, [],
+    AC_MSG_ERROR([pthread_mutex_init() not found.  libmlx4 requires libpthread.]))
 
 dnl Checks for header files.
 AC_CHECK_HEADER(infiniband/driver.h, [],
diff --git a/libmlx5/configure.ac b/libmlx5/configure.ac
index c7591268a137..dca7c81a88ce 100644
--- a/libmlx5/configure.ac
+++ b/libmlx5/configure.ac
@@ -51,6 +51,9 @@ AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
 AC_CHECK_LIB(ibverbs, ibv_register_driver_ext,
     AC_DEFINE(HAVE_IBV_EXT, 1, [adding verbs extension support]))
 
+AC_CHECK_LIB(pthread, pthread_mutex_init, [],
+    AC_MSG_ERROR([pthread_mutex_init() not found.  libmlx5 requires libpthread.]))
+
 dnl Checks for header files.
 AC_CHECK_HEADER(infiniband/driver.h, [],
     AC_MSG_ERROR([<infiniband/driver.h> not found.  libmlx5 requires libibverbs.]))
diff --git a/libmthca/configure.in b/libmthca/configure.in
index ffd5db6962f6..c5454436bf8f 100644
--- a/libmthca/configure.in
+++ b/libmthca/configure.in
@@ -30,6 +30,8 @@ AC_PROG_CC
 dnl Checks for libraries
 AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
     AC_MSG_ERROR([ibv_get_device_list() not found.  libmthca requires libibverbs.]))
+AC_CHECK_LIB(pthread, pthread_mutex_init, [],
+    AC_MSG_ERROR([pthread_mutex_init() not found.  libmthca requires libpthread.]))
 
 dnl Checks for header files.
 AC_CHECK_HEADER(infiniband/driver.h, [],
diff --git a/libnes/configure.in b/libnes/configure.in
index 0cde51420753..08e9948ed13c 100644
--- a/libnes/configure.in
+++ b/libnes/configure.in
@@ -23,6 +23,8 @@ then
 AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
     AC_MSG_ERROR([ibv_get_device_list() not found.  libnes requires libibverbs.]))
 fi
+AC_CHECK_LIB(pthread, pthread_mutex_init, [],
+    AC_MSG_ERROR([pthread_mutex_init() not found.  libnes requires libpthread.]))
 
 dnl Checks for header files.
 AC_CHECK_HEADERS(sysfs/libsysfs.h)
diff --git a/libocrdma/configure.in b/libocrdma/configure.in
index a0c8175035f9..659d79f52f25 100644
--- a/libocrdma/configure.in
+++ b/libocrdma/configure.in
@@ -24,6 +24,8 @@ then
 AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
     AC_MSG_ERROR([ibv_get_device_list() not found.  libocrdma requires libibverbs.]))
 fi
+AC_CHECK_LIB(pthread, pthread_mutex_init, [],
+    AC_MSG_ERROR([pthread_mutex_init() not found.  libocrdma requires libpthread.]))
 
 dnl Checks for header files.
 AC_CHECK_HEADERS(sysfs/libsysfs.h)
diff --git a/librxe/configure.in b/librxe/configure.in
index 4f349cc9bae7..957d65000704 100644
--- a/librxe/configure.in
+++ b/librxe/configure.in
@@ -36,6 +36,8 @@ if test x$enable_repackage = x || test x$enable_repackage = xno; then
         AC_MSG_ERROR([<infiniband/driver.h> not found.  librxe requires libibverbs.]))
     AC_CHECK_FUNCS(ibv_read_sysfs_file ibv_dontfork_range ibv_dofork_range \
         ibv_register_driver)
+    AC_CHECK_LIB(pthread, pthread_mutex_init, [],
+    AC_MSG_ERROR([pthread_mutex_init() not found.  librxe requires libpthread.]))
 
     dummy=if$$
     cat <<IBV_VERSION > $dummy.c
-- 
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 02/28] Include pthreads in the provider libraries

[Steve Wise]

> It is a mistake to not explicitly link to the libraries required.
> Not linking causes the symbols to drop the symbol version which could
> cause runtime problems down the road if pthreads ever goes through
> another symbol version change.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

> ---
>  libcxgb3/configure.in      | 2 ++
>  libcxgb4/configure.in      | 2 ++

Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>

libcxgb* patches applied.

Thanks,

Steve.

--
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 02/28] Include pthreads in the provider libraries

[Yishai Hadas]

On 9/6/2016 12:07 AM, Jason Gunthorpe wrote:
> It is a mistake to not explicitly link to the libraries required.
> Not linking causes the symbols to drop the symbol version which could
> cause runtime problems down the road if pthreads ever goes through
> another symbol version change.

Where in this patch the provider links explicitly with pthreads as you 
pointed that should be done ? can see only a check that pthread exists 
and has the pthread_mutex_init symbol.


> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> ---
>  libcxgb3/configure.in      | 2 ++
>  libcxgb4/configure.in      | 2 ++
>  libhfi1verbs/configure.in  | 2 ++
>  libi40iw/configure.ac      | 2 ++
>  libipathverbs/configure.in | 2 ++
>  libmlx4/configure.ac       | 2 ++
>  libmlx5/configure.ac       | 3 +++
>  libmthca/configure.in      | 2 ++
>  libnes/configure.in        | 2 ++
>  libocrdma/configure.in     | 2 ++
>  librxe/configure.in        | 2 ++
>  11 files changed, 23 insertions(+)
>
> diff --git a/libcxgb3/configure.in b/libcxgb3/configure.in
> index 9efc82d34f33..db9177230cae 100644
> --- a/libcxgb3/configure.in
> +++ b/libcxgb3/configure.in
> @@ -24,6 +24,8 @@ then
>  AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
>      AC_MSG_ERROR([ibv_get_device_list() not found.  libcxgb3 requires libibverbs.]))
>  fi
> +AC_CHECK_LIB(pthread, pthread_mutex_init, [],
> +    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires libpthread.]))
>
>  dnl Checks for header files.
>  AC_CHECK_HEADERS(sysfs/libsysfs.h)
> diff --git a/libcxgb4/configure.in b/libcxgb4/configure.in
> index 4a74596ce8c2..336ab1c57daf 100644
> --- a/libcxgb4/configure.in
> +++ b/libcxgb4/configure.in
> @@ -24,6 +24,8 @@ then
>  AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
>      AC_MSG_ERROR([ibv_get_device_list() not found.  libcxgb4 requires libibverbs.]))
>  fi
> +AC_CHECK_LIB(pthread, pthread_mutex_init, [],
> +    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires libpthread.]))
>
>  dnl Checks for header files.
>  AC_CHECK_HEADERS(sysfs/libsysfs.h)
> diff --git a/libhfi1verbs/configure.in b/libhfi1verbs/configure.in
> index 7611a55bae20..3f6e6779ce74 100644
> --- a/libhfi1verbs/configure.in
> +++ b/libhfi1verbs/configure.in
> @@ -81,6 +81,8 @@ AC_PROG_CC
>  dnl Checks for libraries
>  AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
>      AC_MSG_ERROR([ibv_get_device_list() not found.  libhfi1verbs requires libibverbs.]))
> +AC_CHECK_LIB(pthread, pthread_mutex_init, [],
> +    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires libpthread.]))
>
>  dnl Checks for header files.
>  AC_CHECK_HEADER(infiniband/driver.h, [],
> diff --git a/libi40iw/configure.ac b/libi40iw/configure.ac
> index 04e246d75366..e3849b03561b 100644
> --- a/libi40iw/configure.ac
> +++ b/libi40iw/configure.ac
> @@ -23,6 +23,8 @@ then
>  AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
>      AC_MSG_ERROR([ibv_get_device_list() not found.  libi40iw requires libibverbs.]))
>  fi
> +AC_CHECK_LIB(pthread, pthread_mutex_init, [],
> +    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires libpthread.]))
>
>  dnl Checks for header files.
>  AC_CHECK_HEADERS(sysfs/libsysfs.h)
> diff --git a/libipathverbs/configure.in b/libipathverbs/configure.in
> index 70093c3a8acb..f2cf2e3f90ee 100644
> --- a/libipathverbs/configure.in
> +++ b/libipathverbs/configure.in
> @@ -81,6 +81,8 @@ AC_CHECK_SIZEOF(long)
>  dnl Checks for library functions
>  AC_CHECK_FUNCS(ibv_read_sysfs_file ibv_dontfork_range ibv_dofork_range \
>      ibv_register_driver)
> +AC_CHECK_LIB(pthread, pthread_mutex_init, [],
> +    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires libpthread.]))
>
>  dnl Now check if for libibverbs 1.0 vs 1.1
>  dummy=if$$
> diff --git a/libmlx4/configure.ac b/libmlx4/configure.ac
> index a50faa2e7353..01bf7cbcf163 100644
> --- a/libmlx4/configure.ac
> +++ b/libmlx4/configure.ac
> @@ -29,6 +29,8 @@ AC_LANG([C])
>  dnl Checks for libraries
>  AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
>      AC_MSG_ERROR([ibv_get_device_list() not found.  libmlx4 requires libibverbs.]))
> +AC_CHECK_LIB(pthread, pthread_mutex_init, [],
> +    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires libpthread.]))
>
>  dnl Checks for header files.
>  AC_CHECK_HEADER(infiniband/driver.h, [],
> diff --git a/libmlx5/configure.ac b/libmlx5/configure.ac
> index c7591268a137..cd235474245e 100644
> --- a/libmlx5/configure.ac
> +++ b/libmlx5/configure.ac
> @@ -51,6 +51,9 @@ AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
>  AC_CHECK_LIB(ibverbs, ibv_register_driver_ext,
>      AC_DEFINE(HAVE_IBV_EXT, 1, [adding verbs extension support]))
>
> +AC_CHECK_LIB(pthread, pthread_mutex_init, [],
> +    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires libpthread.]))
> +
>  dnl Checks for header files.
>  AC_CHECK_HEADER(infiniband/driver.h, [],
>      AC_MSG_ERROR([<infiniband/driver.h> not found.  libmlx5 requires libibverbs.]))
> diff --git a/libmthca/configure.in b/libmthca/configure.in
> index ffd5db6962f6..fb665ed2dd40 100644
> --- a/libmthca/configure.in
> +++ b/libmthca/configure.in
> @@ -30,6 +30,8 @@ AC_PROG_CC
>  dnl Checks for libraries
>  AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
>      AC_MSG_ERROR([ibv_get_device_list() not found.  libmthca requires libibverbs.]))
> +AC_CHECK_LIB(pthread, pthread_mutex_init, [],
> +    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires libpthread.]))
>
>  dnl Checks for header files.
>  AC_CHECK_HEADER(infiniband/driver.h, [],
> diff --git a/libnes/configure.in b/libnes/configure.in
> index 0cde51420753..a25d129cc159 100644
> --- a/libnes/configure.in
> +++ b/libnes/configure.in
> @@ -23,6 +23,8 @@ then
>  AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
>      AC_MSG_ERROR([ibv_get_device_list() not found.  libnes requires libibverbs.]))
>  fi
> +AC_CHECK_LIB(pthread, pthread_mutex_init, [],
> +    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires libpthread.]))
>
>  dnl Checks for header files.
>  AC_CHECK_HEADERS(sysfs/libsysfs.h)
> diff --git a/libocrdma/configure.in b/libocrdma/configure.in
> index a0c8175035f9..2fd54b5ecd72 100644
> --- a/libocrdma/configure.in
> +++ b/libocrdma/configure.in
> @@ -24,6 +24,8 @@ then
>  AC_CHECK_LIB(ibverbs, ibv_get_device_list, [],
>      AC_MSG_ERROR([ibv_get_device_list() not found.  libocrdma requires libibverbs.]))
>  fi
> +AC_CHECK_LIB(pthread, pthread_mutex_init, [],
> +    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires libpthread.]))
>
>  dnl Checks for header files.
>  AC_CHECK_HEADERS(sysfs/libsysfs.h)
> diff --git a/librxe/configure.in b/librxe/configure.in
> index 4f349cc9bae7..8927cef50468 100644
> --- a/librxe/configure.in
> +++ b/librxe/configure.in
> @@ -36,6 +36,8 @@ if test x$enable_repackage = x || test x$enable_repackage = xno; then
>          AC_MSG_ERROR([<infiniband/driver.h> not found.  librxe requires libibverbs.]))
>      AC_CHECK_FUNCS(ibv_read_sysfs_file ibv_dontfork_range ibv_dofork_range \
>          ibv_register_driver)
> +    AC_CHECK_LIB(pthread, pthread_mutex_init, [],
> +    AC_MSG_ERROR([pthread_mutex_init() not found.  libibverbs requires libpthread.]))
>
>      dummy=if$$
>      cat <<IBV_VERSION > $dummy.c
>

--
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 02/28] Include pthreads in the provider libraries

[Jason Gunthorpe]

On Wed, Sep 14, 2016 at 06:15:45PM +0300, Yishai Hadas wrote:
> On 9/6/2016 12:07 AM, Jason Gunthorpe wrote:
> >It is a mistake to not explicitly link to the libraries required.
> >Not linking causes the symbols to drop the symbol version which could
> >cause runtime problems down the road if pthreads ever goes through
> >another symbol version change.
> 
> Where in this patch the provider links explicitly with pthreads as you
> pointed that should be done ? can see only a check that pthread exists and
> has the pthread_mutex_init symbol.

AC_CHECK_LIB adds the library to the link line as well.

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

Re: [PATCH 02/28] Include pthreads in the provider libraries

[Yishai Hadas]

On 9/14/2016 7:01 PM, Jason Gunthorpe wrote:
> On Wed, Sep 14, 2016 at 06:15:45PM +0300, Yishai Hadas wrote:
>> On 9/6/2016 12:07 AM, Jason Gunthorpe wrote:
>>> It is a mistake to not explicitly link to the libraries required.
>>> Not linking causes the symbols to drop the symbol version which could
>>> cause runtime problems down the road if pthreads ever goes through
>>> another symbol version change.
>>
>> Where in this patch the provider links explicitly with pthreads as you
>> pointed that should be done ? can see only a check that pthread exists and
>> has the pthread_mutex_init symbol.
>
> AC_CHECK_LIB adds the library to the link line as well.
>

Just verified, this is correct, will take into libmlx4/libmlx5.

--
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 03/28] Be explicit about _GNU_SOURCE

[Jason Gunthorpe]

The recommended way to use this macro is at the top of the source file,
avoid globally setting it via 'gcc -D' as few source files actually
need it. In this tree we only need it in 17 out of 83 sources.

_GNU_SOURCE changes the behaviour of a few select calls away from the C99
standard and should generally be minimized.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 iwpm/Makefile.am                    | 2 +-
 libcxgb3/Makefile.am                | 2 +-
 libcxgb4/Makefile.am                | 2 +-
 libhfi1verbs/Makefile.am            | 2 +-
 libi40iw/Makefile.am                | 2 +-
 libibcm/Makefile.am                 | 2 +-
 libibcm/src/cm.c                    | 2 +-
 libibumad/src/sysfs.c               | 3 ---
 libibverbs/configure.ac             | 1 -
 libibverbs/examples/asyncwatch.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/src/device.c             | 2 +-
 libibverbs/src/init.c               | 2 +-
 libibverbs/src/sysfs.c              | 2 +-
 libipathverbs/Makefile.am           | 2 +-
 libmlx4/Makefile.am                 | 2 +-
 libmlx4/src/dbrec.c                 | 2 +-
 libmlx5/Makefile.am                 | 2 +-
 libmlx5/src/dbrec.c                 | 3 +--
 libmlx5/src/mlx5.c                  | 3 +--
 libmthca/Makefile.am                | 2 +-
 libmthca/src/memfree.c              | 2 +-
 libnes/Makefile.am                  | 2 +-
 libocrdma/Makefile.am               | 2 +-
 librdmacm/Makefile.am               | 2 +-
 librdmacm/examples/rping.c          | 2 +-
 librdmacm/src/preload.c             | 2 +-
 librdmacm/src/rsocket.c             | 2 +-
 librxe/Makefile.am                  | 2 +-
 33 files changed, 31 insertions(+), 37 deletions(-)

diff --git a/iwpm/Makefile.am b/iwpm/Makefile.am
index 1a1473c4bd78..fbb4334da961 100644
--- a/iwpm/Makefile.am
+++ b/iwpm/Makefile.am
@@ -1,5 +1,5 @@
 AM_CPPFLAGS = -I$(includedir)/libnl3 -I/usr/include/libnl3
-AM_CFLAGS = -O2 -Wall -D_GNU_SOURCE
+AM_CFLAGS = -O2 -Wall
 ACLOCAL_AMFLAGS = -I m4
 
 bin_PROGRAMS = src/iwpmd
diff --git a/libcxgb3/Makefile.am b/libcxgb3/Makefile.am
index 9fa18de0c79d..99f1cae08531 100644
--- a/libcxgb3/Makefile.am
+++ b/libcxgb3/Makefile.am
@@ -2,7 +2,7 @@
 
 lib_LTLIBRARIES = src/libcxgb3.la
 
-AM_CFLAGS = -g -Wall -D_GNU_SOURCE
+AM_CFLAGS = -g -Wall
 
 if HAVE_LD_VERSION_SCRIPT
     cxgb3_version_script = -Wl,--version-script=$(srcdir)/src/iwch.map
diff --git a/libcxgb4/Makefile.am b/libcxgb4/Makefile.am
index 97f577d55734..f75d9641ce12 100644
--- a/libcxgb4/Makefile.am
+++ b/libcxgb4/Makefile.am
@@ -1,6 +1,6 @@
 lib_LTLIBRARIES = src/libcxgb4.la
 
-AM_CFLAGS = -g -Wall -D_GNU_SOURCE -DNDEBUG -DOVERFLOW_DETECTION -fno-strict-aliasing
+AM_CFLAGS = -g -Wall -DNDEBUG -DOVERFLOW_DETECTION -fno-strict-aliasing
 
 if HAVE_LD_VERSION_SCRIPT
     cxgb4_version_script = -Wl,--version-script=$(srcdir)/src/cxgb4.map
diff --git a/libhfi1verbs/Makefile.am b/libhfi1verbs/Makefile.am
index d3a109203742..2c690b0570fb 100644
--- a/libhfi1verbs/Makefile.am
+++ b/libhfi1verbs/Makefile.am
@@ -51,7 +51,7 @@
 # Copyright (c) 2007. QLogic Corp. All rights reserved.
 # Copyright (c) 2003, 2004, 2005. PathScale, Inc. All rights reserved.
 
-AM_CFLAGS = -g -Wall -D_GNU_SOURCE
+AM_CFLAGS = -g -Wall
 
 hfiverbs_version_script = @HFIVERBS_VERSION_SCRIPT@
 
diff --git a/libi40iw/Makefile.am b/libi40iw/Makefile.am
index 92772e5c8503..98d6f8ae1226 100644
--- a/libi40iw/Makefile.am
+++ b/libi40iw/Makefile.am
@@ -1,7 +1,7 @@
 lib_LTLIBRARIES = src/libi40iw.la
 
 AM_CPPFLAGS = -I$(srcdir)/src
-AM_CFLAGS = -O2 -Wall -D_GNU_SOURCE
+AM_CFLAGS = -O2 -Wall
 
 if HAVE_LD_VERSION_SCRIPT
     i40iw_version_script = -Wl,--version-script=$(srcdir)/src/i40iw.map
diff --git a/libibcm/Makefile.am b/libibcm/Makefile.am
index 2333930db548..e4be9f887b1c 100644
--- a/libibcm/Makefile.am
+++ b/libibcm/Makefile.am
@@ -3,7 +3,7 @@ INCLUDES = -I$(srcdir)/include
 lib_LTLIBRARIES = src/libibcm.la
 
 ACLOCAL_AMFLAGS = -I config
-AM_CFLAGS = -g -Wall -D_GNU_SOURCE
+AM_CFLAGS = -g -Wall
 
 src_libibcm_la_CFLAGS = $(AM_CFLAGS)
 
diff --git a/libibcm/src/cm.c b/libibcm/src/cm.c
index 996268141a04..5c56381fd5db 100644
--- a/libibcm/src/cm.c
+++ b/libibcm/src/cm.c
@@ -32,7 +32,7 @@
  *
  * $Id$
  */
-
+#define _GNU_SOURCE
 #if HAVE_CONFIG_H
 #  include <config.h>
 #endif /* HAVE_CONFIG_H */
diff --git a/libibumad/src/sysfs.c b/libibumad/src/sysfs.c
index 20448d6b85bd..5d9460851897 100644
--- a/libibumad/src/sysfs.c
+++ b/libibumad/src/sysfs.c
@@ -30,9 +30,6 @@
  * SOFTWARE.
  *
  */
-
-#define _GNU_SOURCE
-
 #if HAVE_CONFIG_H
 #  include <config.h>
 #endif				/* HAVE_CONFIG_H */
diff --git a/libibverbs/configure.ac b/libibverbs/configure.ac
index 90c33dfb7c75..a8046f3c5a72 100644
--- a/libibverbs/configure.ac
+++ b/libibverbs/configure.ac
@@ -11,7 +11,6 @@ m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])])
 
 dnl Checks for programs
 AC_PROG_CC
-AC_USE_SYSTEM_EXTENSIONS
 AC_PROG_LN_S
 LT_INIT
 
diff --git a/libibverbs/examples/asyncwatch.c b/libibverbs/examples/asyncwatch.c
index a77c1c8802c2..df1261503b7d 100644
--- a/libibverbs/examples/asyncwatch.c
+++ b/libibverbs/examples/asyncwatch.c
@@ -29,7 +29,7 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-
+#define _GNU_SOURCE
 #if HAVE_CONFIG_H
 #  include <config.h>
 #endif /* HAVE_CONFIG_H */
diff --git a/libibverbs/examples/rc_pingpong.c b/libibverbs/examples/rc_pingpong.c
index 8d5835774590..967678362833 100644
--- a/libibverbs/examples/rc_pingpong.c
+++ b/libibverbs/examples/rc_pingpong.c
@@ -29,7 +29,7 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-
+#define _GNU_SOURCE
 #if HAVE_CONFIG_H
 #  include <config.h>
 #endif /* HAVE_CONFIG_H */
diff --git a/libibverbs/examples/srq_pingpong.c b/libibverbs/examples/srq_pingpong.c
index f61acb006c83..a1061c31972d 100644
--- a/libibverbs/examples/srq_pingpong.c
+++ b/libibverbs/examples/srq_pingpong.c
@@ -29,7 +29,7 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-
+#define _GNU_SOURCE
 #if HAVE_CONFIG_H
 #  include <config.h>
 #endif /* HAVE_CONFIG_H */
diff --git a/libibverbs/examples/uc_pingpong.c b/libibverbs/examples/uc_pingpong.c
index 272dc2668708..b25d16c79021 100644
--- a/libibverbs/examples/uc_pingpong.c
+++ b/libibverbs/examples/uc_pingpong.c
@@ -29,7 +29,7 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-
+#define _GNU_SOURCE
 #if HAVE_CONFIG_H
 #  include <config.h>
 #endif /* HAVE_CONFIG_H */
diff --git a/libibverbs/examples/ud_pingpong.c b/libibverbs/examples/ud_pingpong.c
index 6d32cedc1284..fa99b9e51dfb 100644
--- a/libibverbs/examples/ud_pingpong.c
+++ b/libibverbs/examples/ud_pingpong.c
@@ -29,7 +29,7 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-
+#define _GNU_SOURCE
 #if HAVE_CONFIG_H
 #  include <config.h>
 #endif /* HAVE_CONFIG_H */
diff --git a/libibverbs/examples/xsrq_pingpong.c b/libibverbs/examples/xsrq_pingpong.c
index 70c576098581..ff00180f2644 100644
--- a/libibverbs/examples/xsrq_pingpong.c
+++ b/libibverbs/examples/xsrq_pingpong.c
@@ -30,7 +30,7 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-
+#define _GNU_SOURCE
 #if HAVE_CONFIG_H
 #  include <config.h>
 #endif /* HAVE_CONFIG_H */
diff --git a/libibverbs/src/device.c b/libibverbs/src/device.c
index e520295af0c4..937e6918eb10 100644
--- a/libibverbs/src/device.c
+++ b/libibverbs/src/device.c
@@ -30,7 +30,7 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-
+#define _GNU_SOURCE
 #if HAVE_CONFIG_H
 #  include <config.h>
 #endif /* HAVE_CONFIG_H */
diff --git a/libibverbs/src/init.c b/libibverbs/src/init.c
index dbdd7954a090..bca0e02d11e1 100644
--- a/libibverbs/src/init.c
+++ b/libibverbs/src/init.c
@@ -30,7 +30,7 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-
+#define _GNU_SOURCE
 #if HAVE_CONFIG_H
 #  include <config.h>
 #endif /* HAVE_CONFIG_H */
diff --git a/libibverbs/src/sysfs.c b/libibverbs/src/sysfs.c
index 0b6ad1e8cc9e..2e68da4bc97f 100644
--- a/libibverbs/src/sysfs.c
+++ b/libibverbs/src/sysfs.c
@@ -29,7 +29,7 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-
+#define _GNU_SOURCE
 #if HAVE_CONFIG_H
 #  include <config.h>
 #endif /* HAVE_CONFIG_H */
diff --git a/libipathverbs/Makefile.am b/libipathverbs/Makefile.am
index 21cd83440b42..f9d4698c5e2c 100644
--- a/libipathverbs/Makefile.am
+++ b/libipathverbs/Makefile.am
@@ -33,7 +33,7 @@
 # combinations of this program with other software, or any other
 # product whatsoever.
 
-AM_CFLAGS = -g -Wall -D_GNU_SOURCE
+AM_CFLAGS = -g -Wall
 
 ipathverbs_version_script = @IPATHVERBS_VERSION_SCRIPT@
 
diff --git a/libmlx4/Makefile.am b/libmlx4/Makefile.am
index d11f40ad35e3..9bb90e964fe9 100644
--- a/libmlx4/Makefile.am
+++ b/libmlx4/Makefile.am
@@ -1,4 +1,4 @@
-AM_CFLAGS = -g -Wall -D_GNU_SOURCE
+AM_CFLAGS = -g -Wall
 
 mlx4_version_script = @MLX4_VERSION_SCRIPT@
 
diff --git a/libmlx4/src/dbrec.c b/libmlx4/src/dbrec.c
index 02ef237b3921..21ff93664df4 100644
--- a/libmlx4/src/dbrec.c
+++ b/libmlx4/src/dbrec.c
@@ -29,7 +29,7 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-
+#define _GNU_SOURCE
 #if HAVE_CONFIG_H
 #  include <config.h>
 #endif /* HAVE_CONFIG_H */
diff --git a/libmlx5/Makefile.am b/libmlx5/Makefile.am
index 39ca65d24028..345d5afbcfee 100644
--- a/libmlx5/Makefile.am
+++ b/libmlx5/Makefile.am
@@ -1,4 +1,4 @@
-AM_CFLAGS = -g -Wall -D_GNU_SOURCE
+AM_CFLAGS = -g -Wall
 ACLOCAL_AMFLAGS = -I m4
 
 mlx5_version_script = @MLX5_VERSION_SCRIPT@
diff --git a/libmlx5/src/dbrec.c b/libmlx5/src/dbrec.c
index 21fcd88e323a..dbc0e650b6f4 100644
--- a/libmlx5/src/dbrec.c
+++ b/libmlx5/src/dbrec.c
@@ -29,8 +29,7 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-
-
+#define _GNU_SOURCE
 #if HAVE_CONFIG_H
 #  include <config.h>
 #endif /* HAVE_CONFIG_H */
diff --git a/libmlx5/src/mlx5.c b/libmlx5/src/mlx5.c
index 5885f0b7b4aa..0d1d0fdb1111 100644
--- a/libmlx5/src/mlx5.c
+++ b/libmlx5/src/mlx5.c
@@ -29,8 +29,7 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-
-
+#define _GNU_SOURCE
 #if HAVE_CONFIG_H
 #  include <config.h>
 #endif /* HAVE_CONFIG_H */
diff --git a/libmthca/Makefile.am b/libmthca/Makefile.am
index e9be461693cd..1dd9a7ba8601 100644
--- a/libmthca/Makefile.am
+++ b/libmthca/Makefile.am
@@ -1,4 +1,4 @@
-AM_CFLAGS = -g -Wall -D_GNU_SOURCE
+AM_CFLAGS = -g -Wall
 
 mthca_version_script = @MTHCA_VERSION_SCRIPT@
 
diff --git a/libmthca/src/memfree.c b/libmthca/src/memfree.c
index 53f01fcd6369..87de4c0f5899 100644
--- a/libmthca/src/memfree.c
+++ b/libmthca/src/memfree.c
@@ -29,7 +29,7 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-
+#define _GNU_SOURCE
 #if HAVE_CONFIG_H
 #  include <config.h>
 #endif /* HAVE_CONFIG_H */
diff --git a/libnes/Makefile.am b/libnes/Makefile.am
index a5fdfc27517e..ade8e8b1eee8 100644
--- a/libnes/Makefile.am
+++ b/libnes/Makefile.am
@@ -1,7 +1,7 @@
 
 lib_LTLIBRARIES = src/libnes.la
 
-AM_CFLAGS = -g -Wall -D_GNU_SOURCE
+AM_CFLAGS = -g -Wall
 
 if HAVE_LD_VERSION_SCRIPT
     nes_version_script = -Wl,--version-script=$(srcdir)/src/nes.map
diff --git a/libocrdma/Makefile.am b/libocrdma/Makefile.am
index c07f38c0bc65..9b786e2b51a4 100644
--- a/libocrdma/Makefile.am
+++ b/libocrdma/Makefile.am
@@ -1,7 +1,7 @@
 
 lib_LTLIBRARIES = src/libocrdma.la
 
-AM_CFLAGS = -Wall -D_GNU_SOURCE
+AM_CFLAGS = -Wall
 
 if HAVE_LD_VERSION_SCRIPT
     ocrdma_version_script = -Wl,--version-script=$(srcdir)/src/ocrdma.map
diff --git a/librdmacm/Makefile.am b/librdmacm/Makefile.am
index bf721345f68d..84af01a14b4b 100644
--- a/librdmacm/Makefile.am
+++ b/librdmacm/Makefile.am
@@ -5,7 +5,7 @@ lib_LTLIBRARIES = src/librdmacm.la
 rslib_LTLIBRARIES = src/librspreload.la
 
 ACLOCAL_AMFLAGS = -I config
-AM_CFLAGS = -g -Wall -D_GNU_SOURCE -DSYSCONFDIR=\"$(sysconfdir)\" -DRDMADIR=\"@rdmadir@\"
+AM_CFLAGS = -g -Wall -DSYSCONFDIR=\"$(sysconfdir)\" -DRDMADIR=\"@rdmadir@\"
 
 src_librdmacm_la_CFLAGS = $(AM_CFLAGS)
 src_librspreload_la_CFLAGS = $(AM_CFLAGS)
diff --git a/librdmacm/examples/rping.c b/librdmacm/examples/rping.c
index a5aa8c5f38d6..2f6de08dbdf9 100644
--- a/librdmacm/examples/rping.c
+++ b/librdmacm/examples/rping.c
@@ -30,7 +30,7 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-
+#define _GNU_SOURCE
 #include <getopt.h>
 #include <stdlib.h>
 #include <string.h>
diff --git a/librdmacm/src/preload.c b/librdmacm/src/preload.c
index 3a0bc4c8c71d..a0bc1cfbd7a5 100644
--- a/librdmacm/src/preload.c
+++ b/librdmacm/src/preload.c
@@ -30,7 +30,7 @@
  * SOFTWARE.
  *
  */
-
+#define _GNU_SOURCE
 #if HAVE_CONFIG_H
 #  include <config.h>
 #endif /* HAVE_CONFIG_H */
diff --git a/librdmacm/src/rsocket.c b/librdmacm/src/rsocket.c
index c4f1b57dbb9a..818505fbe02e 100644
--- a/librdmacm/src/rsocket.c
+++ b/librdmacm/src/rsocket.c
@@ -30,7 +30,7 @@
  * SOFTWARE.
  *
  */
-
+#define _GNU_SOURCE
 #if HAVE_CONFIG_H
 #  include <config.h>
 #endif /* HAVE_CONFIG_H */
diff --git a/librxe/Makefile.am b/librxe/Makefile.am
index fe6c36c4eaba..972ae3b390b7 100644
--- a/librxe/Makefile.am
+++ b/librxe/Makefile.am
@@ -1,4 +1,4 @@
-AM_CFLAGS = -g -Wall -D_GNU_SOURCE
+AM_CFLAGS = -g -Wall
   
 rxe_version_script = @RXE_VERSION_SCRIPT@
 
-- 
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 03/28] Be explicit about _GNU_SOURCE

[Hal Rosenstock]

On 9/5/2016 5:07 PM, Jason Gunthorpe wrote:
> The recommended way to use this macro is at the top of the source file,
> avoid globally setting it via 'gcc -D' as few source files actually
> need it. In this tree we only need it in 17 out of 83 sources.
> 
> _GNU_SOURCE changes the behaviour of a few select calls away from the C99
> standard and should generally be minimized.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> ---

>  libibumad/src/sysfs.c               | 3 ---

For libibumad/sysfs.c part:
Thanks. Applied.

Reviewed-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@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

RE: [PATCH 03/28] Be explicit about _GNU_SOURCE

[Steve Wise]

> The recommended way to use this macro is at the top of the source file,
> avoid globally setting it via 'gcc -D' as few source files actually
> need it. In this tree we only need it in 17 out of 83 sources.
> 
> _GNU_SOURCE changes the behaviour of a few select calls away from the C99
> standard and should generally be minimized.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> ---


>  libcxgb3/Makefile.am                | 2 +-
>  libcxgb4/Makefile.am                | 2 +-

Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>

libcxgb* patches applied.

Thanks,

Steve.


--
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 03/28] Be explicit about _GNU_SOURCE

[Hefty, Sean]

> The recommended way to use this macro is at the top of the source file,
> avoid globally setting it via 'gcc -D' as few source files actually
> need it. In this tree we only need it in 17 out of 83 sources.
> 
> _GNU_SOURCE changes the behaviour of a few select calls away from the
> C99
> standard and should generally be minimized.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

For my parts

Acked-by: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@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

Re: [PATCH 03/28] Be explicit about _GNU_SOURCE

[Yishai Hadas]

On 9/6/2016 12:07 AM, Jason Gunthorpe wrote:
> The recommended way to use this macro is at the top of the source file,
> avoid globally setting it via 'gcc -D' as few source files actually
> need it. In this tree we only need it in 17 out of 83 sources.
>
> _GNU_SOURCE changes the behaviour of a few select calls away from the C99
> standard and should generally be minimized.
>

I'm fine with that, will take the relevant parts into libmlx4/libmlx5.

--
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 04/28] cxgb3/4: Display correct version number in error message

[Jason Gunthorpe]

Copy&Paste error was showing the major, not minor version.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libcxgb3/src/iwch.c | 2 +-
 libcxgb4/src/dev.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libcxgb3/src/iwch.c b/libcxgb3/src/iwch.c
index cc6b37cad775..32447e427146 100644
--- a/libcxgb3/src/iwch.c
+++ b/libcxgb3/src/iwch.c
@@ -228,7 +228,7 @@ found:
 	if (fw_min < FW_MIN) {
 		PDBG("libcxgb3: non-fatal firmware version mismatch.  "
 			"Firmware minor number is %u and libcxgb3 needs %u.\n",
-			fw_maj, FW_MAJ);	
+			fw_min, FW_MIN);
 		fflush(stderr);
 	}
 
diff --git a/libcxgb4/src/dev.c b/libcxgb4/src/dev.c
index 657ed192764c..cd3217af77b2 100644
--- a/libcxgb4/src/dev.c
+++ b/libcxgb4/src/dev.c
@@ -448,7 +448,7 @@ found:
 	if (fw_min < FW_MIN) {
 		PDBG("libcxgb4: non-fatal firmware version mismatch.  "
 			"Firmware minor number is %u and libcxgb4 needs %u.\n",
-			fw_maj, FW_MAJ);
+			fw_min, FW_MIN);
 		fflush(stderr);
 	}
 
-- 
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 04/28] cxgb3/4: Display correct version number in error message

[Steve Wise]

> Copy&Paste error was showing the major, not minor version.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> ---
>  libcxgb3/src/iwch.c | 2 +-
>  libcxgb4/src/dev.c  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>

Patches Applied.

Thanks,

Steve.



--
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 05/28] hfi/ipath: Use the name of the provider for the .driver file

[Jason Gunthorpe]

The standard is to use the same name for the library and .driver
file. The library is called hfi1verbs/ipathverbs so should the .driver,
add the verbs suffix.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libhfi1verbs/Makefile.am                          | 4 ++--
 libhfi1verbs/{hfi1.driver => hfi1verbs.driver}    | 0
 libipathverbs/Makefile.am                         | 4 ++--
 libipathverbs/{ipath.driver => ipathverbs.driver} | 0
 4 files changed, 4 insertions(+), 4 deletions(-)
 rename libhfi1verbs/{hfi1.driver => hfi1verbs.driver} (100%)
 rename libipathverbs/{ipath.driver => ipathverbs.driver} (100%)

diff --git a/libhfi1verbs/Makefile.am b/libhfi1verbs/Makefile.am
index 2c690b0570fb..d86ea3dd57e6 100644
--- a/libhfi1verbs/Makefile.am
+++ b/libhfi1verbs/Makefile.am
@@ -63,7 +63,7 @@ if HAVE_IBV_DEVICE_LIBRARY_EXTENSION
     src_libhfi1verbs_la_LDFLAGS = -avoid-version -release @IBV_DEVICE_LIBRARY_EXTENSION@ \
         $(hfiverbs_version_script)
     hfiverbsconfdir = $(sysconfdir)/libibverbs.d
-    hfiverbsconf_DATA = hfi1.driver
+    hfiverbsconf_DATA = hfi1verbs.driver
 else
     hfiverbslibdir = $(libdir)/infiniband
     hfiverbslib_LTLIBRARIES = src/hfiverbs.la
@@ -77,7 +77,7 @@ EXTRA_DIST = src/hfiverbs.h \
     config \
     autom4te.cache \
     libhfi1verbs.spec.in \
-    hfi1.driver
+    hfi1verbs.driver
 
 dist-hook: libhfi1verbs.spec
 	cp libhfi1verbs.spec $(distdir)
diff --git a/libhfi1verbs/hfi1.driver b/libhfi1verbs/hfi1verbs.driver
similarity index 100%
rename from libhfi1verbs/hfi1.driver
rename to libhfi1verbs/hfi1verbs.driver
diff --git a/libipathverbs/Makefile.am b/libipathverbs/Makefile.am
index f9d4698c5e2c..4d3e9258d989 100644
--- a/libipathverbs/Makefile.am
+++ b/libipathverbs/Makefile.am
@@ -45,7 +45,7 @@ if HAVE_IBV_DEVICE_LIBRARY_EXTENSION
     src_libipathverbs_la_LDFLAGS = -avoid-version -release @IBV_DEVICE_LIBRARY_EXTENSION@ \
         $(ipathverbs_version_script)
     ipathverbsconfdir = $(sysconfdir)/libibverbs.d
-    ipathverbsconf_DATA = ipath.driver
+    ipathverbsconf_DATA = ipathverbs.driver
 else
     ipathverbslibdir = $(libdir)/infiniband
     ipathverbslib_LTLIBRARIES = src/ipathverbs.la
@@ -59,7 +59,7 @@ EXTRA_DIST = src/ipathverbs.h \
     src/ipath-abi.h \
     src/ipathverbs.map \
     libipathverbs.spec.in \
-    ipath.driver \
+    ipathverbs.driver \
     truescale-serdes.cmds \
     truescale.conf
 
diff --git a/libipathverbs/ipath.driver b/libipathverbs/ipathverbs.driver
similarity index 100%
rename from libipathverbs/ipath.driver
rename to libipathverbs/ipathverbs.driver
-- 
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 06/28] i40iw: Avoid gcc warning -Wint-to-pointer-cast

[Jason Gunthorpe]

A u64 value needs to be casted to uintptr_t before being converted
back into a pointer, otherwise gcc produces warning on a 32 bit
build.

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

diff --git a/libi40iw/src/i40iw_uverbs.c b/libi40iw/src/i40iw_uverbs.c
index da23e4c9f413..db69b66c1e90 100644
--- a/libi40iw/src/i40iw_uverbs.c
+++ b/libi40iw/src/i40iw_uverbs.c
@@ -858,7 +858,7 @@ int i40iw_upost_send(struct ibv_qp *ib_qp, struct ibv_send_wr *ib_wr, struct ibv
 				info.op_type = I40IW_OP_TYPE_SEND;
 
 			if (ib_wr->send_flags & IBV_SEND_INLINE) {
-				info.op.inline_send.data = (void *)ib_wr->sg_list[0].addr;
+			  info.op.inline_send.data = (void *)(uintptr_t)ib_wr->sg_list[0].addr;
 				info.op.inline_send.len = ib_wr->sg_list[0].length;
 				ret = iwuqp->qp.ops.iw_inline_send(&iwuqp->qp, &info,
 								   ib_wr->wr.rdma.rkey, false);
@@ -881,7 +881,7 @@ int i40iw_upost_send(struct ibv_qp *ib_qp, struct ibv_send_wr *ib_wr, struct ibv
 			info.op_type = I40IW_OP_TYPE_RDMA_WRITE;
 
 			if (ib_wr->send_flags & IBV_SEND_INLINE) {
-				info.op.inline_rdma_write.data = (void *)ib_wr->sg_list[0].addr;
+			  info.op.inline_rdma_write.data = (void *)(uintptr_t)ib_wr->sg_list[0].addr;
 				info.op.inline_rdma_write.len = ib_wr->sg_list[0].length;
 				info.op.inline_rdma_write.rem_addr.tag_off = ib_wr->wr.rdma.remote_addr;
 				info.op.inline_rdma_write.rem_addr.len = ib_wr->sg_list->length;
-- 
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 07/28] iwpm: Add AM_INIT_AUTOMAKE([subdir-objects])

[Jason Gunthorpe]

Otherwise it doesn't build on modern distributions.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 iwpm/configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/iwpm/configure.ac b/iwpm/configure.ac
index 989452a6295c..5d5388ba2ed9 100644
--- a/iwpm/configure.ac
+++ b/iwpm/configure.ac
@@ -7,7 +7,7 @@ AC_CONFIG_SRCDIR([src/iwarp_pm_server.c])
 AC_CONFIG_HEADERS([config.h])
 AC_CONFIG_AUX_DIR(config)
 AC_CONFIG_MACRO_DIR([m4])
-AM_INIT_AUTOMAKE(-Wall -Werror)
+AM_INIT_AUTOMAKE([-Wall -Werror subdir-objects])
 
 # Checks for programs.
 AC_PROG_CC
-- 
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 08/28] ibcm: Actually use the version script when linking

[Jason Gunthorpe]

There is a typo in the Makefile.am that resulted in the version
script being ignored.

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

diff --git a/libibcm/Makefile.am b/libibcm/Makefile.am
index e4be9f887b1c..ff0b1ded12a3 100644
--- a/libibcm/Makefile.am
+++ b/libibcm/Makefile.am
@@ -8,9 +8,9 @@ AM_CFLAGS = -g -Wall
 src_libibcm_la_CFLAGS = $(AM_CFLAGS)
 
 if HAVE_LD_VERSION_SCRIPT
-    ibcm_version_script = -Wl,--version-script=$(srcdir)/src/libibcm.map
+    libibcm_version_script = -Wl,--version-script=$(srcdir)/src/libibcm.map
 else
-    ibcm_version_script =
+    libibcm_version_script =
 endif
 
 src_libibcm_la_SOURCES = src/cm.c
-- 
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 09/28] mlx5: Fix gcc 6.4 uninitialized variable warning

[Jason Gunthorpe]

gcc 6.4 remarks:

../providers/mlx5/cq.c:647:10: warning: 'wc_byte_len' may be used uninitialized in this function [-Wmaybe-uninitialized]
    err = mlx5_copy_to_send_wqe(mqp, wqe_ctr, cqe,
           lazy ? wc_byte_len : wc->byte_len);

The path is because handle_good_req_lazy does not set its wc_byte_len
outputs under certain case conditions.

Perhaps it is possible that the hardware never produces a completion
with opcodes and scatter flags that could trigger this, but gcc is not
wrong, so zero the field rather than hide the warning.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libmlx5/src/cq.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libmlx5/src/cq.c b/libmlx5/src/cq.c
index 88097037eea0..163205bddeb8 100644
--- a/libmlx5/src/cq.c
+++ b/libmlx5/src/cq.c
@@ -235,7 +235,10 @@ static inline void handle_good_req_lazy(struct mlx5_cqe64 *cqe, uint32_t *pwc_by
 		break;
 	case MLX5_OPCODE_UMR:
 		*umr_opcode = wq->wr_data[idx];
+		*pwc_byte_len = 0;
 		break;
+	default:
+		*pwc_byte_len = 0;
 	}
 }
 
-- 
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 09/28] mlx5: Fix gcc 6.4 uninitialized variable warning

[Yishai Hadas]

On 9/6/2016 12:07 AM, Jason Gunthorpe wrote:
> gcc 6.4 remarks:
>
> ../providers/mlx5/cq.c:647:10: warning: 'wc_byte_len' may be used uninitialized in this function [-Wmaybe-uninitialized]
>     err = mlx5_copy_to_send_wqe(mqp, wqe_ctr, cqe,
>            lazy ? wc_byte_len : wc->byte_len);
> The path is because handle_good_req_lazy does not set its wc_byte_len
> outputs under certain case conditions.

wc_byte_len defined as uint32_t uninitialized_var(wc_byte_len) to 
prevent this warning, isn't this macro applicable ?

> Perhaps it is possible that the hardware never produces a completion
> with opcodes and scatter flags that could trigger this,
It can't happen, no real functional issue.

> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> ---
>  libmlx5/src/cq.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/libmlx5/src/cq.c b/libmlx5/src/cq.c
> index 88097037eea0..163205bddeb8 100644
> --- a/libmlx5/src/cq.c
> +++ b/libmlx5/src/cq.c
> @@ -235,7 +235,10 @@ static inline void handle_good_req_lazy(struct mlx5_cqe64 *cqe, uint32_t *pwc_by
>  		break;
>  	case MLX5_OPCODE_UMR:
>  		*umr_opcode = wq->wr_data[idx];
> +		*pwc_byte_len = 0;

This case is a data path flow, need to prevent adding non applicable code.

>  		break;
> +	default:
> +		*pwc_byte_len = 0;
>  	}
>  }
>
>

--
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 09/28] mlx5: Fix gcc 6.4 uninitialized variable warning

[Jason Gunthorpe]

On Wed, Sep 14, 2016 at 06:39:10PM +0300, Yishai Hadas wrote:
> On 9/6/2016 12:07 AM, Jason Gunthorpe wrote:
> >gcc 6.4 remarks:
> >
> >../providers/mlx5/cq.c:647:10: warning: 'wc_byte_len' may be used uninitialized in this function [-Wmaybe-uninitialized]
> >    err = mlx5_copy_to_send_wqe(mqp, wqe_ctr, cqe,
> >           lazy ? wc_byte_len : wc->byte_len);
> >The path is because handle_good_req_lazy does not set its wc_byte_len
> >outputs under certain case conditions.
> 
> wc_byte_len defined as uint32_t uninitialized_var(wc_byte_len) to prevent
> this warning, isn't this macro applicable ?

The macro is not compatible with clang so I've run builds without
it. Turns out we generally don't need it anymore. I'd like to get rid
of it.

I view clang support as very important, since it found a different set
of mistakes :(

Further, it is not approriate to use the macro in a case where the
compiler *is not wrong*

> >Perhaps it is possible that the hardware never produces a completion
> >with opcodes and scatter flags that could trigger this,

> It can't happen, no real functional issue.

Okay, great, in that case you can gain more speed by writing code the
compiler can understand. In this instance we don't need to test for
(cqe64->op_own & MLX5_INLINE_SCATTER_XX) after we have disproven it by
checking for !(MLX5_OPCODE_RDMA_READ,MLX5_OPCODE_ATOMIC_CS,MLX5_OPCODE_ATOMIC_FA)

Here:

>From ac039248746318adfa41933d45495dc74a8eddb6 Mon Sep 17 00:00:00 2001
From: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Date: Fri, 2 Sep 2016 12:30:15 -0600
Subject: [PATCH] mlx5: Fix gcc 6.4 uninitialized variable warning

gcc 6.4 remarks:

../providers/mlx5/cq.c:647:10: warning: 'wc_byte_len' may be used uninitialized in this function [-Wmaybe-uninitialized]
    err = mlx5_copy_to_send_wqe(mqp, wqe_ctr, cqe,
           lazy ? wc_byte_len : wc->byte_len);

The path is because handle_good_req_lazy does not set its wc_byte_len
outputs under certain case conditions.

Yishai says it is not possible for the hardware to produce a completion
that follows this path, so restructure the logic to avoid even looking at
the scatter bits if we already determined they cannot be present.

This avoids the warnings and eliminates some branches on a hot path.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libmlx5/src/cq.c | 59 ++++++++++++++++++++++++++++++--------------------------
 1 file changed, 32 insertions(+), 27 deletions(-)

diff --git a/libmlx5/src/cq.c b/libmlx5/src/cq.c
index 88097037eea0..91c568e2f4b6 100644
--- a/libmlx5/src/cq.c
+++ b/libmlx5/src/cq.c
@@ -222,23 +222,6 @@ static inline void handle_good_req(struct ibv_wc *wc, struct mlx5_cqe64 *cqe, st
 	}
 }
 
-static inline void handle_good_req_lazy(struct mlx5_cqe64 *cqe, uint32_t *pwc_byte_len,
-					int *umr_opcode, struct mlx5_wq *wq, int idx)
-{
-	switch (ntohl(cqe->sop_drop_qpn) >> 24) {
-	case MLX5_OPCODE_RDMA_READ:
-		*pwc_byte_len  = ntohl(cqe->byte_cnt);
-		break;
-	case MLX5_OPCODE_ATOMIC_CS:
-	case MLX5_OPCODE_ATOMIC_FA:
-		*pwc_byte_len  = 8;
-		break;
-	case MLX5_OPCODE_UMR:
-		*umr_opcode = wq->wr_data[idx];
-		break;
-	}
-}
-
 static inline int handle_responder_lazy(struct mlx5_cq *cq, struct mlx5_cqe64 *cqe,
 					struct mlx5_qp *qp, struct mlx5_srq *srq)
 {
@@ -634,7 +617,6 @@ static inline int mlx5_parse_cqe(struct mlx5_cq *cq,
 	switch (opcode) {
 	case MLX5_CQE_REQ:
 	{
-		uint32_t uninitialized_var(wc_byte_len);
 		mqp = get_req_context(mctx, cur_rsc,
 				      (cqe_ver ? (ntohl(cqe64->srqn_uidx) & 0xffffff) : qpn),
 				      cqe_ver);
@@ -643,17 +625,40 @@ static inline int mlx5_parse_cqe(struct mlx5_cq *cq,
 		wq = &mqp->sq;
 		wqe_ctr = ntohs(cqe64->wqe_counter);
 		idx = wqe_ctr & (wq->wqe_cnt - 1);
-		if (lazy)
-			handle_good_req_lazy(cqe64, &wc_byte_len, &cq->umr_opcode, wq, idx);
-		else
+		if (lazy) {
+			uint32_t wc_byte_len;
+
+			switch (ntohl(cqe64->sop_drop_qpn) >> 24) {
+			case MLX5_OPCODE_UMR:
+				cq->umr_opcode = wq->wr_data[idx];
+				break;
+
+			case MLX5_OPCODE_RDMA_READ:
+				wc_byte_len = ntohl(cqe64->byte_cnt);
+				goto scatter_out;
+			case MLX5_OPCODE_ATOMIC_CS:
+			case MLX5_OPCODE_ATOMIC_FA:
+				wc_byte_len = 8;
+
+			scatter_out:
+				if (cqe64->op_own & MLX5_INLINE_SCATTER_32)
+					err = mlx5_copy_to_send_wqe(
+					    mqp, wqe_ctr, cqe, wc_byte_len);
+				else if (cqe64->op_own & MLX5_INLINE_SCATTER_64)
+					err = mlx5_copy_to_send_wqe(
+					    mqp, wqe_ctr, cqe - 1, wc_byte_len);
+				break;
+			}
+		} else {
 			handle_good_req(wc, cqe64, wq, idx);
 
-		if (cqe64->op_own & MLX5_INLINE_SCATTER_32)
-			err = mlx5_copy_to_send_wqe(mqp, wqe_ctr, cqe,
-						    lazy ? wc_byte_len : wc->byte_len);
-		else if (cqe64->op_own & MLX5_INLINE_SCATTER_64)
-			err = mlx5_copy_to_send_wqe(mqp, wqe_ctr, cqe - 1,
-						     lazy ? wc_byte_len : wc->byte_len);
+			if (cqe64->op_own & MLX5_INLINE_SCATTER_32)
+				err = mlx5_copy_to_send_wqe(mqp, wqe_ctr, cqe,
+							    wc->byte_len);
+			else if (cqe64->op_own & MLX5_INLINE_SCATTER_64)
+				err = mlx5_copy_to_send_wqe(
+				    mqp, wqe_ctr, cqe - 1, wc->byte_len);
+		}
 
 		if (lazy) {
 			cq->ibv_cq.wr_id = wq->wrid[idx];
-- 
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

Re: [PATCH 09/28] mlx5: Fix gcc 6.4 uninitialized variable warning

[Yishai Hadas]

On 9/14/2016 7:37 PM, Jason Gunthorpe wrote:

>
> Okay, great, in that case you can gain more speed by writing code the
> compiler can understand. In this instance we don't need to test for
> (cqe64->op_own & MLX5_INLINE_SCATTER_XX) after we have disproven it by
> checking for !(MLX5_OPCODE_RDMA_READ,MLX5_OPCODE_ATOMIC_CS,MLX5_OPCODE_ATOMIC_FA)

I'm fine with that approach, still can improve your patch see below.

>  1 file changed, 32 insertions(+), 27 deletions(-)
>
> diff --git a/libmlx5/src/cq.c b/libmlx5/src/cq.c
> index 88097037eea0..91c568e2f4b6 100644
> --- a/libmlx5/src/cq.c
> +++ b/libmlx5/src/cq.c
> @@ -222,23 +222,6 @@ static inline void handle_good_req(struct ibv_wc *wc, struct mlx5_cqe64 *cqe, st
>  	}
>  }
>
> -static inline void handle_good_req_lazy(struct mlx5_cqe64 *cqe, uint32_t *pwc_byte_len,
> -					int *umr_opcode, struct mlx5_wq *wq, int idx)
> -{
> -	switch (ntohl(cqe->sop_drop_qpn) >> 24) {
> -	case MLX5_OPCODE_RDMA_READ:
> -		*pwc_byte_len  = ntohl(cqe->byte_cnt);
> -		break;
> -	case MLX5_OPCODE_ATOMIC_CS:
> -	case MLX5_OPCODE_ATOMIC_FA:
> -		*pwc_byte_len  = 8;
> -		break;
> -	case MLX5_OPCODE_UMR:
> -		*umr_opcode = wq->wr_data[idx];
> -		break;
> -	}
> -}
> -
>  static inline int handle_responder_lazy(struct mlx5_cq *cq, struct mlx5_cqe64 *cqe,
>  					struct mlx5_qp *qp, struct mlx5_srq *srq)
>  {
> @@ -634,7 +617,6 @@ static inline int mlx5_parse_cqe(struct mlx5_cq *cq,
>  	switch (opcode) {
>  	case MLX5_CQE_REQ:
>  	{
> -		uint32_t uninitialized_var(wc_byte_len);
>  		mqp = get_req_context(mctx, cur_rsc,
>  				      (cqe_ver ? (ntohl(cqe64->srqn_uidx) & 0xffffff) : qpn),
>  				      cqe_ver);
> @@ -643,17 +625,40 @@ static inline int mlx5_parse_cqe(struct mlx5_cq *cq,
>  		wq = &mqp->sq;
>  		wqe_ctr = ntohs(cqe64->wqe_counter);
>  		idx = wqe_ctr & (wq->wqe_cnt - 1);
> -		if (lazy)
> -			handle_good_req_lazy(cqe64, &wc_byte_len, &cq->umr_opcode, wq, idx);
> -		else
> +		if (lazy) {
> +			uint32_t wc_byte_len;
> +
> +			switch (ntohl(cqe64->sop_drop_qpn) >> 24) {
> +			case MLX5_OPCODE_UMR:
> +				cq->umr_opcode = wq->wr_data[idx];
> +				break;
> +
> +			case MLX5_OPCODE_RDMA_READ:
> +				wc_byte_len = ntohl(cqe64->byte_cnt);
> +				goto scatter_out;
> +			case MLX5_OPCODE_ATOMIC_CS:
> +			case MLX5_OPCODE_ATOMIC_FA:
> +				wc_byte_len = 8;
> +
> +			scatter_out:
> +				if (cqe64->op_own & MLX5_INLINE_SCATTER_32)
> +					err = mlx5_copy_to_send_wqe(
> +					    mqp, wqe_ctr, cqe, wc_byte_len);
> +				else if (cqe64->op_own & MLX5_INLINE_SCATTER_64)
> +					err = mlx5_copy_to_send_wqe(
> +					    mqp, wqe_ctr, cqe - 1, wc_byte_len);
> +				break;
> +			}
> +		} else {
>  			handle_good_req(wc, cqe64, wq, idx);
>
> -		if (cqe64->op_own & MLX5_INLINE_SCATTER_32)
> -			err = mlx5_copy_to_send_wqe(mqp, wqe_ctr, cqe,
> -						    lazy ? wc_byte_len : wc->byte_len);
> -		else if (cqe64->op_own & MLX5_INLINE_SCATTER_64)
> -			err = mlx5_copy_to_send_wqe(mqp, wqe_ctr, cqe - 1,
> -						     lazy ? wc_byte_len : wc->byte_len);
> +			if (cqe64->op_own & MLX5_INLINE_SCATTER_32)
> +				err = mlx5_copy_to_send_wqe(mqp, wqe_ctr, cqe,
> +							    wc->byte_len);
> +			else if (cqe64->op_own & MLX5_INLINE_SCATTER_64)
> +				err = mlx5_copy_to_send_wqe(
> +				    mqp, wqe_ctr, cqe - 1, wc->byte_len);
> +		}
>
>  		if (lazy) {
>  			cq->ibv_cq.wr_id = wq->wrid[idx];
>

This 'if' can be omitted and its body can be embedded above for both 
lazy/non-lazy modes.

Will handle that in our side and put into our regression system, once 
the patch will be approved will take it into my formal GIT so that you 
can take it from there.
--
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 09/28] mlx5: Fix gcc 6.4 uninitialized variable warning

[Jason Gunthorpe]

On Thu, Sep 15, 2016 at 05:26:33PM +0300, Yishai Hadas wrote:

> This 'if' can be omitted and its body can be embedded above for both
> lazy/non-lazy modes.

Sure

> Will handle that in our side and put into our regression system, once the
> patch will be approved will take it into my formal GIT so that you can take
> it from there.

Great on both patches

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

Re: [PATCH 09/28] mlx5: Fix gcc 6.4 uninitialized variable warning

[Yishai Hadas]

On 9/15/2016 7:21 PM, Jason Gunthorpe wrote:
> On Thu, Sep 15, 2016 at 05:26:33PM +0300, Yishai Hadas wrote:
>
>> This 'if' can be omitted and its body can be embedded above for both
>> lazy/non-lazy modes.
> Sure
>> Will handle that in our side and put into our regression system, once the
>> patch will be approved will take it into my formal GIT so that you can take
>> it from there.
>
> Great on both patches

Made the above change in addition to some changes in the commit logs and 
applied both.

The other patches for libmlx4/5 were applied as well, thanks.
--
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 10/28] nes: Fix clang 3.6 warning -Wtautological-constant-out-of-range-compare

[Jason Gunthorpe]

clang will miscompile this code because it make assumptions
about the signededness of enums.

Instead create a constant for an invalid ibv_wc_opcode and use that
everywhere. Unsigned enums will use -1, signed enums should use INT_MAX.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libnes/src/nes_uverbs.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/libnes/src/nes_uverbs.c b/libnes/src/nes_uverbs.c
index 12e6222313ad..80891d6243c7 100644
--- a/libnes/src/nes_uverbs.c
+++ b/libnes/src/nes_uverbs.c
@@ -471,13 +471,15 @@ int nes_ima_upoll_cq(struct ibv_cq *cq, int num_entries, struct ibv_wc *entry)
 	}
 
 	while (cqe_count < num_entries) {
-		entry->opcode = -1;
+		const enum ibv_wc_opcode INVAL_OP = -1;
+
+		entry->opcode = INVAL_OP;
 		cqe = &cqes[head];
 		cqe_misc =
 			le32_to_cpu(cqe->cqe_words[NES_NIC_CQE_MISC_IDX]);
 		if (cqe_misc & NES_NIC_CQE_VALID) {
 			memset(entry, 0, sizeof *entry);
-			entry->opcode = -1;
+			entry->opcode = INVAL_OP;
 			cqe->cqe_words[NES_NIC_CQE_MISC_IDX] = 0;
 			entry->status = (cqe_misc & NES_NIC_CQE_ERRV_MASK) >>
 						NES_NIC_CQE_ERRV_SHIFT;
@@ -523,7 +525,7 @@ int nes_ima_upoll_cq(struct ibv_cq *cq, int num_entries, struct ibv_wc *entry)
 			if (++head >= cq_size)
 				head = 0;
 
-			if (entry->opcode != -1) {
+			if (entry->opcode != INVAL_OP) {
 				/* it is possible that no entry will be
 				  available */
 				cqe_count++;
-- 
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 11/28] ocrdma: Fix incorrect type of ibwc_status

[Jason Gunthorpe]

clang 3.6 remarks:

../providers/ocrdma/ocrdma_verbs.c:1517:35: warning: implicit conversion from enumeration type 'enum ibv_wc_status' to different enumeration type 'enum ibv_wc_opcode' [-Wenum-conversion]
        enum ibv_wc_opcode ibwc_status = IBV_WC_GENERAL_ERR;

And it is not wrong, the enum should be ib_wc_status not opcode.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libocrdma/src/ocrdma_verbs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libocrdma/src/ocrdma_verbs.c b/libocrdma/src/ocrdma_verbs.c
index 5248e7e26c7b..b3c77c47d660 100644
--- a/libocrdma/src/ocrdma_verbs.c
+++ b/libocrdma/src/ocrdma_verbs.c
@@ -1539,7 +1539,7 @@ int ocrdma_post_recv(struct ibv_qp *ibqp, struct ibv_recv_wr *wr,
 
 static enum ibv_wc_status ocrdma_to_ibwc_err(uint16_t status)
 {
-	enum ibv_wc_opcode ibwc_status = IBV_WC_GENERAL_ERR;
+	enum ibv_wc_status ibwc_status = IBV_WC_GENERAL_ERR;
 	switch (status) {
 	case OCRDMA_CQE_GENERAL_ERR:
 		ibwc_status = IBV_WC_GENERAL_ERR;
-- 
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 12/28] ocrdma: Fix incorrect enum constant

[Jason Gunthorpe]

clang 3.6 remarks:

../providers/ocrdma/ocrdma_verbs.c:597:14: warning: implicit conversion from enumeration type 'enum ibv_qp_state' to different enumeration type 'enum ocrdma_qp_state' [-Wenum-conversion]
        qp->state = IBV_QPS_RESET;

And it is not wrong.  IBV_QPS_RESET=0 and OCRDMA_QPS_RST=0 so this
patch is a NOP.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libocrdma/src/ocrdma_verbs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libocrdma/src/ocrdma_verbs.c b/libocrdma/src/ocrdma_verbs.c
index b3c77c47d660..60626260656d 100644
--- a/libocrdma/src/ocrdma_verbs.c
+++ b/libocrdma/src/ocrdma_verbs.c
@@ -620,7 +620,7 @@ struct ibv_qp *ocrdma_create_qp(struct ibv_pd *pd,
 			qp->dpp_cq = 0;
 		}
 	}
-	qp->state = IBV_QPS_RESET;
+	qp->state = OCRDMA_QPS_RST;
 	INIT_DBLY_LIST_NODE(&qp->sq_entry);
 	INIT_DBLY_LIST_NODE(&qp->rq_entry);
 	return &qp->ibv_qp;
-- 
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 13/28] rdmacm: Use correct format specifier for size_t

[Jason Gunthorpe]

clang 3.6 remarks:

../librdmacm/examples/rping.c:1203:30: warning: invalid conversion specifier 'Z' [-Wformat-invalid-specifier]
                                       "(valid range is %Zd to %d)\n",

And indeed, the correct specifier for size_t is '%zd'

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

diff --git a/librdmacm/examples/rping.c b/librdmacm/examples/rping.c
index 2f6de08dbdf9..88ce7f96c7e8 100644
--- a/librdmacm/examples/rping.c
+++ b/librdmacm/examples/rping.c
@@ -1200,7 +1200,7 @@ int main(int argc, char *argv[])
 			if ((cb->size < RPING_MIN_BUFSIZE) ||
 			    (cb->size > (RPING_BUFSIZE - 1))) {
 				fprintf(stderr, "Invalid size %d "
-				       "(valid range is %Zd to %d)\n",
+				       "(valid range is %zd to %d)\n",
 				       cb->size, RPING_MIN_BUFSIZE, RPING_BUFSIZE);
 				ret = EINVAL;
 			} else
-- 
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 14/28] rdmacm: Control symbol export from librspreload

[Jason Gunthorpe]

Since librspreload is a LD_PRELOAD library it should only export
symbols it intends to override. The following internal symbols were
leaking out:

 getenv_options
 idm_clear
 idm_set
 idx_insert
 idx_remove
 idx_replace
 set_rsocket_options

The simplest way to fix this is with a map file.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 librdmacm/Makefile.am          |  5 ++++-
 librdmacm/src/librspreload.map | 33 +++++++++++++++++++++++++++++++++
 librdmacm/src/preload.c        |  4 ++--
 3 files changed, 39 insertions(+), 3 deletions(-)
 create mode 100644 librdmacm/src/librspreload.map

diff --git a/librdmacm/Makefile.am b/librdmacm/Makefile.am
index 84af01a14b4b..edfb36bb4149 100644
--- a/librdmacm/Makefile.am
+++ b/librdmacm/Makefile.am
@@ -23,7 +23,10 @@ src_librdmacm_la_LDFLAGS = -version-info 1 -export-dynamic \
 src_librdmacm_la_DEPENDENCIES =  $(srcdir)/src/librdmacm.map
 
 src_librspreload_la_SOURCES = src/preload.c src/indexer.c
-src_librspreload_la_LDFLAGS = -version-info 1 -export-dynamic
+src_librspreload_la_LDFLAGS = -version-info 1
+if HAVE_LD_VERSION_SCRIPT
+    src_librspreload_la_LDFLAGS += -Wl,--version-script=$(srcdir)/src/librspreload.map
+endif
 src_librspreload_la_LIBADD = $(top_builddir)/src/librdmacm.la
 
 bin_PROGRAMS = examples/ucmatose examples/rping examples/udaddy examples/mckey \
diff --git a/librdmacm/src/librspreload.map b/librdmacm/src/librspreload.map
new file mode 100644
index 000000000000..67ecf33b8203
--- /dev/null
+++ b/librdmacm/src/librspreload.map
@@ -0,0 +1,33 @@
+{
+        /* FIXME: It is probably not a great idea to not tag these with the
+	   proper symbol version from glibc, at least if glibc ever changes
+	   the signature this will go sideways.. */
+	global:
+		accept;
+		bind;
+		close;
+		connect;
+		dup2;
+		fcntl;
+		getpeername;
+		getsockname;
+		getsockopt;
+		listen;
+		poll;
+		read;
+		readv;
+		recv;
+		recvfrom;
+		recvmsg;
+		select;
+		send;
+		sendfile;
+		sendmsg;
+		sendto;
+		setsockopt;
+		shutdown;
+		socket;
+		write;
+		writev;
+	local: *;
+};
diff --git a/librdmacm/src/preload.c b/librdmacm/src/preload.c
index a0bc1cfbd7a5..2a90f79b226c 100644
--- a/librdmacm/src/preload.c
+++ b/librdmacm/src/preload.c
@@ -356,7 +356,7 @@ static enum fd_type fd_close(int index, int *fd)
 	return type;
 }
 
-void getenv_options(void)
+static void getenv_options(void)
 {
 	char *var;
 
@@ -521,7 +521,7 @@ err:
 /*
  * Use defaults on failure.
  */
-void set_rsocket_options(int rsocket)
+static void set_rsocket_options(int rsocket)
 {
 	if (sq_size)
 		rsetsockopt(rsocket, SOL_RDMA, RDMA_SQSIZE, &sq_size, sizeof sq_size);
-- 
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 14/28] rdmacm: Control symbol export from librspreload

[Hefty, Sean]

> Since librspreload is a LD_PRELOAD library it should only export
> symbols it intends to override. The following internal symbols were
> leaking out:
> 
>  getenv_options
>  idm_clear
>  idm_set
>  idx_insert
>  idx_remove
>  idx_replace
>  set_rsocket_options
> 
> The simplest way to fix this is with a map file.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

Acked-by: Sean Hefty <sean.hefty-z/GqZRLIEsXQT0dZR+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

[PATCH 15/28] umad: Include umad.h in the canonical way

[Jason Gunthorpe]

Should always be:

#include <infiniband/umad.h>

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libibumad/src/umad.c               | 2 +-
 libibumad/tests/umad_reg2_compat.c | 2 +-
 libibumad/tests/umad_register2.c   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/libibumad/src/umad.c b/libibumad/src/umad.c
index 7b799f9512bb..a7879d385466 100644
--- a/libibumad/src/umad.c
+++ b/libibumad/src/umad.c
@@ -50,7 +50,7 @@
 #include <ctype.h>
 #include <inttypes.h>
 
-#include "umad.h"
+#include <infiniband/umad.h>
 
 #define IB_OPENIB_OUI                 (0x001405)
 
diff --git a/libibumad/tests/umad_reg2_compat.c b/libibumad/tests/umad_reg2_compat.c
index 4f433062b57d..413a3bec6d58 100644
--- a/libibumad/tests/umad_reg2_compat.c
+++ b/libibumad/tests/umad_reg2_compat.c
@@ -39,7 +39,7 @@
 #include <stdio.h>
 #include <inttypes.h>
 
-#include "umad.h"
+#include <infiniband/umad.h>
 
 #define UNLIKELY_MGMT_CLASS 0x2F
 #define UNLIKELY_RMPP_MGMT_CLASS 0x4F
diff --git a/libibumad/tests/umad_register2.c b/libibumad/tests/umad_register2.c
index 93692ee7a856..c2d7846038bd 100644
--- a/libibumad/tests/umad_register2.c
+++ b/libibumad/tests/umad_register2.c
@@ -41,7 +41,7 @@
 #include <errno.h>
 #include <sys/ioctl.h>
 
-#include "umad.h"
+#include <infiniband/umad.h>
 
 #define UNLIKELY_MGMT_CLASS 0x2F
 #define UNLIKELY_RMPP_MGMT_CLASS 0x4F
-- 
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 15/28] umad: Include umad.h in the canonical way

[Hal Rosenstock]

On 9/5/2016 5:08 PM, Jason Gunthorpe wrote:
> Should always be:
> 
> #include <infiniband/umad.h>
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

Thanks. Applied.

Reviewed-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@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

[PATCH 16/28] umad: Fix incorrect arguments to umad_register2 in tests

[Jason Gunthorpe]

The code was attempting to pass int* instead of uint32_t* to
umad_register2, which is not OK.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libibumad/tests/umad_reg2_compat.c | 2 +-
 libibumad/tests/umad_register2.c   | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/libibumad/tests/umad_reg2_compat.c b/libibumad/tests/umad_reg2_compat.c
index 413a3bec6d58..6dd4a48a59b2 100644
--- a/libibumad/tests/umad_reg2_compat.c
+++ b/libibumad/tests/umad_reg2_compat.c
@@ -154,7 +154,7 @@ void test_fall_back(void)
 {
 	int rc = 0;
 	struct umad_reg_attr reg_attr;
-	int agent_id;
+	uint32_t agent_id;
 	int fd;
 
 	fd = open_test_device();
diff --git a/libibumad/tests/umad_register2.c b/libibumad/tests/umad_register2.c
index c2d7846038bd..ed7e816bb63c 100644
--- a/libibumad/tests/umad_register2.c
+++ b/libibumad/tests/umad_register2.c
@@ -92,8 +92,8 @@ void test_fail(void)
 {
 	int rc = 0;
 	struct umad_reg_attr reg_attr;
-	int agent_id;
-	int agent_id2;
+	uint32_t agent_id;
+	uint32_t agent_id2;
 	int fd;
 
 	printf("\n *****\nBegin invalid tests\n");
@@ -200,7 +200,7 @@ void test_oui(void)
 {
 	int rc = 0;
 	struct umad_reg_attr reg_attr;
-	int agent_id;
+	uint32_t agent_id;
 	int fd;
 
 	printf("\n *****\nStart valid oui tests\n");
-- 
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 16/28] umad: Fix incorrect arguments to umad_register2 in tests

[Hal Rosenstock]

On 9/5/2016 5:08 PM, Jason Gunthorpe wrote:
> The code was attempting to pass int* instead of uint32_t* to
> umad_register2, which is not OK.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

Thanks. Applied.

Reviewed-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@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

[PATCH 17/28] verbs: Fix incorrect type of len

[Jason Gunthorpe]

gcc remarks:

../librdmacm/examples/udpong.c:301:11: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
   if (len < 0)

len is set by:
		len = svr_recv(&msg, sizeof msg, &addr, &addrlen);

And svr_recv returns:

static ssize_t svr_recv(struct message *msg, size_t size,

So clearly len is the wrong type, and the error test does
not work.

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

diff --git a/librdmacm/examples/udpong.c b/librdmacm/examples/udpong.c
index 97713a297cf9..525ee82b9620 100644
--- a/librdmacm/examples/udpong.c
+++ b/librdmacm/examples/udpong.c
@@ -291,7 +291,7 @@ out:
 
 static int svr_run(void)
 {
-	size_t len;
+	ssize_t len;
 	int ret;
 
 	ret = svr_bind();
-- 
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 18/28] verbs: Fix clang 3.6 warning -Wtautological-compare

[Jason Gunthorpe]

The signededness of a 'enum X' is undefined in the C standard,
compilers are free to use any type they like. So, coercing -1 into
an enum and expecting '< 0' to work is undefined behaviour, and as
the warning shows at least clang miscompiles this code.

Instead use 0 to indicate undefined MTU from pp_mtu_to_enum,
0 is unused in the mtu enum.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libibverbs/examples/pingpong.c      | 2 +-
 libibverbs/examples/rc_pingpong.c   | 2 +-
 libibverbs/examples/srq_pingpong.c  | 2 +-
 libibverbs/examples/uc_pingpong.c   | 2 +-
 libibverbs/examples/xsrq_pingpong.c | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/libibverbs/examples/pingpong.c b/libibverbs/examples/pingpong.c
index 2fe4a04115fb..f6a50e9c62aa 100644
--- a/libibverbs/examples/pingpong.c
+++ b/libibverbs/examples/pingpong.c
@@ -44,7 +44,7 @@ enum ibv_mtu pp_mtu_to_enum(int mtu)
 	case 1024: return IBV_MTU_1024;
 	case 2048: return IBV_MTU_2048;
 	case 4096: return IBV_MTU_4096;
-	default:   return -1;
+	default:   return 0;
 	}
 }
 
diff --git a/libibverbs/examples/rc_pingpong.c b/libibverbs/examples/rc_pingpong.c
index 967678362833..1fad16a0be7c 100644
--- a/libibverbs/examples/rc_pingpong.c
+++ b/libibverbs/examples/rc_pingpong.c
@@ -768,7 +768,7 @@ int main(int argc, char *argv[])
 
 		case 'm':
 			mtu = pp_mtu_to_enum(strtol(optarg, NULL, 0));
-			if (mtu < 0) {
+			if (mtu == 0) {
 				usage(argv[0]);
 				return 1;
 			}
diff --git a/libibverbs/examples/srq_pingpong.c b/libibverbs/examples/srq_pingpong.c
index a1061c31972d..929b736545c7 100644
--- a/libibverbs/examples/srq_pingpong.c
+++ b/libibverbs/examples/srq_pingpong.c
@@ -697,7 +697,7 @@ int main(int argc, char *argv[])
 
 		case 'm':
 			mtu = pp_mtu_to_enum(strtol(optarg, NULL, 0));
-			if (mtu < 0) {
+			if (mtu == 0) {
 				usage(argv[0]);
 				return 1;
 			}
diff --git a/libibverbs/examples/uc_pingpong.c b/libibverbs/examples/uc_pingpong.c
index b25d16c79021..3802e3821773 100644
--- a/libibverbs/examples/uc_pingpong.c
+++ b/libibverbs/examples/uc_pingpong.c
@@ -604,7 +604,7 @@ int main(int argc, char *argv[])
 
 		case 'm':
 			mtu = pp_mtu_to_enum(strtol(optarg, NULL, 0));
-			if (mtu < 0) {
+			if (mtu == 0) {
 				usage(argv[0]);
 				return 1;
 			}
diff --git a/libibverbs/examples/xsrq_pingpong.c b/libibverbs/examples/xsrq_pingpong.c
index ff00180f2644..a7e345f3850c 100644
--- a/libibverbs/examples/xsrq_pingpong.c
+++ b/libibverbs/examples/xsrq_pingpong.c
@@ -906,7 +906,7 @@ int main(int argc, char *argv[])
 			break;
 		case 'm':
 			ctx.mtu = pp_mtu_to_enum(strtol(optarg, NULL, 0));
-			if (ctx.mtu < 0) {
+			if (ctx.mtu == 0) {
 				usage(argv[0]);
 				return 1;
 			}
-- 
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 19/28] verbs: Avoid gcc 6.1 warning -Wunused-variable

[Jason Gunthorpe]

gcc remarks:

../libibverbs/src/neigh.c:339:6: warning: 'sock_fd' may be used uninitialized in this function [-Wmaybe-uninitialized]
  err = try_send_to(sock_fd, buff, sizeof(buff), &addr_dst);

But this is bogus because create_socket will always return
an error if it does not set psock_fd. It looks like the
insane if logic is just a tish too much for gcc to handle.

Since the result of create_socket is discarded anyhow, simplify the
tortured logic.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libibverbs/src/neigh.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/libibverbs/src/neigh.c b/libibverbs/src/neigh.c
index 799b810a9ec4..dc8c2bc99ae3 100644
--- a/libibverbs/src/neigh.c
+++ b/libibverbs/src/neigh.c
@@ -207,7 +207,7 @@ static int create_socket(struct get_neigh_handler *neigh_handler,
 				    &addr_src.len);
 	if (err) {
 		errno = EADDRNOTAVAIL;
-		return err;
+		return -1;
 	}
 
 	addr_dst->len = sizeof(addr_dst->sktaddr);
@@ -216,24 +216,22 @@ static int create_socket(struct get_neigh_handler *neigh_handler,
 				    &addr_dst->len);
 	if (err) {
 		errno = EADDRNOTAVAIL;
-		return err;
+		return -1;
 	}
 
 	err = set_link_port(&addr_dst->sktaddr, PORT_DISCARD,
 			    neigh_handler->oif);
 	if (err)
-		return err;
+		return -1;
 
 	sock_fd = socket(addr_dst->sktaddr.s.sa_family,
 			 SOCK_DGRAM | SOCK_CLOEXEC, 0);
 	if (sock_fd == -1)
-		return errno ? -errno : -1;
+		return -1;
 	err = bind(sock_fd, &addr_src.sktaddr.s, addr_src.len);
 	if (err) {
-		int bind_err = -errno;
-
 		close(sock_fd);
-		return bind_err ?: EADDRNOTAVAIL;
+		return -1;
 	}
 
 	*psock_fd = sock_fd;
-- 
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 19/28] verbs: Avoid gcc 6.1 warning -Wunused-variable

[Yishai Hadas]

On 9/6/2016 12:08 AM, Jason Gunthorpe wrote:
> gcc remarks:
>
> ../libibverbs/src/neigh.c:339:6: warning: 'sock_fd' may be used uninitialized in this function [-Wmaybe-uninitialized]
>   err = try_send_to(sock_fd, buff, sizeof(buff), &addr_dst);


Why not just initializing 'sock_fd' to any value (e.g. to 0) when it's 
declared and drop all below patch ? as the code checks the return code 
from create_socket() it should be safe.

> But this is bogus because create_socket will always return
> an error if it does not set psock_fd. It looks like the
> insane if logic is just a tish too much for gcc to handle.
>
> Since the result of create_socket is discarded anyhow, simplify the
> tortured logic.
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> ---
>  libibverbs/src/neigh.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/libibverbs/src/neigh.c b/libibverbs/src/neigh.c
> index 799b810a9ec4..dc8c2bc99ae3 100644
> --- a/libibverbs/src/neigh.c
> +++ b/libibverbs/src/neigh.c
> @@ -207,7 +207,7 @@ static int create_socket(struct get_neigh_handler *neigh_handler,
>  				    &addr_src.len);
>  	if (err) {
>  		errno = EADDRNOTAVAIL;
> -		return err;
> +		return -1;
>  	}
>
>  	addr_dst->len = sizeof(addr_dst->sktaddr);
> @@ -216,24 +216,22 @@ static int create_socket(struct get_neigh_handler *neigh_handler,
>  				    &addr_dst->len);
>  	if (err) {
>  		errno = EADDRNOTAVAIL;
> -		return err;
> +		return -1;
>  	}
>
>  	err = set_link_port(&addr_dst->sktaddr, PORT_DISCARD,
>  			    neigh_handler->oif);
>  	if (err)
> -		return err;
> +		return -1;
>
>  	sock_fd = socket(addr_dst->sktaddr.s.sa_family,
>  			 SOCK_DGRAM | SOCK_CLOEXEC, 0);
>  	if (sock_fd == -1)
> -		return errno ? -errno : -1;
> +		return -1;
>  	err = bind(sock_fd, &addr_src.sktaddr.s, addr_src.len);
>  	if (err) {
> -		int bind_err = -errno;
> -
>  		close(sock_fd);
> -		return bind_err ?: EADDRNOTAVAIL;
> +		return -1;
>  	}
>
>  	*psock_fd = sock_fd;
>

--
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 19/28] verbs: Avoid gcc 6.1 warning -Wunused-variable

[Jason Gunthorpe]

On Wed, Sep 14, 2016 at 07:46:15PM +0300, Yishai Hadas wrote:
> On 9/6/2016 12:08 AM, Jason Gunthorpe wrote:
> >gcc remarks:
> >
> >../libibverbs/src/neigh.c:339:6: warning: 'sock_fd' may be used uninitialized in this function [-Wmaybe-uninitialized]
> >  err = try_send_to(sock_fd, buff, sizeof(buff), &addr_dst);
> 
> 
> Why not just initializing 'sock_fd' to any value (e.g. to 0) when it's
> declared and drop all below patch ? as the code checks the return code from
> create_socket() it should be safe.

Because that is a lazy fix. This code is full of cruft computing an
error value that is never used. There is no reason to do that so the
best approach is to remove the dead code cruft.

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 20/28] verbs: Use inttypes.h format string macros

[Jason Gunthorpe]

Otherwise a 32bit compile of ibv_devinfo will print garbage for
these values.

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

diff --git a/libibverbs/examples/devinfo.c b/libibverbs/examples/devinfo.c
index c49765046965..2b673c854860 100644
--- a/libibverbs/examples/devinfo.c
+++ b/libibverbs/examples/devinfo.c
@@ -452,13 +452,13 @@ static int print_hca_cap(struct ibv_device *ib_dev, uint8_t ib_port)
 
 		print_odp_caps(&device_attr.odp_caps);
 		if (device_attr.completion_timestamp_mask)
-			printf("\tcompletion timestamp_mask:\t\t\t0x%016lx\n",
+			printf("\tcompletion timestamp_mask:\t\t\t0x%016" PRIx64 "\n",
 			       device_attr.completion_timestamp_mask);
 		else
 			printf("\tcompletion_timestamp_mask not supported\n");
 
 		if (device_attr.hca_core_clock)
-			printf("\thca_core_clock:\t\t\t%lukHZ\n", device_attr.hca_core_clock);
+			printf("\thca_core_clock:\t\t\t%" PRIu64 "kHZ\n", device_attr.hca_core_clock);
 		else
 			printf("\tcore clock not supported\n");
 
-- 
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 21/28] mlx5: Avoid gcc 5.4 warning -Wempty-body

[Jason Gunthorpe]

gcc 5.4 remarks:

../providers/mlx5/buf.c:95:61: warning: suggest braces around empty body in an 'if' statement [-Wempty-body]

Around code like this:

	if (shmdt(hmem->shmaddr) == -1)
		mlx5_dbg(stderr, MLX5_DBG_CONTIG, "%s\n", strerror(errno));

When mlx5_dgb is defined as an empty macro the code expands to
	if (..)
		;

Which is functionally okay, but strange.

It is much better to make mlx5_dbg an empty static inline, this way it
continues to do parameter type validation even when disabled, and we
can drop the sprinkling of #ifdef MLX5_DEBUG everywhere.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libmlx5/src/mlx5.h  |  6 +++++-
 libmlx5/src/qp.c    |  7 -------
 libmlx5/src/verbs.c | 10 ----------
 3 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/libmlx5/src/mlx5.h b/libmlx5/src/mlx5.h
index 5833339a543f..b4787dade7cb 100644
--- a/libmlx5/src/mlx5.h
+++ b/libmlx5/src/mlx5.h
@@ -158,7 +158,11 @@ do {									\
 } while (0)
 
 #else
-	#define mlx5_dbg(fp, mask, format, arg...)
+static inline void mlx5_dbg(FILE *fp, uint32_t mask, const char *fmt, ...)
+	__attribute__((format(printf, 3, 4)));
+static inline void mlx5_dbg(FILE *fp, uint32_t mask, const char *fmt, ...)
+{
+}
 #endif
 
 enum {
diff --git a/libmlx5/src/qp.c b/libmlx5/src/qp.c
index c805fcae4123..23270e50af7a 100644
--- a/libmlx5/src/qp.c
+++ b/libmlx5/src/qp.c
@@ -356,9 +356,7 @@ static inline int copy_eth_inline_headers(struct ibv_qp *ibqp,
 	int inl_hdr_size = MLX5_ETH_L2_INLINE_HEADER_SIZE;
 	int inl_hdr_copy_size = 0;
 	int j = 0;
-#ifdef MLX5_DEBUG
 	FILE *fp = to_mctx(ibqp->context)->dbg_fp;
-#endif
 
 	if (unlikely(wr->num_sge < 1)) {
 		mlx5_dbg(fp, MLX5_DBG_QP_SEND, "illegal num_sge: %d, minimum is 1\n",
@@ -560,9 +558,7 @@ static inline int set_tso_eth_seg(void **seg, struct ibv_send_wr *wr,
 	int size_of_inl_hdr_start = sizeof(eseg->inline_hdr_start);
 	uint64_t left, left_len, copy_sz;
 	void *pdata = wr->tso.hdr;
-#ifdef MLX5_DEBUG
 	FILE *fp = to_mctx(qp->ibv_qp->context)->dbg_fp;
-#endif
 
 	if (unlikely(wr->tso.hdr_sz < MLX5_ETH_L2_MIN_HEADER_SIZE ||
 		     wr->tso.hdr_sz > qp->max_tso_header)) {
@@ -629,10 +625,7 @@ static inline int _mlx5_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr,
 	uint8_t fence;
 	uint8_t next_fence;
 	uint32_t max_tso = 0;
-
-#ifdef MLX5_DEBUG
 	FILE *fp = to_mctx(ibqp->context)->dbg_fp;
-#endif
 
 	mlx5_spin_lock(&qp->sq.lock);
 
diff --git a/libmlx5/src/verbs.c b/libmlx5/src/verbs.c
index 07e2545e2791..a76821e6f695 100644
--- a/libmlx5/src/verbs.c
+++ b/libmlx5/src/verbs.c
@@ -360,9 +360,7 @@ static struct ibv_cq_ex *create_cq(struct ibv_context *context,
 	int				cqe_sz;
 	int				ret;
 	int				ncqe;
-#ifdef MLX5_DEBUG
 	FILE *fp = to_mctx(context)->dbg_fp;
-#endif
 
 	if (!cq_attr->cqe) {
 		mlx5_dbg(fp, MLX5_DBG_CQ, "CQE invalid\n");
@@ -840,9 +838,7 @@ static int mlx5_calc_sq_size(struct mlx5_context *ctx,
 {
 	int wqe_size;
 	int wq_size;
-#ifdef MLX5_DEBUG
 	FILE *fp = ctx->dbg_fp;
-#endif
 
 	if (!attr->cap.max_send_wr)
 		return 0;
@@ -892,9 +888,7 @@ static int mlx5_calc_rq_size(struct mlx5_context *ctx,
 	int wqe_size;
 	int wq_size;
 	int scat_spc;
-#ifdef MLX5_DEBUG
 	FILE *fp = ctx->dbg_fp;
-#endif
 
 	if (!attr->cap.max_recv_wr)
 		return 0;
@@ -1146,9 +1140,7 @@ struct ibv_qp *create_qp(struct ibv_context *context,
 	struct ibv_qp		       *ibqp;
 	int32_t				usr_idx = 0;
 	uint32_t			uuar_index;
-#ifdef MLX5_DEBUG
 	FILE *fp = ctx->dbg_fp;
-#endif
 
 	if (attr->comp_mask & ~MLX5_CREATE_QP_SUP_COMP_MASK)
 		return NULL;
@@ -1610,9 +1602,7 @@ mlx5_create_xrc_srq(struct ibv_context *context,
 	int max_sge;
 	struct ibv_srq *ibsrq;
 	int uidx;
-#ifdef MLX5_DEBUG
 	FILE *fp = ctx->dbg_fp;
-#endif
 
 	msrq = calloc(1, sizeof(*msrq));
 	if (!msrq)
-- 
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 21/28] mlx5: Avoid gcc 5.4 warning -Wempty-body

[Yishai Hadas]

On 9/6/2016 12:08 AM, Jason Gunthorpe wrote:
> gcc 5.4 remarks:
>
> ../providers/mlx5/buf.c:95:61: warning: suggest braces around empty body in an 'if' statement [-Wempty-body]
>
> Around code like this:
>
> 	if (shmdt(hmem->shmaddr) == -1)
> 		mlx5_dbg(stderr, MLX5_DBG_CONTIG, "%s\n", strerror(errno));
>
> When mlx5_dgb is defined as an empty macro the code expands to
> 	if (..)
> 		;
>
> Which is functionally okay, but strange.

As you pointed, there is no functional issue with that.

> It is much better to make mlx5_dbg an empty static inline, this way it
> continues to do parameter type validation even when disabled, and we
> can drop the sprinkling of #ifdef MLX5_DEBUG everywhere.

The idea was to have an 'empty' code when this macro is used as some 
code is done in the data path, see below notes.


> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> ---
>  libmlx5/src/mlx5.h  |  6 +++++-
>  libmlx5/src/qp.c    |  7 -------
>  libmlx5/src/verbs.c | 10 ----------
>  3 files changed, 5 insertions(+), 18 deletions(-)
>
> diff --git a/libmlx5/src/mlx5.h b/libmlx5/src/mlx5.h
> index 5833339a543f..b4787dade7cb 100644
> --- a/libmlx5/src/mlx5.h
> +++ b/libmlx5/src/mlx5.h
> @@ -158,7 +158,11 @@ do {									\
>  } while (0)
>
>  #else
> -	#define mlx5_dbg(fp, mask, format, arg...)
> +static inline void mlx5_dbg(FILE *fp, uint32_t mask, const char *fmt, ...)
> +	__attribute__((format(printf, 3, 4)));
> +static inline void mlx5_dbg(FILE *fp, uint32_t mask, const char *fmt, ...)
> +{
> +}
>  #endif

Inline is not guaranteed by the compiler, calling empty function in some 
data data path flows should be prevented.

>  enum {
> diff --git a/libmlx5/src/qp.c b/libmlx5/src/qp.c
> index c805fcae4123..23270e50af7a 100644
> --- a/libmlx5/src/qp.c
> +++ b/libmlx5/src/qp.c
> @@ -356,9 +356,7 @@ static inline int copy_eth_inline_headers(struct ibv_qp *ibqp,
>  	int inl_hdr_size = MLX5_ETH_L2_INLINE_HEADER_SIZE;
>  	int inl_hdr_copy_size = 0;
>  	int j = 0;
> -#ifdef MLX5_DEBUG
>  	FILE *fp = to_mctx(ibqp->context)->dbg_fp;
> -#endif

This is data path flow, dropping the #ifdef may cause a redundant 
assignment for 'fp' here.

>  	if (unlikely(wr->num_sge < 1)) {
>  		mlx5_dbg(fp, MLX5_DBG_QP_SEND, "illegal num_sge: %d, minimum is 1\n",
> @@ -560,9 +558,7 @@ static inline int set_tso_eth_seg(void **seg, struct ibv_send_wr *wr,
>  	int size_of_inl_hdr_start = sizeof(eseg->inline_hdr_start);
>  	uint64_t left, left_len, copy_sz;
>  	void *pdata = wr->tso.hdr;
> -#ifdef MLX5_DEBUG
>  	FILE *fp = to_mctx(qp->ibv_qp->context)->dbg_fp;
> -#endif

Again, same issue as of above, here and below in the post_send flow.

>  	if (unlikely(wr->tso.hdr_sz < MLX5_ETH_L2_MIN_HEADER_SIZE ||
>  		     wr->tso.hdr_sz > qp->max_tso_header)) {
> @@ -629,10 +625,7 @@ static inline int _mlx5_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr,
>  	uint8_t fence;
>  	uint8_t next_fence;
>  	uint32_t max_tso = 0;
> -
> -#ifdef MLX5_DEBUG
>  	FILE *fp = to_mctx(ibqp->context)->dbg_fp;
> -#endif
>
>  	mlx5_spin_lock(&qp->sq.lock);
>
> diff --git a/libmlx5/src/verbs.c b/libmlx5/src/verbs.c
> index 07e2545e2791..a76821e6f695 100644
> --- a/libmlx5/src/verbs.c
> +++ b/libmlx5/src/verbs.c
> @@ -360,9 +360,7 @@ static struct ibv_cq_ex *create_cq(struct ibv_context *context,
>  	int				cqe_sz;
>  	int				ret;
>  	int				ncqe;
> -#ifdef MLX5_DEBUG
>  	FILE *fp = to_mctx(context)->dbg_fp;
> -#endif
>
>  	if (!cq_attr->cqe) {
>  		mlx5_dbg(fp, MLX5_DBG_CQ, "CQE invalid\n");
> @@ -840,9 +838,7 @@ static int mlx5_calc_sq_size(struct mlx5_context *ctx,
>  {
>  	int wqe_size;
>  	int wq_size;
> -#ifdef MLX5_DEBUG
>  	FILE *fp = ctx->dbg_fp;
> -#endif
>
>  	if (!attr->cap.max_send_wr)
>  		return 0;
> @@ -892,9 +888,7 @@ static int mlx5_calc_rq_size(struct mlx5_context *ctx,
>  	int wqe_size;
>  	int wq_size;
>  	int scat_spc;
> -#ifdef MLX5_DEBUG
>  	FILE *fp = ctx->dbg_fp;
> -#endif
>
>  	if (!attr->cap.max_recv_wr)
>  		return 0;
> @@ -1146,9 +1140,7 @@ struct ibv_qp *create_qp(struct ibv_context *context,
>  	struct ibv_qp		       *ibqp;
>  	int32_t				usr_idx = 0;
>  	uint32_t			uuar_index;
> -#ifdef MLX5_DEBUG
>  	FILE *fp = ctx->dbg_fp;
> -#endif
>
>  	if (attr->comp_mask & ~MLX5_CREATE_QP_SUP_COMP_MASK)
>  		return NULL;
> @@ -1610,9 +1602,7 @@ mlx5_create_xrc_srq(struct ibv_context *context,
>  	int max_sge;
>  	struct ibv_srq *ibsrq;
>  	int uidx;
> -#ifdef MLX5_DEBUG
>  	FILE *fp = ctx->dbg_fp;
> -#endif
>
>  	msrq = calloc(1, sizeof(*msrq));
>  	if (!msrq)
>

--
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 21/28] mlx5: Avoid gcc 5.4 warning -Wempty-body

[Jason Gunthorpe]

On Wed, Sep 14, 2016 at 07:25:25PM +0300, Yishai Hadas wrote:
> >Which is functionally okay, but strange.
> 
> As you pointed, there is no functional issue with that.

Yet is a common enough errnonous construction to attract a compiler
warning. Don't do it.

Further, it is just a way to attract bit rot in your dbg macros if you
don't check the format string during a normal compile.

> >It is much better to make mlx5_dbg an empty static inline, this way it
> >continues to do parameter type validation even when disabled, and we
> >can drop the sprinkling of #ifdef MLX5_DEBUG everywhere.
> 
> The idea was to have an 'empty' code when this macro is used as some code is
> done in the data path, see below notes.

Yes, I know what the intent was, this change does exactly the same thing.

> >+static inline void mlx5_dbg(FILE *fp, uint32_t mask, const char *fmt, ...)
> >+{
> >+}
> > #endif
> 
> Inline is not guaranteed by the compiler, calling empty function in some
> data data path flows should be prevented.

No compiler we support will leave behind a function call for an empty
static inline.

I just checked, the assembly is the same before/after modulo compiler
non-determinism. There are no extra function calls.

> > enum {
> >diff --git a/libmlx5/src/qp.c b/libmlx5/src/qp.c
> >index c805fcae4123..23270e50af7a 100644
> >+++ b/libmlx5/src/qp.c
> >@@ -356,9 +356,7 @@ static inline int copy_eth_inline_headers(struct ibv_qp *ibqp,
> > 	int inl_hdr_size = MLX5_ETH_L2_INLINE_HEADER_SIZE;
> > 	int inl_hdr_copy_size = 0;
> > 	int j = 0;
> >-#ifdef MLX5_DEBUG
> > 	FILE *fp = to_mctx(ibqp->context)->dbg_fp;
> >-#endif
> 
> This is data path flow, dropping the #ifdef may cause a redundant assignment
> for 'fp' here.

Nope:

$ nm --size libmlx4-rdmv2.so
[before]
0000000000001637 t _mlx5_post_send
[after]
0000000000001637 t _mlx5_post_send

The compilers are very smart, you don't need to hand hold them.

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

Re: [PATCH 21/28] mlx5: Avoid gcc 5.4 warning -Wempty-body

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 2444 bytes --]

On Wed, Sep 14, 2016 at 10:55:19AM -0600, Jason Gunthorpe wrote:
> On Wed, Sep 14, 2016 at 07:25:25PM +0300, Yishai Hadas wrote:
> > >Which is functionally okay, but strange.
> >
> > As you pointed, there is no functional issue with that.
>
> Yet is a common enough errnonous construction to attract a compiler
> warning. Don't do it.
>
> Further, it is just a way to attract bit rot in your dbg macros if you
> don't check the format string during a normal compile.
>
> > >It is much better to make mlx5_dbg an empty static inline, this way it
> > >continues to do parameter type validation even when disabled, and we
> > >can drop the sprinkling of #ifdef MLX5_DEBUG everywhere.
> >
> > The idea was to have an 'empty' code when this macro is used as some code is
> > done in the data path, see below notes.
>
> Yes, I know what the intent was, this change does exactly the same thing.
>
> > >+static inline void mlx5_dbg(FILE *fp, uint32_t mask, const char *fmt, ...)
> > >+{
> > >+}
> > > #endif
> >
> > Inline is not guaranteed by the compiler, calling empty function in some
> > data data path flows should be prevented.
>
> No compiler we support will leave behind a function call for an empty
> static inline.
>
> I just checked, the assembly is the same before/after modulo compiler
> non-determinism. There are no extra function calls.

There is always inline attribute to tell to GCC to inline it.
#define ALWAYS_INLINE inline __attribute__ ((always_inline))

>
> > > enum {
> > >diff --git a/libmlx5/src/qp.c b/libmlx5/src/qp.c
> > >index c805fcae4123..23270e50af7a 100644
> > >+++ b/libmlx5/src/qp.c
> > >@@ -356,9 +356,7 @@ static inline int copy_eth_inline_headers(struct ibv_qp *ibqp,
> > > 	int inl_hdr_size = MLX5_ETH_L2_INLINE_HEADER_SIZE;
> > > 	int inl_hdr_copy_size = 0;
> > > 	int j = 0;
> > >-#ifdef MLX5_DEBUG
> > > 	FILE *fp = to_mctx(ibqp->context)->dbg_fp;
> > >-#endif
> >
> > This is data path flow, dropping the #ifdef may cause a redundant assignment
> > for 'fp' here.
>
> Nope:
>
> $ nm --size libmlx4-rdmv2.so
> [before]
> 0000000000001637 t _mlx5_post_send
> [after]
> 0000000000001637 t _mlx5_post_send
>
> The compilers are very smart, you don't need to hand hold them.
>
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 21/28] mlx5: Avoid gcc 5.4 warning -Wempty-body

[Jason Gunthorpe]

On Thu, Sep 15, 2016 at 08:18:40AM +0300, Leon Romanovsky wrote:

> > I just checked, the assembly is the same before/after modulo compiler
> > non-determinism. There are no extra function calls.
> 
> There is always inline attribute to tell to GCC to inline it.
> #define ALWAYS_INLINE inline __attribute__ ((always_inline))

If Yishai is dead set against this then I'll use

#define mlx5_dbg(fp, mask, format, arg...) do {} while(0)

But this patch really is a nice little #ifdef cleanup with no
performance down side.

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

Re: [PATCH 21/28] mlx5: Avoid gcc 5.4 warning -Wempty-body

[Yishai Hadas]

On 9/14/2016 7:55 PM, Jason Gunthorpe wrote:
> No compiler we support will leave behind a function call for an empty
> static inline.
>
> I just checked, the assembly is the same before/after modulo compiler
> non-determinism. There are no extra function calls.
>
>>> enum {
>>> diff --git a/libmlx5/src/qp.c b/libmlx5/src/qp.c
>>> index c805fcae4123..23270e50af7a 100644
>>> +++ b/libmlx5/src/qp.c
>>> @@ -356,9 +356,7 @@ static inline int copy_eth_inline_headers(struct ibv_qp *ibqp,
>>> 	int inl_hdr_size = MLX5_ETH_L2_INLINE_HEADER_SIZE;
>>> 	int inl_hdr_copy_size = 0;
>>> 	int j = 0;
>>> -#ifdef MLX5_DEBUG
>>> 	FILE *fp = to_mctx(ibqp->context)->dbg_fp;
>>> -#endif
>>
>> This is data path flow, dropping the #ifdef may cause a redundant assignment
>> for 'fp' here.
>
> Nope:
>
> $ nm --size libmlx4-rdmv2.so
> [before]
> 0000000000001637 t _mlx5_post_send
> [after]
> 0000000000001637 t _mlx5_post_send
>
> The compilers are very smart, you don't need to hand hold them.

Thanks for checking that, will take into our regression system to run on 
before formally taking.
--
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 22/28] nes: Avoid gcc 6.1 warning -Wmisleading-indentation

[Jason Gunthorpe]

../providers/nes/nes_uverbs.c:489:5: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
     if (++nesuqp->rq_tail >= nesuqp->rq_size)
     ^~
../providers/nes/nes_uverbs.c:491:6: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'if'
      if (entry->status == NES_CQ_BUF_OV_ERR)
      ^~

Presumably this has been tested as is, so I've opted to preserve the
behaviour, but I can't tell if that is right or not.

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

diff --git a/libnes/src/nes_uverbs.c b/libnes/src/nes_uverbs.c
index 80891d6243c7..6458c51c3f55 100644
--- a/libnes/src/nes_uverbs.c
+++ b/libnes/src/nes_uverbs.c
@@ -518,8 +518,8 @@ int nes_ima_upoll_cq(struct ibv_cq *cq, int num_entries, struct ibv_wc *entry)
 				/* Working on a RQ Completion*/
 				if (++nesuqp->rq_tail >= nesuqp->rq_size)
 					nesuqp->rq_tail = 0;
-					if (entry->status == NES_CQ_BUF_OV_ERR)
-						entry->status = IBV_WC_LOC_LEN_ERR;
+				if (entry->status == NES_CQ_BUF_OV_ERR)
+					entry->status = IBV_WC_LOC_LEN_ERR;
 			}
 
 			if (++head >= cq_size)
-- 
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 23/28] rxe: Avoid gcc 5.4 warning -Wswitch

[Jason Gunthorpe]

convert_send_wr switches incompletely on an enum. Assume the intent was to
not do any copies for other enum members and dummy them in.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 librxe/src/rxe.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/librxe/src/rxe.c b/librxe/src/rxe.c
index 94d0de51f7dd..7164f6627171 100644
--- a/librxe/src/rxe.c
+++ b/librxe/src/rxe.c
@@ -596,6 +596,12 @@ void convert_send_wr(struct rxe_send_wr *kwr, struct ibv_send_wr *uwr)
 		kwr->wr.atomic.swap		= uwr->wr.atomic.swap;
 		kwr->wr.atomic.rkey		= uwr->wr.atomic.rkey;
 		break;
+
+	case IBV_WR_LOCAL_INV:
+	case IBV_WR_BIND_MW:
+	case IBV_WR_SEND_WITH_INV:
+	case IBV_WR_TSO:
+		break;
 	}
 }
 
-- 
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 24/28] Fix gcc 5.4, clang 3.6 warnings about unused objects

[Jason Gunthorpe]

Mostly just delete cruft.

nes has a number of unused related to HAVE_DECL_IBV_QPT_RAW_ETH,
perhaps this code should be deleted entirely because whatever
QPT_RAW_ETH is, it is not part of this repository.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libcxgb3/src/qp.c                  | 41 --------------------------------------
 libibumad/tests/umad_reg2_compat.c |  1 -
 libnes/src/nes_uverbs.c            | 17 +++++++++-------
 libocrdma/src/ocrdma_verbs.c       |  3 ++-
 librdmacm/examples/cmtime.c        | 16 ---------------
 5 files changed, 12 insertions(+), 66 deletions(-)

diff --git a/libcxgb3/src/qp.c b/libcxgb3/src/qp.c
index 34b6fa9f5632..7dd3c7e342a7 100644
--- a/libcxgb3/src/qp.c
+++ b/libcxgb3/src/qp.c
@@ -285,47 +285,6 @@ int t3a_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr,
 	return ret;
 }
 
-/* 
- * XXX: This is going to be moved to firmware. 
- *      Missing pdid/qpid check for now.
- */
-static inline int iwch_sgl2pbl_map(struct iwch_device *rhp,
-				   struct ibv_sge *sg_list, uint32_t num_sgle,
-				   uint32_t *pbl_addr, uint8_t *page_size)
-{
-	int i;
-	struct iwch_mr *mhp;
-	uint32_t offset;
-	for (i = 0; i < num_sgle; i++) {
-		mhp = rhp->mmid2ptr[t3_mmid(sg_list[i].lkey)];
-		if (!mhp) {
-			PDBG("%s %d\n", __FUNCTION__, __LINE__);
-			return -1;
-		}
-		if (sg_list[i].addr < mhp->va_fbo) {
-			PDBG("%s %d\n", __FUNCTION__, __LINE__);
-			return -1;
-		}
-		if (sg_list[i].addr + ((uint64_t) sg_list[i].length) <
-		    sg_list[i].addr) {
-			PDBG("%s %d\n", __FUNCTION__, __LINE__);
-			return -1;
-		}
-		if (sg_list[i].addr + ((uint64_t) sg_list[i].length) >
-		    mhp->va_fbo + ((uint64_t) mhp->len)) {
-			PDBG("%s %d\n", __FUNCTION__, __LINE__);
-			return -1;
-		}
-		offset = sg_list[i].addr - mhp->va_fbo;
-		offset += ((uint32_t) mhp->va_fbo) %
-		    (1UL << (12 + mhp->page_size));
-		pbl_addr[i] = mhp->pbl_addr +
-		    (offset >> (12 + mhp->page_size));
-		page_size[i] = mhp->page_size;
-	}
-	return 0;
-}
-
 static inline int iwch_build_rdma_recv(struct iwch_device *rhp,
 				       union t3_wr *wqe, 
 				       struct ibv_recv_wr *wr)
diff --git a/libibumad/tests/umad_reg2_compat.c b/libibumad/tests/umad_reg2_compat.c
index 6dd4a48a59b2..9c239ee4bfae 100644
--- a/libibumad/tests/umad_reg2_compat.c
+++ b/libibumad/tests/umad_reg2_compat.c
@@ -100,7 +100,6 @@ int open_test_device(void)
 
 void test_register(void)
 {
-	int rc = 0;
 	int agent_id;
 	long method_mask[16 / sizeof(long)];
 	uint32_t class_oui = 0x001405; /* OPENIB_OUI */
diff --git a/libnes/src/nes_uverbs.c b/libnes/src/nes_uverbs.c
index 6458c51c3f55..983d87a80b8b 100644
--- a/libnes/src/nes_uverbs.c
+++ b/libnes/src/nes_uverbs.c
@@ -215,6 +215,7 @@ int nes_udereg_mr(struct ibv_mr *mr)
 	return 0;
 }
 
+#if HAVE_DECL_IBV_QPT_RAW_ETH
 static
 int nes_ima_ureplace_cq(struct ibv_cq *cq,
 			int mcrqf,
@@ -296,6 +297,7 @@ int nes_ima_ureplace_cq(struct ibv_cq *cq,
  err:
 	return ret;
 }
+#endif
 
 /**
  * nes_ucreate_cq
@@ -425,7 +427,6 @@ int nes_ima_upoll_cq(struct ibv_cq *cq, int num_entries, struct ibv_wc *entry)
 	int cqe_count = 0;
 	uint32_t head;
 	uint32_t cq_size;
-	uint16_t qp_size;
 
 	volatile struct nes_hw_nic_cqe *cqe = 0;
 	volatile struct nes_hw_nic_cqe *cqes;
@@ -487,7 +488,6 @@ int nes_ima_upoll_cq(struct ibv_cq *cq, int num_entries, struct ibv_wc *entry)
 			entry->src_qp = nesuqp->qp_id;
 			if (cqe_misc & NES_NIC_CQE_SQ) {
 				entry->opcode = IBV_WC_SEND;
-				qp_size = nesuqp->sq_size;
 
 				entry->wr_id =
 					nesuqp->send_wr_id[nesuqp->sq_tail];
@@ -557,7 +557,6 @@ int nes_upoll_cq(struct ibv_cq *cq, int num_entries, struct ibv_wc *entry)
 	uint32_t wqe_index;
 	uint32_t wq_tail = 0;
 	struct nes_hw_cqe cqe;
-	uint32_t tmp;
 	uint64_t u64temp;
 	int move_cq_head = 1;
 	uint32_t err_code;
@@ -679,7 +678,6 @@ nes_upoll_cq_update:
 					nesvctx = to_nes_uctx(cq->context);
 				nesvctx->nesupd->udoorbell->cqe_alloc = cpu_to_le32(nesucq->cq_id |
 						(nesucq->polled_completions << 16));
-				tmp = nesvctx->nesupd->udoorbell->cqe_alloc;
 				nesucq->polled_completions = 0;
 			}
 		} else {
@@ -699,7 +697,6 @@ nes_upoll_cq_update:
 			nesvctx = to_nes_uctx(cq->context);
 		nesvctx->nesupd->udoorbell->cqe_alloc = cpu_to_le32(nesucq->cq_id |
 				(nesucq->polled_completions << 16));
-		tmp = nesvctx->nesupd->udoorbell->cqe_alloc;
 		nesucq->polled_completions = 0;
 	}
 	nesucq->head = head;
@@ -1140,7 +1137,6 @@ struct ibv_qp *nes_ucreate_qp(struct ibv_pd *pd, struct ibv_qp_init_attr *attr)
 	struct nes_uqp *nesuqp;
 	int	sqdepth, rqdepth;
 	int	 status = 1;
-	int i = 0;
 
 	/* fprintf(stderr, PFX "%s\n", __FUNCTION__); */
 
@@ -1211,6 +1207,8 @@ struct ibv_qp *nes_ucreate_qp(struct ibv_pd *pd, struct ibv_qp_init_attr *attr)
 
 #if HAVE_DECL_IBV_QPT_RAW_ETH
 	if (attr->qp_type == IBV_QPT_RAW_ETH) {
+		int i = 0;
+
 		nesuqp->nes_ud_sksq_fd = open("/dev/infiniband/nes_ud_sksq",
 						O_RDWR);
 		if (nesuqp->nes_ud_sksq_fd <= 0)
@@ -1327,7 +1325,6 @@ int nes_udestroy_qp(struct ibv_qp *qp)
 {
 	struct nes_uqp *nesuqp = to_nes_uqp(qp);
 	int ret = 0;
-	int i = 0;
 
 	// fprintf(stderr, PFX "%s addr&mr= %p  \n", __FUNCTION__, &nesuqp->mr );
 
@@ -1353,6 +1350,8 @@ int nes_udestroy_qp(struct ibv_qp *qp)
 
 #if HAVE_DECL_IBV_QPT_RAW_ETH
 	if (qp->qp_type == IBV_QPT_RAW_ETH) {
+		int i = 0;
+
 		if (nesuqp->pend_rx_wr) {
 			for (i = 0; i < NES_UD_RX_BATCH_SZ; i++)
 				if (nesuqp->pend_rx_wr[i].sg_list) {
@@ -1381,6 +1380,7 @@ int nes_udestroy_qp(struct ibv_qp *qp)
 	return 0;
 }
 
+#if HAVE_DECL_IBV_QPT_RAW_ETH
 static inline
 int nes_ima_upost_send(struct ibv_qp *ib_qp, struct ibv_send_wr *ib_wr,
 		struct ibv_send_wr **bad_wr)
@@ -1457,6 +1457,7 @@ int nes_ima_upost_send(struct ibv_qp *ib_qp, struct ibv_send_wr *ib_wr,
 out:
 	return ret;
 }
+#endif
 
 /**
  * nes_upost_send
@@ -1653,6 +1654,7 @@ int nes_upost_send(struct ibv_qp *ib_qp, struct ibv_send_wr *ib_wr,
 	return err;
 }
 
+#if HAVE_DECL_IBV_QPT_RAW_ETH
 static inline
 int nes_ima_upost_recv(struct ibv_qp *ib_qp, struct ibv_recv_wr *ib_wr,
 		struct ibv_recv_wr **bad_wr)
@@ -1727,6 +1729,7 @@ int nes_ima_upost_recv(struct ibv_qp *ib_qp, struct ibv_recv_wr *ib_wr,
 out:
 	return ret;
 }
+#endif
 
 /**
  * nes_upost_recv
diff --git a/libocrdma/src/ocrdma_verbs.c b/libocrdma/src/ocrdma_verbs.c
index 60626260656d..8eb70db2693a 100644
--- a/libocrdma/src/ocrdma_verbs.c
+++ b/libocrdma/src/ocrdma_verbs.c
@@ -945,10 +945,11 @@ static inline void *ocrdma_hwq_head(struct ocrdma_qp_hwq_info *q)
 	return q->va + (q->head * q->entry_size);
 }
 
-static inline void *ocrdma_wq_tail(struct ocrdma_qp_hwq_info *q)
+/*static inline void *ocrdma_wq_tail(struct ocrdma_qp_hwq_info *q)
 {
 	return q->va + (q->tail * q->entry_size);
 }
+*/
 
 static inline void *ocrdma_hwq_head_from_idx(struct ocrdma_qp_hwq_info *q,
 					     uint32_t idx)
diff --git a/librdmacm/examples/cmtime.c b/librdmacm/examples/cmtime.c
index e45980b9bb04..f0b4d0276288 100644
--- a/librdmacm/examples/cmtime.c
+++ b/librdmacm/examples/cmtime.c
@@ -128,13 +128,6 @@ static inline int __list_empty(struct work_list *list)
 	return list->list.next == &list->list;
 }
 
-static inline int list_empty(struct work_list *work_list)
-{
-	pthread_mutex_lock(&work_list->lock);
-	return work_list->list.next == &work_list->list;
-	pthread_mutex_unlock(&work_list->lock);
-}
-
 static inline struct list_head *__list_remove_head(struct work_list *work_list)
 {
 	struct list_head *list_item;
@@ -144,15 +137,6 @@ static inline struct list_head *__list_remove_head(struct work_list *work_list)
 	return list_item;
 }
 
-static inline struct list_head *list_remove_head(struct work_list *work_list)
-{
-	struct list_head *list_item;
-	pthread_mutex_lock(&work_list->lock);
-	list_item = __list_remove_head(work_list);
-	pthread_mutex_unlock(&work_list->lock);
-	return list_item;
-}
-
 static inline void list_add_tail(struct work_list *work_list, struct list_head *req)
 {
 	int empty;
-- 
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 24/28] Fix gcc 5.4, clang 3.6 warnings about unused objects

[Steve Wise]

> Mostly just delete cruft.
> 
> nes has a number of unused related to HAVE_DECL_IBV_QPT_RAW_ETH,
> perhaps this code should be deleted entirely because whatever
> QPT_RAW_ETH is, it is not part of this repository.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> ---
>  libcxgb3/src/qp.c                  | 41
--------------------------------------

Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>

libcxgb3 patch applied.

Thanks,

Steve.


--
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 25/28] Avoid gcc 5.4 warning -Wtype-limits

[Jason Gunthorpe]

Eg: comparison of unsigned expression < 0 is always false

These are all harmless cases where some simple adjustments will
supress the warning.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libcxgb3/src/iwch.c    | 2 +-
 libcxgb4/src/dev.c     | 4 ++--
 libibverbs/src/neigh.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/libcxgb3/src/iwch.c b/libcxgb3/src/iwch.c
index 32447e427146..dd0d3711f2e2 100644
--- a/libcxgb3/src/iwch.c
+++ b/libcxgb3/src/iwch.c
@@ -225,7 +225,7 @@ found:
 
 	DBGLOG("libcxgb3");
 
-	if (fw_min < FW_MIN) {
+	if ((signed int)fw_min < FW_MIN) {
 		PDBG("libcxgb3: non-fatal firmware version mismatch.  "
 			"Firmware minor number is %u and libcxgb3 needs %u.\n",
 			fw_min, FW_MIN);
diff --git a/libcxgb4/src/dev.c b/libcxgb4/src/dev.c
index cd3217af77b2..ebcc72ecc991 100644
--- a/libcxgb4/src/dev.c
+++ b/libcxgb4/src/dev.c
@@ -435,7 +435,7 @@ found:
 	cp = strtok(NULL, ".");
 	sscanf(cp, "%i", &fw_min);
 
-	if (fw_maj < FW_MAJ) {
+	if ((signed int)fw_maj < FW_MAJ) {
 		fprintf(stderr, "libcxgb4: Fatal firmware version mismatch.  "
 			"Firmware major number is %u and libcxgb4 needs %u.\n",
 			fw_maj, FW_MAJ);
@@ -445,7 +445,7 @@ found:
 
 	DBGLOG("libcxgb4");
 
-	if (fw_min < FW_MIN) {
+	if ((signed int)fw_min < FW_MIN) {
 		PDBG("libcxgb4: non-fatal firmware version mismatch.  "
 			"Firmware minor number is %u and libcxgb4 needs %u.\n",
 			fw_min, FW_MIN);
diff --git a/libibverbs/src/neigh.c b/libibverbs/src/neigh.c
index dc8c2bc99ae3..6b6e58cd52f8 100644
--- a/libibverbs/src/neigh.c
+++ b/libibverbs/src/neigh.c
@@ -727,7 +727,7 @@ uint16_t neigh_get_vlan_id_from_dev(struct get_neigh_handler *neigh_handler)
 
 void neigh_set_vlan_id(struct get_neigh_handler *neigh_handler, uint16_t vid)
 {
-	if (vid >= 0 && vid <= 0xfff)
+	if (vid <= 0xfff)
 		neigh_handler->vid = vid;
 }
 
-- 
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 25/28] Avoid gcc 5.4 warning -Wtype-limits

[Steve Wise]

> Eg: comparison of unsigned expression < 0 is always false
> 
> These are all harmless cases where some simple adjustments will
> supress the warning.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> ---
>  libcxgb3/src/iwch.c    | 2 +-
>  libcxgb4/src/dev.c     | 4 ++--

Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>

libcxgb* patches applied.

Thanks,

Steve. 


--
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 26/28] Avoid gcc 5.4 warning -Wunused-result

[Jason Gunthorpe]

It used to be you could suppress this with (void), however the gcc
developers have decided to get rid of that.

So, look closely at each occurrence and decide what to do:
- *pingpong: Join the error handling with the if statement directly
   above
- niegh: read on a timer_fd should never fail, so just use assert.
   The assert is compiled out for Release builds so this is no-change
- acm: Failure of ucma_set_server_port is detected by a 0 return
   so check fscanf and return appropriately. This is no change since
   fscanf failure was assumed to have left server_port as 0 (though
   I doubt the standard supports that usage)
- rsocket: This looks super sketchy. At least lets make the intent clear
   with a read_all/write_all wrapper that calls assert. Most likely
   this code is wrong..
   Mangle the code with failable_fscanf to make it clear, but as with
   acm, I don't think the standard supports this usage.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libibverbs/examples/rc_pingpong.c  | 15 +++++-----
 libibverbs/examples/srq_pingpong.c | 13 +++++++--
 libibverbs/examples/uc_pingpong.c  | 15 +++++-----
 libibverbs/examples/ud_pingpong.c  | 17 +++++-------
 libibverbs/src/neigh.c             |  7 +++--
 librdmacm/src/acm.c                |  3 +-
 librdmacm/src/rsocket.c            | 56 +++++++++++++++++++++++++++-----------
 7 files changed, 78 insertions(+), 48 deletions(-)

diff --git a/libibverbs/examples/rc_pingpong.c b/libibverbs/examples/rc_pingpong.c
index 1fad16a0be7c..7bcc413a0f1d 100644
--- a/libibverbs/examples/rc_pingpong.c
+++ b/libibverbs/examples/rc_pingpong.c
@@ -208,14 +208,13 @@ static struct pingpong_dest *pp_client_exch_dest(const char *servername, int por
 		goto out;
 	}
 
-	if (read(sockfd, msg, sizeof msg) != sizeof msg) {
-		perror("client read");
-		fprintf(stderr, "Couldn't read remote address\n");
+	if (read(sockfd, msg, sizeof msg) != sizeof msg ||
+	    write(sockfd, "done", sizeof "done") != sizeof "done") {
+		perror("client read/write");
+		fprintf(stderr, "Couldn't read/write remote address\n");
 		goto out;
 	}
 
-	write(sockfd, "done", sizeof "done");
-
 	rem_dest = malloc(sizeof *rem_dest);
 	if (!rem_dest)
 		goto out;
@@ -316,14 +315,14 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
 	gid_to_wire_gid(&my_dest->gid, gid);
 	sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn,
 							my_dest->psn, gid);
-	if (write(connfd, msg, sizeof msg) != sizeof msg) {
-		fprintf(stderr, "Couldn't send local address\n");
+	if (write(connfd, msg, sizeof msg) != sizeof msg ||
+	    read(connfd, msg, sizeof msg) != sizeof msg) {
+		fprintf(stderr, "Couldn't send/recv local address\n");
 		free(rem_dest);
 		rem_dest = NULL;
 		goto out;
 	}
 
-	read(connfd, msg, sizeof msg);
 
 out:
 	close(connfd);
diff --git a/libibverbs/examples/srq_pingpong.c b/libibverbs/examples/srq_pingpong.c
index 929b736545c7..e6492dc553fd 100644
--- a/libibverbs/examples/srq_pingpong.c
+++ b/libibverbs/examples/srq_pingpong.c
@@ -222,8 +222,10 @@ static struct pingpong_dest *pp_client_exch_dest(const char *servername, int por
 		wire_gid_to_gid(gid, &rem_dest[i].gid);
 	}
 
-	write(sockfd, "done", sizeof "done");
-
+	if (write(sockfd, "done", sizeof "done") != sizeof "done") {
+		perror("client write");
+		goto out;
+	}
 out:
 	close(sockfd);
 	return rem_dest;
@@ -333,7 +335,12 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
 		}
 	}
 
-	read(connfd, msg, sizeof msg);
+	if (read(connfd, msg, sizeof msg) != sizeof msg) {
+		perror("client write");
+		free(rem_dest);
+		rem_dest = NULL;
+		goto out;
+	}
 
 out:
 	close(connfd);
diff --git a/libibverbs/examples/uc_pingpong.c b/libibverbs/examples/uc_pingpong.c
index 3802e3821773..d132de98694a 100644
--- a/libibverbs/examples/uc_pingpong.c
+++ b/libibverbs/examples/uc_pingpong.c
@@ -176,13 +176,13 @@ static struct pingpong_dest *pp_client_exch_dest(const char *servername, int por
 		goto out;
 	}
 
-	if (read(sockfd, msg, sizeof msg) != sizeof msg) {
-		perror("client read");
-		fprintf(stderr, "Couldn't read remote address\n");
+	if (read(sockfd, msg, sizeof msg) != sizeof msg ||
+	    write(sockfd, "done", sizeof "done") != sizeof "done") {
+		perror("client read/write");
+		fprintf(stderr, "Couldn't read/write remote address\n");
 		goto out;
 	}
 
-	write(sockfd, "done", sizeof "done");
 
 	rem_dest = malloc(sizeof *rem_dest);
 	if (!rem_dest)
@@ -284,15 +284,14 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
 	gid_to_wire_gid(&my_dest->gid, gid);
 	sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn,
 							my_dest->psn, gid);
-	if (write(connfd, msg, sizeof msg) != sizeof msg) {
-		fprintf(stderr, "Couldn't send local address\n");
+	if (write(connfd, msg, sizeof msg) != sizeof msg ||
+	    read(connfd, msg, sizeof msg) != sizeof msg) {
+		fprintf(stderr, "Couldn't send/recv local address\n");
 		free(rem_dest);
 		rem_dest = NULL;
 		goto out;
 	}
 
-	read(connfd, msg, sizeof msg);
-
 out:
 	close(connfd);
 	return rem_dest;
diff --git a/libibverbs/examples/ud_pingpong.c b/libibverbs/examples/ud_pingpong.c
index fa99b9e51dfb..67da4bd90f32 100644
--- a/libibverbs/examples/ud_pingpong.c
+++ b/libibverbs/examples/ud_pingpong.c
@@ -176,14 +176,13 @@ static struct pingpong_dest *pp_client_exch_dest(const char *servername, int por
 		goto out;
 	}
 
-	if (read(sockfd, msg, sizeof msg) != sizeof msg) {
-		perror("client read");
-		fprintf(stderr, "Couldn't read remote address\n");
+	if (read(sockfd, msg, sizeof msg) != sizeof msg ||
+	    write(sockfd, "done", sizeof "done") != sizeof "done") {
+		perror("client read/write");
+		fprintf(stderr, "Couldn't read/write remote address\n");
 		goto out;
 	}
 
-	write(sockfd, "done", sizeof "done");
-
 	rem_dest = malloc(sizeof *rem_dest);
 	if (!rem_dest)
 		goto out;
@@ -282,15 +281,13 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
 	gid_to_wire_gid(&my_dest->gid, gid);
 	sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn,
 							my_dest->psn, gid);
-	if (write(connfd, msg, sizeof msg) != sizeof msg) {
-		fprintf(stderr, "Couldn't send local address\n");
+	if (write(connfd, msg, sizeof msg) != sizeof msg ||
+	    read(connfd, msg, sizeof msg) != sizeof msg) {
+		fprintf(stderr, "Couldn't send/recv local address\n");
 		free(rem_dest);
 		rem_dest = NULL;
 		goto out;
 	}
-
-	read(connfd, msg, sizeof msg);
-
 out:
 	close(connfd);
 	return rem_dest;
diff --git a/libibverbs/src/neigh.c b/libibverbs/src/neigh.c
index 6b6e58cd52f8..5acfcf06fcde 100644
--- a/libibverbs/src/neigh.c
+++ b/libibverbs/src/neigh.c
@@ -19,6 +19,7 @@
 #include <unistd.h>
 #include <ifaddrs.h>
 #include <netdb.h>
+#include <assert.h>
 #ifndef _LINUX_IF_H
 #include <net/if.h>
 #else
@@ -372,9 +373,11 @@ static struct nl_addr *process_get_neigh_mac(
 
 			if (FD_ISSET(timer_fd, &fdset)) {
 				uint64_t read_val;
+				ssize_t rc;
 
-				(void)read(timer_fd, &read_val,
-					   sizeof(read_val));
+				rc =
+				    read(timer_fd, &read_val, sizeof(read_val));
+				assert(rc == sizeof(read_val));
 				if (++retries >=  NUM_OF_TRIES) {
 					if (!errno)
 						errno = EDESTADDRREQ;
diff --git a/librdmacm/src/acm.c b/librdmacm/src/acm.c
index f0da01e6d286..c097bb923b55 100644
--- a/librdmacm/src/acm.c
+++ b/librdmacm/src/acm.c
@@ -121,7 +121,8 @@ static int ucma_set_server_port(void)
 	FILE *f;
 
 	if ((f = fopen("/var/run/ibacm.port", "r" STREAM_CLOEXEC))) {
-		fscanf(f, "%" SCNu16, &server_port);
+		if (fscanf(f, "%" SCNu16, &server_port) != 1)
+			server_port = 0;
 		fclose(f);
 	}
 	return server_port;
diff --git a/librdmacm/src/rsocket.c b/librdmacm/src/rsocket.c
index 818505fbe02e..5645f40d2460 100644
--- a/librdmacm/src/rsocket.c
+++ b/librdmacm/src/rsocket.c
@@ -404,6 +404,20 @@ struct ds_udp_header {
 
 #define ds_next_qp(qp) container_of((qp)->list.next, struct ds_qp, list)
 
+static void write_all(int fd, const void *msg, size_t len)
+{
+	// FIXME: if fd is a socket this really needs to handle EINTR and other conditions.
+	ssize_t rc = write(fd, msg, len);
+	assert(rc == len);
+}
+
+static void read_all(int fd, void *msg, size_t len)
+{
+	// FIXME: if fd is a socket this really needs to handle EINTR and other conditions.
+	ssize_t rc = read(fd, msg, len);
+	assert(rc == len);
+}
+
 static void ds_insert_qp(struct rsocket *rs, struct ds_qp *qp)
 {
 	if (!rs->qp_list)
@@ -444,8 +458,8 @@ static int rs_notify_svc(struct rs_svc *svc, struct rsocket *rs, int cmd)
 	msg.cmd = cmd;
 	msg.status = EINVAL;
 	msg.rs = rs;
-	write(svc->sock[0], &msg, sizeof msg);
-	read(svc->sock[0], &msg, sizeof msg);
+	write_all(svc->sock[0], &msg, sizeof msg);
+	read_all(svc->sock[0], &msg, sizeof msg);
 	ret = rdma_seterrno(msg.status);
 	if (svc->cnt)
 		goto unlock;
@@ -484,6 +498,15 @@ static int rs_scale_to_value(int value, int bits)
 	       value : (value & ~(1 << (bits - 1))) << bits;
 }
 
+/* gcc > ~5 will not allow (void)fscanf to suppress -Wunused-result, but this
+   will do it.  In this case ignoring the result is OK (but horribly
+   unfriendly to user) since the library has a sane default. */
+#define failable_fscanf(f, fmt, ...)                                           \
+	{                                                                      \
+		int rc = fscanf(f, fmt, __VA_ARGS__);                          \
+		(void) rc;                                                     \
+	}
+
 void rs_configure(void)
 {
 	FILE *f;
@@ -501,27 +524,27 @@ void rs_configure(void)
 	ucma_ib_init();
 
 	if ((f = fopen(RS_CONF_DIR "/polling_time", "r"))) {
-		(void) fscanf(f, "%u", &polling_time);
+		failable_fscanf(f, "%u", &polling_time);
 		fclose(f);
 	}
 
 	if ((f = fopen(RS_CONF_DIR "/inline_default", "r"))) {
-		(void) fscanf(f, "%hu", &def_inline);
+		failable_fscanf(f, "%hu", &def_inline);
 		fclose(f);
 	}
 
 	if ((f = fopen(RS_CONF_DIR "/sqsize_default", "r"))) {
-		(void) fscanf(f, "%hu", &def_sqsize);
+		failable_fscanf(f, "%hu", &def_sqsize);
 		fclose(f);
 	}
 
 	if ((f = fopen(RS_CONF_DIR "/rqsize_default", "r"))) {
-		(void) fscanf(f, "%hu", &def_rqsize);
+		failable_fscanf(f, "%hu", &def_rqsize);
 		fclose(f);
 	}
 
 	if ((f = fopen(RS_CONF_DIR "/mem_default", "r"))) {
-		(void) fscanf(f, "%u", &def_mem);
+		failable_fscanf(f, "%u", &def_mem);
 		fclose(f);
 
 		if (def_mem < 1)
@@ -529,14 +552,14 @@ void rs_configure(void)
 	}
 
 	if ((f = fopen(RS_CONF_DIR "/wmem_default", "r"))) {
-		(void) fscanf(f, "%u", &def_wmem);
+		failable_fscanf(f, "%u", &def_wmem);
 		fclose(f);
 		if (def_wmem < RS_SNDLOWAT)
 			def_wmem = RS_SNDLOWAT << 1;
 	}
 
 	if ((f = fopen(RS_CONF_DIR "/iomap_size", "r"))) {
-		(void) fscanf(f, "%hu", &def_iomap_size);
+		failable_fscanf(f, "%hu", &def_iomap_size);
 		fclose(f);
 
 		/* round to supported values */
@@ -3345,7 +3368,8 @@ static int rs_set_keepalive(struct rsocket *rs, int on)
 	if (on) {
 		if (!rs->keepalive_time) {
 			if ((f = fopen("/proc/sys/net/ipv4/tcp_keepalive_time", "r"))) {
-				(void) fscanf(f, "%u", &rs->keepalive_time);
+				if (fscanf(f, "%u", &rs->keepalive_time) != 1)
+					rs->keepalive_time = 7200;
 				fclose(f);
 			} else {
 				rs->keepalive_time = 7200;
@@ -3985,7 +4009,7 @@ static void udp_svc_process_sock(struct rs_svc *svc)
 {
 	struct rs_svc_msg msg;
 
-	read(svc->sock[1], &msg, sizeof msg);
+	read_all(svc->sock[1], &msg, sizeof msg);
 	switch (msg.cmd) {
 	case RS_SVC_ADD_DGRAM:
 		msg.status = rs_svc_add_rs(svc, msg.rs);
@@ -4009,7 +4033,7 @@ static void udp_svc_process_sock(struct rs_svc *svc)
 		break;
 	}
 
-	write(svc->sock[1], &msg, sizeof msg);
+	write_all(svc->sock[1], &msg, sizeof msg);
 }
 
 static uint8_t udp_svc_sgid_index(struct ds_dest *dest, union ibv_gid *sgid)
@@ -4184,7 +4208,7 @@ static void *udp_svc_run(void *arg)
 	ret = rs_svc_grow_sets(svc, 4);
 	if (ret) {
 		msg.status = ret;
-		write(svc->sock[1], &msg, sizeof msg);
+		write_all(svc->sock[1], &msg, sizeof msg);
 		return (void *) (uintptr_t) ret;
 	}
 
@@ -4222,7 +4246,7 @@ static void tcp_svc_process_sock(struct rs_svc *svc)
 	struct rs_svc_msg msg;
 	int i;
 
-	read(svc->sock[1], &msg, sizeof msg);
+	read_all(svc->sock[1], &msg, sizeof msg);
 	switch (msg.cmd) {
 	case RS_SVC_ADD_KEEPALIVE:
 		msg.status = rs_svc_add_rs(svc, msg.rs);
@@ -4253,7 +4277,7 @@ static void tcp_svc_process_sock(struct rs_svc *svc)
 	default:
 		break;
 	}
-	write(svc->sock[1], &msg, sizeof msg);
+	write_all(svc->sock[1], &msg, sizeof msg);
 }
 
 /*
@@ -4282,7 +4306,7 @@ static void *tcp_svc_run(void *arg)
 	ret = rs_svc_grow_sets(svc, 16);
 	if (ret) {
 		msg.status = ret;
-		write(svc->sock[1], &msg, sizeof msg);
+		write_all(svc->sock[1], &msg, sizeof msg);
 		return (void *) (uintptr_t) ret;
 	}
 
-- 
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 26/28] Avoid gcc 5.4 warning -Wunused-result

[Hefty, Sean]

> It used to be you could suppress this with (void), however the gcc
> developers have decided to get rid of that.
> 
> So, look closely at each occurrence and decide what to do:
> - *pingpong: Join the error handling with the if statement directly
>    above
> - niegh: read on a timer_fd should never fail, so just use assert.
>    The assert is compiled out for Release builds so this is no-change
> - acm: Failure of ucma_set_server_port is detected by a 0 return
>    so check fscanf and return appropriately. This is no change since
>    fscanf failure was assumed to have left server_port as 0 (though
>    I doubt the standard supports that usage)
> - rsocket: This looks super sketchy. At least lets make the intent
> clear
>    with a read_all/write_all wrapper that calls assert. Most likely
>    this code is wrong..
>    Mangle the code with failable_fscanf to make it clear, but as with
>    acm, I don't think the standard supports this usage.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

For my parts

Acked-by: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

The rsocket code is sending a small message between a socketpair to notify a service thread that it needs to add a new rsocket to its processing.  It could probably check for a failure on the write calls and report that back to the caller.  If the read fails, I'm not sure what the service thread could do. 
--
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 26/28] Avoid gcc 5.4 warning -Wunused-result

[Jason Gunthorpe]

On Tue, Sep 13, 2016 at 09:58:56PM +0000, Hefty, Sean wrote:
> > It used to be you could suppress this with (void), however the gcc
> > developers have decided to get rid of that.
> > 
> > So, look closely at each occurrence and decide what to do:
> > - *pingpong: Join the error handling with the if statement directly
> >    above
> > - niegh: read on a timer_fd should never fail, so just use assert.
> >    The assert is compiled out for Release builds so this is no-change
> > - acm: Failure of ucma_set_server_port is detected by a 0 return
> >    so check fscanf and return appropriately. This is no change since
> >    fscanf failure was assumed to have left server_port as 0 (though
> >    I doubt the standard supports that usage)
> > - rsocket: This looks super sketchy. At least lets make the intent
> > clear
> >    with a read_all/write_all wrapper that calls assert. Most likely
> >    this code is wrong..
> >    Mangle the code with failable_fscanf to make it clear, but as with
> >    acm, I don't think the standard supports this usage.
> > 
> > Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> 
> For my parts
> 
> Acked-by: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Thanks

> The rsocket code is sending a small message between a socketpair to
> notify a service thread that it needs to add a new rsocket to its
> processing.  It could probably check for a failure on the write
> calls and report that back to the caller.  If the read fails, I'm
> not sure what the service thread could do.

In these situations I usually use close on the fd as a synchronous way
to ask the worker thread to exit..

But my comments about being 'wrong' are more to do with not handling
EINTR. If you are not reading from a file, and are doing blocking
reads, then you have to deal with it in the general library case.

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

Re: [PATCH 26/28] Avoid gcc 5.4 warning -Wunused-result

[Yishai Hadas]

On 9/6/2016 12:08 AM, Jason Gunthorpe wrote:
> It used to be you could suppress this with (void), however the gcc
> developers have decided to get rid of that.
>
> So, look closely at each occurrence and decide what to do:
> - *pingpong: Join the error handling with the if statement directly
>    above
> - niegh: read on a timer_fd should never fail, so just use assert.
>    The assert is compiled out for Release builds so this is no-change
> - acm: Failure of ucma_set_server_port is detected by a 0 return
>    so check fscanf and return appropriately. This is no change since
>    fscanf failure was assumed to have left server_port as 0 (though
>    I doubt the standard supports that usage)
> - rsocket: This looks super sketchy. At least lets make the intent clear
>    with a read_all/write_all wrapper that calls assert. Most likely
>    this code is wrong..
>    Mangle the code with failable_fscanf to make it clear, but as with
>    acm, I don't think the standard supports this usage.
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> ---
>  libibverbs/examples/rc_pingpong.c  | 15 +++++-----
>  libibverbs/examples/srq_pingpong.c | 13 +++++++--
>  libibverbs/examples/uc_pingpong.c  | 15 +++++-----
>  libibverbs/examples/ud_pingpong.c  | 17 +++++-------
>  libibverbs/src/neigh.c             |  7 +++--
>  librdmacm/src/acm.c                |  3 +-
>  librdmacm/src/rsocket.c            | 56 +++++++++++++++++++++++++++-----------
>  7 files changed, 78 insertions(+), 48 deletions(-)
>
> diff --git a/libibverbs/examples/rc_pingpong.c b/libibverbs/examples/rc_pingpong.c
> index 1fad16a0be7c..7bcc413a0f1d 100644
> --- a/libibverbs/examples/rc_pingpong.c
> +++ b/libibverbs/examples/rc_pingpong.c
> @@ -208,14 +208,13 @@ static struct pingpong_dest *pp_client_exch_dest(const char *servername, int por
>  		goto out;
>  	}
>
> -	if (read(sockfd, msg, sizeof msg) != sizeof msg) {
> -		perror("client read");
> -		fprintf(stderr, "Couldn't read remote address\n");
> +	if (read(sockfd, msg, sizeof msg) != sizeof msg ||
> +	    write(sockfd, "done", sizeof "done") != sizeof "done") {
> +		perror("client read/write");
> +		fprintf(stderr, "Couldn't read/write remote address\n");

It's just an example, however, better to have an accurate error message 
and separate into 2 stubs.

>  		goto out;
>  	}
>
> -	write(sockfd, "done", sizeof "done");
> -
>  	rem_dest = malloc(sizeof *rem_dest);
>  	if (!rem_dest)
>  		goto out;
> @@ -316,14 +315,14 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
>  	gid_to_wire_gid(&my_dest->gid, gid);
>  	sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn,
>  							my_dest->psn, gid);
> -	if (write(connfd, msg, sizeof msg) != sizeof msg) {
> -		fprintf(stderr, "Couldn't send local address\n");
> +	if (write(connfd, msg, sizeof msg) != sizeof msg ||
> +	    read(connfd, msg, sizeof msg) != sizeof msg) {
> +		fprintf(stderr, "Couldn't send/recv local address\n");

Same comment as of above.

>  		free(rem_dest);
>  		rem_dest = NULL;
>  		goto out;
>  	}
>
> -	read(connfd, msg, sizeof msg);
>
>  out:
>  	close(connfd);
> diff --git a/libibverbs/examples/srq_pingpong.c b/libibverbs/examples/srq_pingpong.c
> index 929b736545c7..e6492dc553fd 100644
> --- a/libibverbs/examples/srq_pingpong.c
> +++ b/libibverbs/examples/srq_pingpong.c
> @@ -222,8 +222,10 @@ static struct pingpong_dest *pp_client_exch_dest(const char *servername, int por
>  		wire_gid_to_gid(gid, &rem_dest[i].gid);
>  	}
>
> -	write(sockfd, "done", sizeof "done");
> -
> +	if (write(sockfd, "done", sizeof "done") != sizeof "done") {
> +		perror("client write");
> +		goto out;
> +	}
>  out:
>  	close(sockfd);
>  	return rem_dest;
> @@ -333,7 +335,12 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
>  		}
>  	}
>
> -	read(connfd, msg, sizeof msg);
> +	if (read(connfd, msg, sizeof msg) != sizeof msg) {
> +		perror("client write");
> +		free(rem_dest);
> +		rem_dest = NULL;
> +		goto out;
> +	}
>
>  out:
>  	close(connfd);
> diff --git a/libibverbs/examples/uc_pingpong.c b/libibverbs/examples/uc_pingpong.c
> index 3802e3821773..d132de98694a 100644
> --- a/libibverbs/examples/uc_pingpong.c
> +++ b/libibverbs/examples/uc_pingpong.c
> @@ -176,13 +176,13 @@ static struct pingpong_dest *pp_client_exch_dest(const char *servername, int por
>  		goto out;
>  	}
>
> -	if (read(sockfd, msg, sizeof msg) != sizeof msg) {
> -		perror("client read");
> -		fprintf(stderr, "Couldn't read remote address\n");
> +	if (read(sockfd, msg, sizeof msg) != sizeof msg ||
> +	    write(sockfd, "done", sizeof "done") != sizeof "done") {
> +		perror("client read/write");
> +		fprintf(stderr, "Couldn't read/write remote address\n");
>  		goto out;
>  	}
>
> -	write(sockfd, "done", sizeof "done");
>
>  	rem_dest = malloc(sizeof *rem_dest);
>  	if (!rem_dest)
> @@ -284,15 +284,14 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
>  	gid_to_wire_gid(&my_dest->gid, gid);
>  	sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn,
>  							my_dest->psn, gid);
> -	if (write(connfd, msg, sizeof msg) != sizeof msg) {
> -		fprintf(stderr, "Couldn't send local address\n");
> +	if (write(connfd, msg, sizeof msg) != sizeof msg ||
> +	    read(connfd, msg, sizeof msg) != sizeof msg) {
> +		fprintf(stderr, "Couldn't send/recv local address\n");
>  		free(rem_dest);
>  		rem_dest = NULL;
>  		goto out;
>  	}
>
> -	read(connfd, msg, sizeof msg);
> -
>  out:
>  	close(connfd);
>  	return rem_dest;
> diff --git a/libibverbs/examples/ud_pingpong.c b/libibverbs/examples/ud_pingpong.c
> index fa99b9e51dfb..67da4bd90f32 100644
> --- a/libibverbs/examples/ud_pingpong.c
> +++ b/libibverbs/examples/ud_pingpong.c
> @@ -176,14 +176,13 @@ static struct pingpong_dest *pp_client_exch_dest(const char *servername, int por
>  		goto out;
>  	}
>
> -	if (read(sockfd, msg, sizeof msg) != sizeof msg) {
> -		perror("client read");
> -		fprintf(stderr, "Couldn't read remote address\n");
> +	if (read(sockfd, msg, sizeof msg) != sizeof msg ||
> +	    write(sockfd, "done", sizeof "done") != sizeof "done") {
> +		perror("client read/write");
> +		fprintf(stderr, "Couldn't read/write remote address\n");
>  		goto out;
>  	}
>
> -	write(sockfd, "done", sizeof "done");
> -
>  	rem_dest = malloc(sizeof *rem_dest);
>  	if (!rem_dest)
>  		goto out;
> @@ -282,15 +281,13 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
>  	gid_to_wire_gid(&my_dest->gid, gid);
>  	sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn,
>  							my_dest->psn, gid);
> -	if (write(connfd, msg, sizeof msg) != sizeof msg) {
> -		fprintf(stderr, "Couldn't send local address\n");
> +	if (write(connfd, msg, sizeof msg) != sizeof msg ||
> +	    read(connfd, msg, sizeof msg) != sizeof msg) {
> +		fprintf(stderr, "Couldn't send/recv local address\n");
>  		free(rem_dest);
>  		rem_dest = NULL;
>  		goto out;
>  	}
> -
> -	read(connfd, msg, sizeof msg);
> -
>  out:
>  	close(connfd);
>  	return rem_dest;
> diff --git a/libibverbs/src/neigh.c b/libibverbs/src/neigh.c
> index 6b6e58cd52f8..5acfcf06fcde 100644
> --- a/libibverbs/src/neigh.c
> +++ b/libibverbs/src/neigh.c
> @@ -19,6 +19,7 @@
>  #include <unistd.h>
>  #include <ifaddrs.h>
>  #include <netdb.h>
> +#include <assert.h>
>  #ifndef _LINUX_IF_H
>  #include <net/if.h>
>  #else
> @@ -372,9 +373,11 @@ static struct nl_addr *process_get_neigh_mac(
>
>  			if (FD_ISSET(timer_fd, &fdset)) {
>  				uint64_t read_val;
> +				ssize_t rc;
>
> -				(void)read(timer_fd, &read_val,
> -					   sizeof(read_val));
> +				rc =
> +				    read(timer_fd, &read_val, sizeof(read_val));
> +				assert(rc == sizeof(read_val));
>  				if (++retries >=  NUM_OF_TRIES) {
>  					if (!errno)
>  						errno = EDESTADDRREQ;
> diff --git a/librdmacm/src/acm.c b/librdmacm/src/acm.c
> index f0da01e6d286..c097bb923b55 100644
> --- a/librdmacm/src/acm.c
> +++ b/librdmacm/src/acm.c
> @@ -121,7 +121,8 @@ static int ucma_set_server_port(void)
>  	FILE *f;
>
>  	if ((f = fopen("/var/run/ibacm.port", "r" STREAM_CLOEXEC))) {
> -		fscanf(f, "%" SCNu16, &server_port);
> +		if (fscanf(f, "%" SCNu16, &server_port) != 1)
> +			server_port = 0;
>  		fclose(f);
>  	}
>  	return server_port;
> diff --git a/librdmacm/src/rsocket.c b/librdmacm/src/rsocket.c
> index 818505fbe02e..5645f40d2460 100644
> --- a/librdmacm/src/rsocket.c
> +++ b/librdmacm/src/rsocket.c
> @@ -404,6 +404,20 @@ struct ds_udp_header {
>
>  #define ds_next_qp(qp) container_of((qp)->list.next, struct ds_qp, list)
>
> +static void write_all(int fd, const void *msg, size_t len)
> +{
> +	// FIXME: if fd is a socket this really needs to handle EINTR and other conditions.
> +	ssize_t rc = write(fd, msg, len);
> +	assert(rc == len);
> +}
> +
> +static void read_all(int fd, void *msg, size_t len)
> +{
> +	// FIXME: if fd is a socket this really needs to handle EINTR and other conditions.
> +	ssize_t rc = read(fd, msg, len);
> +	assert(rc == len);
> +}
> +
>  static void ds_insert_qp(struct rsocket *rs, struct ds_qp *qp)
>  {
>  	if (!rs->qp_list)
> @@ -444,8 +458,8 @@ static int rs_notify_svc(struct rs_svc *svc, struct rsocket *rs, int cmd)
>  	msg.cmd = cmd;
>  	msg.status = EINVAL;
>  	msg.rs = rs;
> -	write(svc->sock[0], &msg, sizeof msg);
> -	read(svc->sock[0], &msg, sizeof msg);
> +	write_all(svc->sock[0], &msg, sizeof msg);
> +	read_all(svc->sock[0], &msg, sizeof msg);
>  	ret = rdma_seterrno(msg.status);
>  	if (svc->cnt)
>  		goto unlock;
> @@ -484,6 +498,15 @@ static int rs_scale_to_value(int value, int bits)
>  	       value : (value & ~(1 << (bits - 1))) << bits;
>  }
>
> +/* gcc > ~5 will not allow (void)fscanf to suppress -Wunused-result, but this
> +   will do it.  In this case ignoring the result is OK (but horribly
> +   unfriendly to user) since the library has a sane default. */
> +#define failable_fscanf(f, fmt, ...)                                           \
> +	{                                                                      \
> +		int rc = fscanf(f, fmt, __VA_ARGS__);                          \
> +		(void) rc;                                                     \
> +	}
> +
>  void rs_configure(void)
>  {
>  	FILE *f;
> @@ -501,27 +524,27 @@ void rs_configure(void)
>  	ucma_ib_init();
>
>  	if ((f = fopen(RS_CONF_DIR "/polling_time", "r"))) {
> -		(void) fscanf(f, "%u", &polling_time);
> +		failable_fscanf(f, "%u", &polling_time);
>  		fclose(f);
>  	}
>
>  	if ((f = fopen(RS_CONF_DIR "/inline_default", "r"))) {
> -		(void) fscanf(f, "%hu", &def_inline);
> +		failable_fscanf(f, "%hu", &def_inline);
>  		fclose(f);
>  	}
>
>  	if ((f = fopen(RS_CONF_DIR "/sqsize_default", "r"))) {
> -		(void) fscanf(f, "%hu", &def_sqsize);
> +		failable_fscanf(f, "%hu", &def_sqsize);
>  		fclose(f);
>  	}
>
>  	if ((f = fopen(RS_CONF_DIR "/rqsize_default", "r"))) {
> -		(void) fscanf(f, "%hu", &def_rqsize);
> +		failable_fscanf(f, "%hu", &def_rqsize);
>  		fclose(f);
>  	}
>
>  	if ((f = fopen(RS_CONF_DIR "/mem_default", "r"))) {
> -		(void) fscanf(f, "%u", &def_mem);
> +		failable_fscanf(f, "%u", &def_mem);
>  		fclose(f);
>
>  		if (def_mem < 1)
> @@ -529,14 +552,14 @@ void rs_configure(void)
>  	}
>
>  	if ((f = fopen(RS_CONF_DIR "/wmem_default", "r"))) {
> -		(void) fscanf(f, "%u", &def_wmem);
> +		failable_fscanf(f, "%u", &def_wmem);
>  		fclose(f);
>  		if (def_wmem < RS_SNDLOWAT)
>  			def_wmem = RS_SNDLOWAT << 1;
>  	}
>
>  	if ((f = fopen(RS_CONF_DIR "/iomap_size", "r"))) {
> -		(void) fscanf(f, "%hu", &def_iomap_size);
> +		failable_fscanf(f, "%hu", &def_iomap_size);
>  		fclose(f);
>
>  		/* round to supported values */
> @@ -3345,7 +3368,8 @@ static int rs_set_keepalive(struct rsocket *rs, int on)
>  	if (on) {
>  		if (!rs->keepalive_time) {
>  			if ((f = fopen("/proc/sys/net/ipv4/tcp_keepalive_time", "r"))) {
> -				(void) fscanf(f, "%u", &rs->keepalive_time);
> +				if (fscanf(f, "%u", &rs->keepalive_time) != 1)
> +					rs->keepalive_time = 7200;
>  				fclose(f);
>  			} else {
>  				rs->keepalive_time = 7200;
> @@ -3985,7 +4009,7 @@ static void udp_svc_process_sock(struct rs_svc *svc)
>  {
>  	struct rs_svc_msg msg;
>
> -	read(svc->sock[1], &msg, sizeof msg);
> +	read_all(svc->sock[1], &msg, sizeof msg);
>  	switch (msg.cmd) {
>  	case RS_SVC_ADD_DGRAM:
>  		msg.status = rs_svc_add_rs(svc, msg.rs);
> @@ -4009,7 +4033,7 @@ static void udp_svc_process_sock(struct rs_svc *svc)
>  		break;
>  	}
>
> -	write(svc->sock[1], &msg, sizeof msg);
> +	write_all(svc->sock[1], &msg, sizeof msg);
>  }
>
>  static uint8_t udp_svc_sgid_index(struct ds_dest *dest, union ibv_gid *sgid)
> @@ -4184,7 +4208,7 @@ static void *udp_svc_run(void *arg)
>  	ret = rs_svc_grow_sets(svc, 4);
>  	if (ret) {
>  		msg.status = ret;
> -		write(svc->sock[1], &msg, sizeof msg);
> +		write_all(svc->sock[1], &msg, sizeof msg);
>  		return (void *) (uintptr_t) ret;
>  	}
>
> @@ -4222,7 +4246,7 @@ static void tcp_svc_process_sock(struct rs_svc *svc)
>  	struct rs_svc_msg msg;
>  	int i;
>
> -	read(svc->sock[1], &msg, sizeof msg);
> +	read_all(svc->sock[1], &msg, sizeof msg);
>  	switch (msg.cmd) {
>  	case RS_SVC_ADD_KEEPALIVE:
>  		msg.status = rs_svc_add_rs(svc, msg.rs);
> @@ -4253,7 +4277,7 @@ static void tcp_svc_process_sock(struct rs_svc *svc)
>  	default:
>  		break;
>  	}
> -	write(svc->sock[1], &msg, sizeof msg);
> +	write_all(svc->sock[1], &msg, sizeof msg);
>  }
>
>  /*
> @@ -4282,7 +4306,7 @@ static void *tcp_svc_run(void *arg)
>  	ret = rs_svc_grow_sets(svc, 16);
>  	if (ret) {
>  		msg.status = ret;
> -		write(svc->sock[1], &msg, sizeof msg);
> +		write_all(svc->sock[1], &msg, sizeof msg);
>  		return (void *) (uintptr_t) ret;
>  	}
>
>

--
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 26/28] Avoid gcc 5.4 warning -Wunused-result

[Jason Gunthorpe]

On Wed, Sep 14, 2016 at 07:35:42PM +0300, Yishai Hadas wrote:
> >+	if (read(sockfd, msg, sizeof msg) != sizeof msg ||
> >+	    write(sockfd, "done", sizeof "done") != sizeof "done") {
> >+		perror("client read/write");
> >+		fprintf(stderr, "Couldn't read/write remote address\n");
> 
> It's just an example, however, better to have an accurate error message and
> separate into 2 stubs.

There is no value to the user in distinguishing the two cases. They
both mean the socket closed unexpectedly.

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

Re: [PATCH 26/28] Avoid gcc 5.4 warning -Wunused-result

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1012 bytes --]

On Wed, Sep 14, 2016 at 10:58:30AM -0600, Jason Gunthorpe wrote:
> On Wed, Sep 14, 2016 at 07:35:42PM +0300, Yishai Hadas wrote:
> > >+	if (read(sockfd, msg, sizeof msg) != sizeof msg ||
> > >+	    write(sockfd, "done", sizeof "done") != sizeof "done") {
> > >+		perror("client read/write");
> > >+		fprintf(stderr, "Couldn't read/write remote address\n");
> >
> > It's just an example, however, better to have an accurate error message and
> > separate into 2 stubs.
>
> There is no value to the user in distinguishing the two cases. They
> both mean the socket closed unexpectedly.

Agree, it will give nothing to the user. We are all treating pingpong
applications very similar - works/doesn't work, while "doesn't work"
actually says that something very basic is broken.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 26/28] Avoid gcc 5.4 warning -Wunused-result

[Yishai Hadas]

On 9/6/2016 12:08 AM, Jason Gunthorpe wrote:
> It used to be you could suppress this with (void), however the gcc
> developers have decided to get rid of that.
>
> So, look closely at each occurrence and decide what to do:
> - *pingpong: Join the error handling with the if statement directly
>    above
> - niegh: read on a timer_fd should never fail, so just use assert.
>    The assert is compiled out for Release builds so this is no-change
> - acm: Failure of ucma_set_server_port is detected by a 0 return
>    so check fscanf and return appropriately. This is no change since
>    fscanf failure was assumed to have left server_port as 0 (though
>    I doubt the standard supports that usage)
> - rsocket: This looks super sketchy. At least lets make the intent clear
>    with a read_all/write_all wrapper that calls assert. Most likely
>    this code is wrong..
>    Mangle the code with failable_fscanf to make it clear, but as with
>    acm, I don't think the standard supports this usage.
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>


This patch breaks rc/uc/ud/srq pingpong in a basic run, please see below.

Can you please prepare some fix for that as it was already merged ?

> ---
>  libibverbs/examples/rc_pingpong.c  | 15 +++++-----
>  libibverbs/examples/srq_pingpong.c | 13 +++++++--
>  libibverbs/examples/uc_pingpong.c  | 15 +++++-----
>  libibverbs/examples/ud_pingpong.c  | 17 +++++-------
>  libibverbs/src/neigh.c             |  7 +++--
>  librdmacm/src/acm.c                |  3 +-
>  librdmacm/src/rsocket.c            | 56 +++++++++++++++++++++++++++-----------
>  7 files changed, 78 insertions(+), 48 deletions(-)
>
> diff --git a/libibverbs/examples/rc_pingpong.c b/libibverbs/examples/rc_pingpong.c
> index 1fad16a0be7c..7bcc413a0f1d 100644
> --- a/libibverbs/examples/rc_pingpong.c
> +++ b/libibverbs/examples/rc_pingpong.c
> @@ -208,14 +208,13 @@ static struct pingpong_dest *pp_client_exch_dest(const char *servername, int por
>  		goto out;
>  	}
>
> -	if (read(sockfd, msg, sizeof msg) != sizeof msg) {
> -		perror("client read");
> -		fprintf(stderr, "Couldn't read remote address\n");
> +	if (read(sockfd, msg, sizeof msg) != sizeof msg ||
> +	    write(sockfd, "done", sizeof "done") != sizeof "done") {
> +		perror("client read/write");
> +		fprintf(stderr, "Couldn't read/write remote address\n");
>  		goto out;
>  	}
>
> -	write(sockfd, "done", sizeof "done");
> -
>  	rem_dest = malloc(sizeof *rem_dest);
>  	if (!rem_dest)
>  		goto out;
> @@ -316,14 +315,14 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
>  	gid_to_wire_gid(&my_dest->gid, gid);
>  	sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn,
>  							my_dest->psn, gid);
> -	if (write(connfd, msg, sizeof msg) != sizeof msg) {
> -		fprintf(stderr, "Couldn't send local address\n");
> +	if (write(connfd, msg, sizeof msg) != sizeof msg ||
> +	    read(connfd, msg, sizeof msg) != sizeof msg) {
> +		fprintf(stderr, "Couldn't send/recv local address\n");

At that step the server expects to read a "done" response from its 
client, checking whether the read was done for sizeof msg which is much 
larger will fail.

Same issue appears below in several places in srq/ud/uc pingpong.

>  		free(rem_dest);
>  		rem_dest = NULL;
>  		goto out;
>  	}
>
> -	read(connfd, msg, sizeof msg);
>
>  out:
>  	close(connfd);
> diff --git a/libibverbs/examples/srq_pingpong.c b/libibverbs/examples/srq_pingpong.c
> index 929b736545c7..e6492dc553fd 100644
> --- a/libibverbs/examples/srq_pingpong.c
> +++ b/libibverbs/examples/srq_pingpong.c
> @@ -222,8 +222,10 @@ static struct pingpong_dest *pp_client_exch_dest(const char *servername, int por
>  		wire_gid_to_gid(gid, &rem_dest[i].gid);
>  	}
>
> -	write(sockfd, "done", sizeof "done");
> -
> +	if (write(sockfd, "done", sizeof "done") != sizeof "done") {
> +		perror("client write");
> +		goto out;
> +	}
>  out:
>  	close(sockfd);
>  	return rem_dest;
> @@ -333,7 +335,12 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
>  		}
>  	}
>
> -	read(connfd, msg, sizeof msg);
> +	if (read(connfd, msg, sizeof msg) != sizeof msg) {
> +		perror("client write");
> +		free(rem_dest);
> +		rem_dest = NULL;
> +		goto out;
> +	}
>
>  out:
>  	close(connfd);
> diff --git a/libibverbs/examples/uc_pingpong.c b/libibverbs/examples/uc_pingpong.c
> index 3802e3821773..d132de98694a 100644
> --- a/libibverbs/examples/uc_pingpong.c
> +++ b/libibverbs/examples/uc_pingpong.c
> @@ -176,13 +176,13 @@ static struct pingpong_dest *pp_client_exch_dest(const char *servername, int por
>  		goto out;
>  	}
>
> -	if (read(sockfd, msg, sizeof msg) != sizeof msg) {
> -		perror("client read");
> -		fprintf(stderr, "Couldn't read remote address\n");
> +	if (read(sockfd, msg, sizeof msg) != sizeof msg ||
> +	    write(sockfd, "done", sizeof "done") != sizeof "done") {
> +		perror("client read/write");
> +		fprintf(stderr, "Couldn't read/write remote address\n");
>  		goto out;
>  	}
>
> -	write(sockfd, "done", sizeof "done");
>
>  	rem_dest = malloc(sizeof *rem_dest);
>  	if (!rem_dest)
> @@ -284,15 +284,14 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
>  	gid_to_wire_gid(&my_dest->gid, gid);
>  	sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn,
>  							my_dest->psn, gid);
> -	if (write(connfd, msg, sizeof msg) != sizeof msg) {
> -		fprintf(stderr, "Couldn't send local address\n");
> +	if (write(connfd, msg, sizeof msg) != sizeof msg ||
> +	    read(connfd, msg, sizeof msg) != sizeof msg) {
> +		fprintf(stderr, "Couldn't send/recv local address\n");
>  		free(rem_dest);
>  		rem_dest = NULL;
>  		goto out;
>  	}
>
> -	read(connfd, msg, sizeof msg);
> -
>  out:
>  	close(connfd);
>  	return rem_dest;
> diff --git a/libibverbs/examples/ud_pingpong.c b/libibverbs/examples/ud_pingpong.c
> index fa99b9e51dfb..67da4bd90f32 100644
> --- a/libibverbs/examples/ud_pingpong.c
> +++ b/libibverbs/examples/ud_pingpong.c
> @@ -176,14 +176,13 @@ static struct pingpong_dest *pp_client_exch_dest(const char *servername, int por
>  		goto out;
>  	}
>
> -	if (read(sockfd, msg, sizeof msg) != sizeof msg) {
> -		perror("client read");
> -		fprintf(stderr, "Couldn't read remote address\n");
> +	if (read(sockfd, msg, sizeof msg) != sizeof msg ||
> +	    write(sockfd, "done", sizeof "done") != sizeof "done") {
> +		perror("client read/write");
> +		fprintf(stderr, "Couldn't read/write remote address\n");
>  		goto out;
>  	}
>
> -	write(sockfd, "done", sizeof "done");
> -
>  	rem_dest = malloc(sizeof *rem_dest);
>  	if (!rem_dest)
>  		goto out;
> @@ -282,15 +281,13 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
>  	gid_to_wire_gid(&my_dest->gid, gid);
>  	sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn,
>  							my_dest->psn, gid);
> -	if (write(connfd, msg, sizeof msg) != sizeof msg) {
> -		fprintf(stderr, "Couldn't send local address\n");
> +	if (write(connfd, msg, sizeof msg) != sizeof msg ||
> +	    read(connfd, msg, sizeof msg) != sizeof msg) {
> +		fprintf(stderr, "Couldn't send/recv local address\n");
>  		free(rem_dest);
>  		rem_dest = NULL;
>  		goto out;
>  	}
> -
> -	read(connfd, msg, sizeof msg);
> -
>  out:
>  	close(connfd);
>  	return rem_dest;
> diff --git a/libibverbs/src/neigh.c b/libibverbs/src/neigh.c
> index 6b6e58cd52f8..5acfcf06fcde 100644
> --- a/libibverbs/src/neigh.c
> +++ b/libibverbs/src/neigh.c
> @@ -19,6 +19,7 @@
>  #include <unistd.h>
>  #include <ifaddrs.h>
>  #include <netdb.h>
> +#include <assert.h>
>  #ifndef _LINUX_IF_H
>  #include <net/if.h>
>  #else
> @@ -372,9 +373,11 @@ static struct nl_addr *process_get_neigh_mac(
>
>  			if (FD_ISSET(timer_fd, &fdset)) {
>  				uint64_t read_val;
> +				ssize_t rc;
>
> -				(void)read(timer_fd, &read_val,
> -					   sizeof(read_val));
> +				rc =
> +				    read(timer_fd, &read_val, sizeof(read_val));
> +				assert(rc == sizeof(read_val));
>  				if (++retries >=  NUM_OF_TRIES) {
>  					if (!errno)
>  						errno = EDESTADDRREQ;
> diff --git a/librdmacm/src/acm.c b/librdmacm/src/acm.c
> index f0da01e6d286..c097bb923b55 100644
> --- a/librdmacm/src/acm.c
> +++ b/librdmacm/src/acm.c
> @@ -121,7 +121,8 @@ static int ucma_set_server_port(void)
>  	FILE *f;
>
>  	if ((f = fopen("/var/run/ibacm.port", "r" STREAM_CLOEXEC))) {
> -		fscanf(f, "%" SCNu16, &server_port);
> +		if (fscanf(f, "%" SCNu16, &server_port) != 1)
> +			server_port = 0;
>  		fclose(f);
>  	}
>  	return server_port;
> diff --git a/librdmacm/src/rsocket.c b/librdmacm/src/rsocket.c
> index 818505fbe02e..5645f40d2460 100644
> --- a/librdmacm/src/rsocket.c
> +++ b/librdmacm/src/rsocket.c
> @@ -404,6 +404,20 @@ struct ds_udp_header {
>
>  #define ds_next_qp(qp) container_of((qp)->list.next, struct ds_qp, list)
>
> +static void write_all(int fd, const void *msg, size_t len)
> +{
> +	// FIXME: if fd is a socket this really needs to handle EINTR and other conditions.
> +	ssize_t rc = write(fd, msg, len);
> +	assert(rc == len);
> +}
> +
> +static void read_all(int fd, void *msg, size_t len)
> +{
> +	// FIXME: if fd is a socket this really needs to handle EINTR and other conditions.
> +	ssize_t rc = read(fd, msg, len);
> +	assert(rc == len);
> +}
> +
>  static void ds_insert_qp(struct rsocket *rs, struct ds_qp *qp)
>  {
>  	if (!rs->qp_list)
> @@ -444,8 +458,8 @@ static int rs_notify_svc(struct rs_svc *svc, struct rsocket *rs, int cmd)
>  	msg.cmd = cmd;
>  	msg.status = EINVAL;
>  	msg.rs = rs;
> -	write(svc->sock[0], &msg, sizeof msg);
> -	read(svc->sock[0], &msg, sizeof msg);
> +	write_all(svc->sock[0], &msg, sizeof msg);
> +	read_all(svc->sock[0], &msg, sizeof msg);
>  	ret = rdma_seterrno(msg.status);
>  	if (svc->cnt)
>  		goto unlock;
> @@ -484,6 +498,15 @@ static int rs_scale_to_value(int value, int bits)
>  	       value : (value & ~(1 << (bits - 1))) << bits;
>  }
>
> +/* gcc > ~5 will not allow (void)fscanf to suppress -Wunused-result, but this
> +   will do it.  In this case ignoring the result is OK (but horribly
> +   unfriendly to user) since the library has a sane default. */
> +#define failable_fscanf(f, fmt, ...)                                           \
> +	{                                                                      \
> +		int rc = fscanf(f, fmt, __VA_ARGS__);                          \
> +		(void) rc;                                                     \
> +	}
> +
>  void rs_configure(void)
>  {
>  	FILE *f;
> @@ -501,27 +524,27 @@ void rs_configure(void)
>  	ucma_ib_init();
>
>  	if ((f = fopen(RS_CONF_DIR "/polling_time", "r"))) {
> -		(void) fscanf(f, "%u", &polling_time);
> +		failable_fscanf(f, "%u", &polling_time);
>  		fclose(f);
>  	}
>
>  	if ((f = fopen(RS_CONF_DIR "/inline_default", "r"))) {
> -		(void) fscanf(f, "%hu", &def_inline);
> +		failable_fscanf(f, "%hu", &def_inline);
>  		fclose(f);
>  	}
>
>  	if ((f = fopen(RS_CONF_DIR "/sqsize_default", "r"))) {
> -		(void) fscanf(f, "%hu", &def_sqsize);
> +		failable_fscanf(f, "%hu", &def_sqsize);
>  		fclose(f);
>  	}
>
>  	if ((f = fopen(RS_CONF_DIR "/rqsize_default", "r"))) {
> -		(void) fscanf(f, "%hu", &def_rqsize);
> +		failable_fscanf(f, "%hu", &def_rqsize);
>  		fclose(f);
>  	}
>
>  	if ((f = fopen(RS_CONF_DIR "/mem_default", "r"))) {
> -		(void) fscanf(f, "%u", &def_mem);
> +		failable_fscanf(f, "%u", &def_mem);
>  		fclose(f);
>
>  		if (def_mem < 1)
> @@ -529,14 +552,14 @@ void rs_configure(void)
>  	}
>
>  	if ((f = fopen(RS_CONF_DIR "/wmem_default", "r"))) {
> -		(void) fscanf(f, "%u", &def_wmem);
> +		failable_fscanf(f, "%u", &def_wmem);
>  		fclose(f);
>  		if (def_wmem < RS_SNDLOWAT)
>  			def_wmem = RS_SNDLOWAT << 1;
>  	}
>
>  	if ((f = fopen(RS_CONF_DIR "/iomap_size", "r"))) {
> -		(void) fscanf(f, "%hu", &def_iomap_size);
> +		failable_fscanf(f, "%hu", &def_iomap_size);
>  		fclose(f);
>
>  		/* round to supported values */
> @@ -3345,7 +3368,8 @@ static int rs_set_keepalive(struct rsocket *rs, int on)
>  	if (on) {
>  		if (!rs->keepalive_time) {
>  			if ((f = fopen("/proc/sys/net/ipv4/tcp_keepalive_time", "r"))) {
> -				(void) fscanf(f, "%u", &rs->keepalive_time);
> +				if (fscanf(f, "%u", &rs->keepalive_time) != 1)
> +					rs->keepalive_time = 7200;
>  				fclose(f);
>  			} else {
>  				rs->keepalive_time = 7200;
> @@ -3985,7 +4009,7 @@ static void udp_svc_process_sock(struct rs_svc *svc)
>  {
>  	struct rs_svc_msg msg;
>
> -	read(svc->sock[1], &msg, sizeof msg);
> +	read_all(svc->sock[1], &msg, sizeof msg);
>  	switch (msg.cmd) {
>  	case RS_SVC_ADD_DGRAM:
>  		msg.status = rs_svc_add_rs(svc, msg.rs);
> @@ -4009,7 +4033,7 @@ static void udp_svc_process_sock(struct rs_svc *svc)
>  		break;
>  	}
>
> -	write(svc->sock[1], &msg, sizeof msg);
> +	write_all(svc->sock[1], &msg, sizeof msg);
>  }
>
>  static uint8_t udp_svc_sgid_index(struct ds_dest *dest, union ibv_gid *sgid)
> @@ -4184,7 +4208,7 @@ static void *udp_svc_run(void *arg)
>  	ret = rs_svc_grow_sets(svc, 4);
>  	if (ret) {
>  		msg.status = ret;
> -		write(svc->sock[1], &msg, sizeof msg);
> +		write_all(svc->sock[1], &msg, sizeof msg);
>  		return (void *) (uintptr_t) ret;
>  	}
>
> @@ -4222,7 +4246,7 @@ static void tcp_svc_process_sock(struct rs_svc *svc)
>  	struct rs_svc_msg msg;
>  	int i;
>
> -	read(svc->sock[1], &msg, sizeof msg);
> +	read_all(svc->sock[1], &msg, sizeof msg);
>  	switch (msg.cmd) {
>  	case RS_SVC_ADD_KEEPALIVE:
>  		msg.status = rs_svc_add_rs(svc, msg.rs);
> @@ -4253,7 +4277,7 @@ static void tcp_svc_process_sock(struct rs_svc *svc)
>  	default:
>  		break;
>  	}
> -	write(svc->sock[1], &msg, sizeof msg);
> +	write_all(svc->sock[1], &msg, sizeof msg);
>  }
>
>  /*
> @@ -4282,7 +4306,7 @@ static void *tcp_svc_run(void *arg)
>  	ret = rs_svc_grow_sets(svc, 16);
>  	if (ret) {
>  		msg.status = ret;
> -		write(svc->sock[1], &msg, sizeof msg);
> +		write_all(svc->sock[1], &msg, sizeof msg);
>  		return (void *) (uintptr_t) ret;
>  	}
>
>

--
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 26/28] Avoid gcc 5.4 warning -Wunused-result

[Jason Gunthorpe]

On Sun, Oct 09, 2016 at 04:31:17PM +0300, Yishai Hadas wrote:

> >-	if (write(connfd, msg, sizeof msg) != sizeof msg) {
> >-		fprintf(stderr, "Couldn't send local address\n");
> >+	if (write(connfd, msg, sizeof msg) != sizeof msg ||
> >+	    read(connfd, msg, sizeof msg) != sizeof msg) {
> >+		fprintf(stderr, "Couldn't send/recv local address\n");
> 
> At that step the server expects to read a "done" response from its client,
> checking whether the read was done for sizeof msg which is much larger will
> fail.

This OK?

>From db525af53140c3b7604ab45406ed8845cb6171e1 Mon Sep 17 00:00:00 2001
From: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Date: Sun, 9 Oct 2016 17:17:54 -0600
Subject: [PATCH] verbs: Fix read error check in pingpong

Turns out these reads rely on the short readed caused by EOS.

Fixes: f3912df771db (Avoid gcc 5.4 warning -Wunused-result)
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 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/libibverbs/examples/rc_pingpong.c b/libibverbs/examples/rc_pingpong.c
index c92e551c00e6..aca7bac4491b 100644
--- a/libibverbs/examples/rc_pingpong.c
+++ b/libibverbs/examples/rc_pingpong.c
@@ -309,7 +309,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
 	sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn,
 							my_dest->psn, gid);
 	if (write(connfd, msg, sizeof msg) != sizeof msg ||
-	    read(connfd, msg, sizeof msg) != sizeof msg) {
+	    read(connfd, msg, sizeof msg) != sizeof "done") {
 		fprintf(stderr, "Couldn't send/recv local address\n");
 		free(rem_dest);
 		rem_dest = NULL;
diff --git a/libibverbs/examples/srq_pingpong.c b/libibverbs/examples/srq_pingpong.c
index 863ff10dd0f4..8f55d78e87d4 100644
--- a/libibverbs/examples/srq_pingpong.c
+++ b/libibverbs/examples/srq_pingpong.c
@@ -333,7 +333,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
 		}
 	}
 
-	if (read(connfd, msg, sizeof msg) != sizeof msg) {
+	if (read(connfd, msg, sizeof msg) != sizeof "done") {
 		perror("client write");
 		free(rem_dest);
 		rem_dest = NULL;
diff --git a/libibverbs/examples/uc_pingpong.c b/libibverbs/examples/uc_pingpong.c
index 2b105b947cf3..b565bacaff2a 100644
--- a/libibverbs/examples/uc_pingpong.c
+++ b/libibverbs/examples/uc_pingpong.c
@@ -283,7 +283,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
 	sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn,
 							my_dest->psn, gid);
 	if (write(connfd, msg, sizeof msg) != sizeof msg ||
-	    read(connfd, msg, sizeof msg) != sizeof msg) {
+	    read(connfd, msg, sizeof msg) != sizeof "done") {
 		fprintf(stderr, "Couldn't send/recv local address\n");
 		free(rem_dest);
 		rem_dest = NULL;
diff --git a/libibverbs/examples/ud_pingpong.c b/libibverbs/examples/ud_pingpong.c
index d0cd73cc0fae..ddb68cf8624b 100644
--- a/libibverbs/examples/ud_pingpong.c
+++ b/libibverbs/examples/ud_pingpong.c
@@ -280,7 +280,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
 	sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn,
 							my_dest->psn, gid);
 	if (write(connfd, msg, sizeof msg) != sizeof msg ||
-	    read(connfd, msg, sizeof msg) != sizeof msg) {
+	    read(connfd, msg, sizeof msg) != sizeof "done") {
 		fprintf(stderr, "Couldn't send/recv local address\n");
 		free(rem_dest);
 		rem_dest = NULL;
-- 
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 26/28] Avoid gcc 5.4 warning -Wunused-result

[Yishai Hadas]

On 10/10/2016 2:19 AM, Jason Gunthorpe wrote:
> On Sun, Oct 09, 2016 at 04:31:17PM +0300, Yishai Hadas wrote:
>
>>> -	if (write(connfd, msg, sizeof msg) != sizeof msg) {
>>> -		fprintf(stderr, "Couldn't send local address\n");
>>> +	if (write(connfd, msg, sizeof msg) != sizeof msg ||
>>> +	    read(connfd, msg, sizeof msg) != sizeof msg) {
>>> +		fprintf(stderr, "Couldn't send/recv local address\n");
>>
>> At that step the server expects to read a "done" response from its client,
>> checking whether the read was done for sizeof msg which is much larger will
>> fail.
>
> This OK?

Yes, below patch solves the issue.

> From db525af53140c3b7604ab45406ed8845cb6171e1 Mon Sep 17 00:00:00 2001
> From: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> Date: Sun, 9 Oct 2016 17:17:54 -0600
> Subject: [PATCH] verbs: Fix read error check in pingpong
>
> Turns out these reads rely on the short readed caused by EOS.
>
> Fixes: f3912df771db (Avoid gcc 5.4 warning -Wunused-result)
> 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 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/libibverbs/examples/rc_pingpong.c b/libibverbs/examples/rc_pingpong.c
> index c92e551c00e6..aca7bac4491b 100644
> --- a/libibverbs/examples/rc_pingpong.c
> +++ b/libibverbs/examples/rc_pingpong.c
> @@ -309,7 +309,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
>  	sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn,
>  							my_dest->psn, gid);
>  	if (write(connfd, msg, sizeof msg) != sizeof msg ||
> -	    read(connfd, msg, sizeof msg) != sizeof msg) {
> +	    read(connfd, msg, sizeof msg) != sizeof "done") {
>  		fprintf(stderr, "Couldn't send/recv local address\n");
>  		free(rem_dest);
>  		rem_dest = NULL;
> diff --git a/libibverbs/examples/srq_pingpong.c b/libibverbs/examples/srq_pingpong.c
> index 863ff10dd0f4..8f55d78e87d4 100644
> --- a/libibverbs/examples/srq_pingpong.c
> +++ b/libibverbs/examples/srq_pingpong.c
> @@ -333,7 +333,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
>  		}
>  	}
>
> -	if (read(connfd, msg, sizeof msg) != sizeof msg) {
> +	if (read(connfd, msg, sizeof msg) != sizeof "done") {
>  		perror("client write");
>  		free(rem_dest);
>  		rem_dest = NULL;
> diff --git a/libibverbs/examples/uc_pingpong.c b/libibverbs/examples/uc_pingpong.c
> index 2b105b947cf3..b565bacaff2a 100644
> --- a/libibverbs/examples/uc_pingpong.c
> +++ b/libibverbs/examples/uc_pingpong.c
> @@ -283,7 +283,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
>  	sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn,
>  							my_dest->psn, gid);
>  	if (write(connfd, msg, sizeof msg) != sizeof msg ||
> -	    read(connfd, msg, sizeof msg) != sizeof msg) {
> +	    read(connfd, msg, sizeof msg) != sizeof "done") {
>  		fprintf(stderr, "Couldn't send/recv local address\n");
>  		free(rem_dest);
>  		rem_dest = NULL;
> diff --git a/libibverbs/examples/ud_pingpong.c b/libibverbs/examples/ud_pingpong.c
> index d0cd73cc0fae..ddb68cf8624b 100644
> --- a/libibverbs/examples/ud_pingpong.c
> +++ b/libibverbs/examples/ud_pingpong.c
> @@ -280,7 +280,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
>  	sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn,
>  							my_dest->psn, gid);
>  	if (write(connfd, msg, sizeof msg) != sizeof msg ||
> -	    read(connfd, msg, sizeof msg) != sizeof msg) {
> +	    read(connfd, msg, sizeof msg) != sizeof "done") {
>  		fprintf(stderr, "Couldn't send/recv local address\n");
>  		free(rem_dest);
>  		rem_dest = NULL;
>

--
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 26/28] Avoid gcc 5.4 warning -Wunused-result

[Doug Ledford]

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

On 10/10/2016 6:06 AM, Yishai Hadas wrote:
> On 10/10/2016 2:19 AM, Jason Gunthorpe wrote:
>> On Sun, Oct 09, 2016 at 04:31:17PM +0300, Yishai Hadas wrote:
>>
>>>> -    if (write(connfd, msg, sizeof msg) != sizeof msg) {
>>>> -        fprintf(stderr, "Couldn't send local address\n");
>>>> +    if (write(connfd, msg, sizeof msg) != sizeof msg ||
>>>> +        read(connfd, msg, sizeof msg) != sizeof msg) {
>>>> +        fprintf(stderr, "Couldn't send/recv local address\n");
>>>
>>> At that step the server expects to read a "done" response from its
>>> client,
>>> checking whether the read was done for sizeof msg which is much
>>> larger will
>>> fail.
>>
>> This OK?
> 
> Yes, below patch solves the issue.

Merged, thanks.


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

Re: [PATCH 26/28] Avoid gcc 5.4 warning -Wunused-result

[Bart Van Assche]

On 09/05/16 14:08, Jason Gunthorpe wrote:
> It used to be you could suppress this with (void), however the gcc
> developers have decided to get rid of that.
>
> So, look closely at each occurrence and decide what to do:
> [ ... ]

Hi Jason,

Are you aware that there are less intrusive ways to suppress "unused 
result" warnings, e.g. by using a macro like the one below?

#define UNUSED_VALUE(v) do { if (v) { } } while (0)

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 26/28] Avoid gcc 5.4 warning -Wunused-result

[Jason Gunthorpe]

On Sun, Oct 09, 2016 at 08:14:26AM -0700, Bart Van Assche wrote:

> Are you aware that there are less intrusive ways to suppress "unused result"
> warnings, e.g. by using a macro like the one below?
> 
> #define UNUSED_VALUE(v) do { if (v) { } } while (0)

Hmm, I think the only place I'd use that is the fscanf stuff - and
that is only because I don't know about about that code to put in
sensible error handling. It doesn't seem much better than the
failable_fscanf wrapper?

All the other places gained at least some kind of improvement in error
handling..

Generally this warning is not wrong so I'm not excited to have
'official' infrastructure to ignore it.

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 27/28] Avoid clang 3.6 warning -Wmissing-field-initializers

[Jason Gunthorpe]

The canonical way to zero fill a struct is {}.

Sometimes people will write this as {0} which does the same thing
if the first struct member is integral.

However the preference for {} is because it allows the compiler to see
that the intent is for every member to be zero, and this is not an
inadvertent incomplete initialization of an array or struct.

We have a random jumble of both styles, so lets prefer {} since
it avoids a useful warning.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libibverbs/examples/asyncwatch.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   | 4 ++--
 libibverbs/examples/xsrq_pingpong.c | 2 +-
 libmlx5/src/verbs.c                 | 4 ++--
 7 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/libibverbs/examples/asyncwatch.c b/libibverbs/examples/asyncwatch.c
index df1261503b7d..c78994d24bed 100644
--- a/libibverbs/examples/asyncwatch.c
+++ b/libibverbs/examples/asyncwatch.c
@@ -105,7 +105,7 @@ int main(int argc, char *argv[])
 		static struct option long_options[] = {
 			{ .name = "ib-dev",    .has_arg = 1, .val = 'd' },
 			{ .name = "help",      .has_arg = 0, .val = 'h' },
-			{ 0 }
+			{}
 		};
 
 		c = getopt_long(argc, argv, "d:h", long_options, NULL);
diff --git a/libibverbs/examples/rc_pingpong.c b/libibverbs/examples/rc_pingpong.c
index 7bcc413a0f1d..9054a68b7eb5 100644
--- a/libibverbs/examples/rc_pingpong.c
+++ b/libibverbs/examples/rc_pingpong.c
@@ -731,7 +731,7 @@ int main(int argc, char *argv[])
 			{ .name = "gid-idx",  .has_arg = 1, .val = 'g' },
 			{ .name = "odp",      .has_arg = 0, .val = 'o' },
 			{ .name = "ts",       .has_arg = 0, .val = 't' },
-			{ 0 }
+			{}
 		};
 
 		c = getopt_long(argc, argv, "p:d:i:s:m:r:n:l:eg:ot",
diff --git a/libibverbs/examples/srq_pingpong.c b/libibverbs/examples/srq_pingpong.c
index e6492dc553fd..f17972580b57 100644
--- a/libibverbs/examples/srq_pingpong.c
+++ b/libibverbs/examples/srq_pingpong.c
@@ -665,7 +665,7 @@ int main(int argc, char *argv[])
 			{ .name = "sl",       .has_arg = 1, .val = 'l' },
 			{ .name = "events",   .has_arg = 0, .val = 'e' },
 			{ .name = "gid-idx",  .has_arg = 1, .val = 'g' },
-			{ 0 }
+			{}
 		};
 
 		c = getopt_long(argc, argv, "p:d:i:s:m:q:r:n:l:eg:",
diff --git a/libibverbs/examples/uc_pingpong.c b/libibverbs/examples/uc_pingpong.c
index d132de98694a..7d982d36a5ef 100644
--- a/libibverbs/examples/uc_pingpong.c
+++ b/libibverbs/examples/uc_pingpong.c
@@ -568,7 +568,7 @@ int main(int argc, char *argv[])
 			{ .name = "sl",       .has_arg = 1, .val = 'l' },
 			{ .name = "events",   .has_arg = 0, .val = 'e' },
 			{ .name = "gid-idx",  .has_arg = 1, .val = 'g' },
-			{ 0 }
+			{}
 		};
 
 		c = getopt_long(argc, argv, "p:d:i:s:m:r:n:l:eg:",
diff --git a/libibverbs/examples/ud_pingpong.c b/libibverbs/examples/ud_pingpong.c
index 67da4bd90f32..deefb9b81013 100644
--- a/libibverbs/examples/ud_pingpong.c
+++ b/libibverbs/examples/ud_pingpong.c
@@ -324,7 +324,7 @@ static struct pingpong_context *pp_init_ctx(struct ibv_device *ib_dev, int size,
 	}
 
 	{
-		struct ibv_port_attr port_info = { 0 };
+		struct ibv_port_attr port_info = {};
 		int mtu;
 
 		if (ibv_query_port(ctx->context, port, &port_info)) {
@@ -588,7 +588,7 @@ int main(int argc, char *argv[])
 			{ .name = "sl",       .has_arg = 1, .val = 'l' },
 			{ .name = "events",   .has_arg = 0, .val = 'e' },
 			{ .name = "gid-idx",  .has_arg = 1, .val = 'g' },
-			{ 0 }
+			{}
 		};
 
 		c = getopt_long(argc, argv, "p:d:i:s:r:n:l:eg:",
diff --git a/libibverbs/examples/xsrq_pingpong.c b/libibverbs/examples/xsrq_pingpong.c
index a7e345f3850c..903548ed6824 100644
--- a/libibverbs/examples/xsrq_pingpong.c
+++ b/libibverbs/examples/xsrq_pingpong.c
@@ -875,7 +875,7 @@ int main(int argc, char *argv[])
 			{ .name = "sl",        .has_arg = 1, .val = 'l' },
 			{ .name = "events",    .has_arg = 0, .val = 'e' },
 			{ .name = "gid-idx",   .has_arg = 1, .val = 'g' },
-			{ 0 }
+			{}
 		};
 
 		c = getopt_long(argc, argv, "p:d:i:s:m:c:n:l:eg:", long_options,
diff --git a/libmlx5/src/verbs.c b/libmlx5/src/verbs.c
index a76821e6f695..054633cbed29 100644
--- a/libmlx5/src/verbs.c
+++ b/libmlx5/src/verbs.c
@@ -1561,8 +1561,8 @@ mlx5_open_xrcd(struct ibv_context *context,
 {
 	int err;
 	struct verbs_xrcd *xrcd;
-	struct ibv_open_xrcd cmd = {0};
-	struct ibv_open_xrcd_resp resp = {0};
+	struct ibv_open_xrcd cmd = {};
+	struct ibv_open_xrcd_resp resp = {};
 
 	xrcd = calloc(1, sizeof(*xrcd));
 	if (!xrcd)
-- 
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 28/28] Avoid gcc warning -Wpointer-to-int-cast

[Jason Gunthorpe]

In C99 casting a pointer to an integer should always be done via
uintptr_t.

When compiling on 32 bit all these sites produce warnings.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 iwpm/src/iwarp_pm_helper.c  | 2 +-
 libhfi1verbs/src/verbs.c    | 2 +-
 libi40iw/src/i40iw_uverbs.c | 2 +-
 libipathverbs/src/verbs.c   | 2 +-
 librdmacm/src/rsocket.c     | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/iwpm/src/iwarp_pm_helper.c b/iwpm/src/iwarp_pm_helper.c
index f5c7b96a22e1..89d2b6cef5e6 100644
--- a/iwpm/src/iwarp_pm_helper.c
+++ b/iwpm/src/iwarp_pm_helper.c
@@ -82,7 +82,7 @@ iwpm_mapping_request *create_iwpm_map_request(struct nlmsghdr *req_nlh,
 	/* assochandle helps match iwpm request sent to remote peer with future iwpm accept/reject */
 	iwpm_map_req->assochandle = assochandle;
 	if (!assochandle)
-		iwpm_map_req->assochandle = (__u64)iwpm_map_req;
+		iwpm_map_req->assochandle = (uintptr_t)iwpm_map_req;
 
 	memcpy(&iwpm_map_req->src_addr, src_addr, sizeof(struct sockaddr_storage));
 	/* keep record of remote IP address and port */
diff --git a/libhfi1verbs/src/verbs.c b/libhfi1verbs/src/verbs.c
index 854c5676908d..e245ad9e5b4f 100644
--- a/libhfi1verbs/src/verbs.c
+++ b/libhfi1verbs/src/verbs.c
@@ -607,7 +607,7 @@ int hfi1_modify_srq(struct ibv_srq *ibsrq,
 			 (sizeof(struct ibv_sge) * srq->rq.max_sge)) *
 			srq->rq.size;
 	}
-	cmd.offset_addr = (__u64) &offset;
+	cmd.offset_addr = (uintptr_t) &offset;
 	ret = ibv_cmd_modify_srq(ibsrq, attr, attr_mask,
 				 &cmd.ibv_cmd, sizeof cmd);
 	if (ret) {
diff --git a/libi40iw/src/i40iw_uverbs.c b/libi40iw/src/i40iw_uverbs.c
index db69b66c1e90..8369e10f61ee 100644
--- a/libi40iw/src/i40iw_uverbs.c
+++ b/libi40iw/src/i40iw_uverbs.c
@@ -557,7 +557,7 @@ static int i40iw_vmapped_qp(struct i40iw_uqp *iwuqp, struct ibv_pd *pd,
 		return 0;
 	}
 	cmd.user_wqe_buffers = (__u64)((uintptr_t)info->sq);
-	cmd.user_compl_ctx = (u64)&iwuqp->qp;
+	cmd.user_compl_ctx = (uintptr_t)&iwuqp->qp;
 
 	ret = ibv_cmd_create_qp(pd, &iwuqp->ibv_qp, attr, &cmd.ibv_cmd, sizeof(cmd),
 				&resp->ibv_resp, sizeof(struct i40iw_ucreate_qp_resp));
diff --git a/libipathverbs/src/verbs.c b/libipathverbs/src/verbs.c
index 17d54cd4026b..578a38af3428 100644
--- a/libipathverbs/src/verbs.c
+++ b/libipathverbs/src/verbs.c
@@ -583,7 +583,7 @@ int ipath_modify_srq(struct ibv_srq *ibsrq,
 			 (sizeof(struct ibv_sge) * srq->rq.max_sge)) *
 			srq->rq.size;
 	}
-	cmd.offset_addr = (__u64) &offset;
+	cmd.offset_addr = (uintptr_t) &offset;
 	ret = ibv_cmd_modify_srq(ibsrq, attr, attr_mask,
 				 &cmd.ibv_cmd, sizeof cmd);
 	if (ret) {
diff --git a/librdmacm/src/rsocket.c b/librdmacm/src/rsocket.c
index 5645f40d2460..205101f1702f 100644
--- a/librdmacm/src/rsocket.c
+++ b/librdmacm/src/rsocket.c
@@ -4290,7 +4290,7 @@ static void tcp_svc_send_keepalive(struct rsocket *rs)
 	if (rs_ctrl_avail(rs) && (rs->state & rs_connected)) {
 		rs->ctrl_seqno++;
 		rs_post_write(rs, NULL, 0, rs_msg_set(RS_OP_CTRL, RS_CTRL_KEEPALIVE),
-			      0, (uint64_t) NULL, (uint64_t) NULL);
+			      0, (uintptr_t) NULL, (uintptr_t) NULL);
 	}
 	fastlock_release(&rs->cq_lock);
 }	
-- 
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