[PATCH for-next] RDMA/cma: Remove unnecessary INIT->INIT transition

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH for-next] RDMA/cma: Remove unnecessary INIT->INIT transition
@ 2021-06-17 15:46 Håkon Bugge
  2021-06-17 20:40   ` kernel test robot
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Håkon Bugge @ 2021-06-17 15:46 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe, Leon Romanovsky; +Cc: linux-rdma

In rdma_create_qp(), a connected QP will be transitioned to the INIT
state.

Afterwards, the QP will be transitioned to the RTR state by the
cma_modify_qp_rtr() function. But this function starts by performing
an ib_modify_qp() to the INIT state again, before another
ib_modify_qp() is performed to transition the QP to the RTR state.

Hence, there is no need to transition the QP to the INIT state in
rdma_create_qp().

Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
---
 drivers/infiniband/core/cma.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 2b9ffc2..937e44e 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -925,19 +925,6 @@ static int cma_init_ud_qp(struct rdma_id_private *id_priv, struct ib_qp *qp)
 	return ret;
 }
 
-static int cma_init_conn_qp(struct rdma_id_private *id_priv, struct ib_qp *qp)
-{
-	struct ib_qp_attr qp_attr;
-	int qp_attr_mask, ret;
-
-	qp_attr.qp_state = IB_QPS_INIT;
-	ret = rdma_init_qp_attr(&id_priv->id, &qp_attr, &qp_attr_mask);
-	if (ret)
-		return ret;
-
-	return ib_modify_qp(qp, &qp_attr, qp_attr_mask);
-}
-
 int rdma_create_qp(struct rdma_cm_id *id, struct ib_pd *pd,
 		   struct ib_qp_init_attr *qp_init_attr)
 {
@@ -960,8 +947,6 @@ int rdma_create_qp(struct rdma_cm_id *id, struct ib_pd *pd,
 
 	if (id->qp_type == IB_QPT_UD)
 		ret = cma_init_ud_qp(id_priv, qp);
-	else
-		ret = cma_init_conn_qp(id_priv, qp);
 	if (ret)
 		goto out_destroy;
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH for-next] RDMA/cma: Remove unnecessary INIT->INIT transition
  2021-06-17 15:46 [PATCH for-next] RDMA/cma: Remove unnecessary INIT->INIT transition Håkon Bugge
@ 2021-06-17 20:40   ` kernel test robot
  2021-06-20 12:19 ` Mark Zhang
  2021-06-21 10:07 ` Leon Romanovsky
  2 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2021-06-17 20:40 UTC (permalink / raw)
  To: Håkon Bugge, Doug Ledford, Jason Gunthorpe, Leon Romanovsky
  Cc: kbuild-all, clang-built-linux, linux-rdma

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

Hi "Håkon,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on rdma/for-next]
[also build test WARNING on v5.13-rc6 next-20210617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/H-kon-Bugge/RDMA-cma-Remove-unnecessary-INIT-INIT-transition/20210617-234819
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git for-next
config: powerpc64-randconfig-r011-20210617 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 64720f57bea6a6bf033feef4a5751ab9c0c3b401)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install powerpc64 cross compiling tool for clang build
        # apt-get install binutils-powerpc64-linux-gnu
        # https://github.com/0day-ci/linux/commit/c0ea00a31278d0ffe083f121ddb13e27f999c0a8
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review H-kon-Bugge/RDMA-cma-Remove-unnecessary-INIT-INIT-transition/20210617-234819
        git checkout c0ea00a31278d0ffe083f121ddb13e27f999c0a8
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
                   __do_##name al;                                 \
                   ^~~~~~~~~~~~~~
   <scratch space>:249:1: note: expanded from here
   __do_insb
   ^
   arch/powerpc/include/asm/io.h:556:56: note: expanded from macro '__do_insb'
   #define __do_insb(p, b, n)      readsb((PCI_IO_ADDR)_IO_BASE+(p), (b), (n))
                                          ~~~~~~~~~~~~~~~~~~~~~^
   In file included from drivers/infiniband/core/cma.c:14:
   In file included from include/linux/igmp.h:13:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/powerpc/include/asm/io.h:619:
   arch/powerpc/include/asm/io-defs.h:45:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(insw, (unsigned long p, void *b, unsigned long c),
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
                   __do_##name al;                                 \
                   ^~~~~~~~~~~~~~
   <scratch space>:251:1: note: expanded from here
   __do_insw
   ^
   arch/powerpc/include/asm/io.h:557:56: note: expanded from macro '__do_insw'
   #define __do_insw(p, b, n)      readsw((PCI_IO_ADDR)_IO_BASE+(p), (b), (n))
                                          ~~~~~~~~~~~~~~~~~~~~~^
   In file included from drivers/infiniband/core/cma.c:14:
   In file included from include/linux/igmp.h:13:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/powerpc/include/asm/io.h:619:
   arch/powerpc/include/asm/io-defs.h:47:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(insl, (unsigned long p, void *b, unsigned long c),
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
                   __do_##name al;                                 \
                   ^~~~~~~~~~~~~~
   <scratch space>:253:1: note: expanded from here
   __do_insl
   ^
   arch/powerpc/include/asm/io.h:558:56: note: expanded from macro '__do_insl'
   #define __do_insl(p, b, n)      readsl((PCI_IO_ADDR)_IO_BASE+(p), (b), (n))
                                          ~~~~~~~~~~~~~~~~~~~~~^
   In file included from drivers/infiniband/core/cma.c:14:
   In file included from include/linux/igmp.h:13:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/powerpc/include/asm/io.h:619:
   arch/powerpc/include/asm/io-defs.h:49:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(outsb, (unsigned long p, const void *b, unsigned long c),
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
                   __do_##name al;                                 \
                   ^~~~~~~~~~~~~~
   <scratch space>:255:1: note: expanded from here
   __do_outsb
   ^
   arch/powerpc/include/asm/io.h:559:58: note: expanded from macro '__do_outsb'
   #define __do_outsb(p, b, n)     writesb((PCI_IO_ADDR)_IO_BASE+(p),(b),(n))
                                           ~~~~~~~~~~~~~~~~~~~~~^
   In file included from drivers/infiniband/core/cma.c:14:
   In file included from include/linux/igmp.h:13:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/powerpc/include/asm/io.h:619:
   arch/powerpc/include/asm/io-defs.h:51:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(outsw, (unsigned long p, const void *b, unsigned long c),
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
                   __do_##name al;                                 \
                   ^~~~~~~~~~~~~~
   <scratch space>:257:1: note: expanded from here
   __do_outsw
   ^
   arch/powerpc/include/asm/io.h:560:58: note: expanded from macro '__do_outsw'
   #define __do_outsw(p, b, n)     writesw((PCI_IO_ADDR)_IO_BASE+(p),(b),(n))
                                           ~~~~~~~~~~~~~~~~~~~~~^
   In file included from drivers/infiniband/core/cma.c:14:
   In file included from include/linux/igmp.h:13:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/powerpc/include/asm/io.h:619:
   arch/powerpc/include/asm/io-defs.h:53:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(outsl, (unsigned long p, const void *b, unsigned long c),
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
                   __do_##name al;                                 \
                   ^~~~~~~~~~~~~~
   <scratch space>:259:1: note: expanded from here
   __do_outsl
   ^
   arch/powerpc/include/asm/io.h:561:58: note: expanded from macro '__do_outsl'
   #define __do_outsl(p, b, n)     writesl((PCI_IO_ADDR)_IO_BASE+(p),(b),(n))
                                           ~~~~~~~~~~~~~~~~~~~~~^
>> drivers/infiniband/core/cma.c:948:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
           if (id->qp_type == IB_QPT_UD)
               ^~~~~~~~~~~~~~~~~~~~~~~~
   drivers/infiniband/core/cma.c:950:6: note: uninitialized use occurs here
           if (ret)
               ^~~
   drivers/infiniband/core/cma.c:948:2: note: remove the 'if' if its condition is always true
           if (id->qp_type == IB_QPT_UD)
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/infiniband/core/cma.c:933:9: note: initialize the variable 'ret' to silence this warning
           int ret;
                  ^
                   = 0
   14 warnings generated.


vim +948 drivers/infiniband/core/cma.c

e51060f08a6196 Sean Hefty        2006-06-17  927  
e51060f08a6196 Sean Hefty        2006-06-17  928  int rdma_create_qp(struct rdma_cm_id *id, struct ib_pd *pd,
e51060f08a6196 Sean Hefty        2006-06-17  929  		   struct ib_qp_init_attr *qp_init_attr)
e51060f08a6196 Sean Hefty        2006-06-17  930  {
e51060f08a6196 Sean Hefty        2006-06-17  931  	struct rdma_id_private *id_priv;
e51060f08a6196 Sean Hefty        2006-06-17  932  	struct ib_qp *qp;
e51060f08a6196 Sean Hefty        2006-06-17  933  	int ret;
e51060f08a6196 Sean Hefty        2006-06-17  934  
e51060f08a6196 Sean Hefty        2006-06-17  935  	id_priv = container_of(id, struct rdma_id_private, id);
ed999f820a6c57 Chuck Lever       2019-12-18  936  	if (id->device != pd->device) {
ed999f820a6c57 Chuck Lever       2019-12-18  937  		ret = -EINVAL;
ed999f820a6c57 Chuck Lever       2019-12-18  938  		goto out_err;
ed999f820a6c57 Chuck Lever       2019-12-18  939  	}
e51060f08a6196 Sean Hefty        2006-06-17  940  
0691a286d59183 Christoph Hellwig 2016-05-03  941  	qp_init_attr->port_num = id->port_num;
e51060f08a6196 Sean Hefty        2006-06-17  942  	qp = ib_create_qp(pd, qp_init_attr);
ed999f820a6c57 Chuck Lever       2019-12-18  943  	if (IS_ERR(qp)) {
ed999f820a6c57 Chuck Lever       2019-12-18  944  		ret = PTR_ERR(qp);
ed999f820a6c57 Chuck Lever       2019-12-18  945  		goto out_err;
ed999f820a6c57 Chuck Lever       2019-12-18  946  	}
e51060f08a6196 Sean Hefty        2006-06-17  947  
b26f9b9949013f Sean Hefty        2010-04-01 @948  	if (id->qp_type == IB_QPT_UD)
c8f6a362bf3eb2 Sean Hefty        2007-02-15  949  		ret = cma_init_ud_qp(id_priv, qp);
e51060f08a6196 Sean Hefty        2006-06-17  950  	if (ret)
ed999f820a6c57 Chuck Lever       2019-12-18  951  		goto out_destroy;
e51060f08a6196 Sean Hefty        2006-06-17  952  
e51060f08a6196 Sean Hefty        2006-06-17  953  	id->qp = qp;
e51060f08a6196 Sean Hefty        2006-06-17  954  	id_priv->qp_num = qp->qp_num;
e51060f08a6196 Sean Hefty        2006-06-17  955  	id_priv->srq = (qp->srq != NULL);
ed999f820a6c57 Chuck Lever       2019-12-18  956  	trace_cm_qp_create(id_priv, pd, qp_init_attr, 0);
e51060f08a6196 Sean Hefty        2006-06-17  957  	return 0;
ed999f820a6c57 Chuck Lever       2019-12-18  958  out_destroy:
e51060f08a6196 Sean Hefty        2006-06-17  959  	ib_destroy_qp(qp);
ed999f820a6c57 Chuck Lever       2019-12-18  960  out_err:
ed999f820a6c57 Chuck Lever       2019-12-18  961  	trace_cm_qp_create(id_priv, pd, qp_init_attr, ret);
e51060f08a6196 Sean Hefty        2006-06-17  962  	return ret;
e51060f08a6196 Sean Hefty        2006-06-17  963  }
e51060f08a6196 Sean Hefty        2006-06-17  964  EXPORT_SYMBOL(rdma_create_qp);
e51060f08a6196 Sean Hefty        2006-06-17  965  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26823 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH for-next] RDMA/cma: Remove unnecessary INIT->INIT transition
@ 2021-06-17 20:40   ` kernel test robot
  0 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2021-06-17 20:40 UTC (permalink / raw)
  To: kbuild-all

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

Hi "Håkon,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on rdma/for-next]
[also build test WARNING on v5.13-rc6 next-20210617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/H-kon-Bugge/RDMA-cma-Remove-unnecessary-INIT-INIT-transition/20210617-234819
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git for-next
config: powerpc64-randconfig-r011-20210617 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 64720f57bea6a6bf033feef4a5751ab9c0c3b401)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install powerpc64 cross compiling tool for clang build
        # apt-get install binutils-powerpc64-linux-gnu
        # https://github.com/0day-ci/linux/commit/c0ea00a31278d0ffe083f121ddb13e27f999c0a8
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review H-kon-Bugge/RDMA-cma-Remove-unnecessary-INIT-INIT-transition/20210617-234819
        git checkout c0ea00a31278d0ffe083f121ddb13e27f999c0a8
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
                   __do_##name al;                                 \
                   ^~~~~~~~~~~~~~
   <scratch space>:249:1: note: expanded from here
   __do_insb
   ^
   arch/powerpc/include/asm/io.h:556:56: note: expanded from macro '__do_insb'
   #define __do_insb(p, b, n)      readsb((PCI_IO_ADDR)_IO_BASE+(p), (b), (n))
                                          ~~~~~~~~~~~~~~~~~~~~~^
   In file included from drivers/infiniband/core/cma.c:14:
   In file included from include/linux/igmp.h:13:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/powerpc/include/asm/io.h:619:
   arch/powerpc/include/asm/io-defs.h:45:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(insw, (unsigned long p, void *b, unsigned long c),
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
                   __do_##name al;                                 \
                   ^~~~~~~~~~~~~~
   <scratch space>:251:1: note: expanded from here
   __do_insw
   ^
   arch/powerpc/include/asm/io.h:557:56: note: expanded from macro '__do_insw'
   #define __do_insw(p, b, n)      readsw((PCI_IO_ADDR)_IO_BASE+(p), (b), (n))
                                          ~~~~~~~~~~~~~~~~~~~~~^
   In file included from drivers/infiniband/core/cma.c:14:
   In file included from include/linux/igmp.h:13:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/powerpc/include/asm/io.h:619:
   arch/powerpc/include/asm/io-defs.h:47:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(insl, (unsigned long p, void *b, unsigned long c),
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
                   __do_##name al;                                 \
                   ^~~~~~~~~~~~~~
   <scratch space>:253:1: note: expanded from here
   __do_insl
   ^
   arch/powerpc/include/asm/io.h:558:56: note: expanded from macro '__do_insl'
   #define __do_insl(p, b, n)      readsl((PCI_IO_ADDR)_IO_BASE+(p), (b), (n))
                                          ~~~~~~~~~~~~~~~~~~~~~^
   In file included from drivers/infiniband/core/cma.c:14:
   In file included from include/linux/igmp.h:13:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/powerpc/include/asm/io.h:619:
   arch/powerpc/include/asm/io-defs.h:49:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(outsb, (unsigned long p, const void *b, unsigned long c),
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
                   __do_##name al;                                 \
                   ^~~~~~~~~~~~~~
   <scratch space>:255:1: note: expanded from here
   __do_outsb
   ^
   arch/powerpc/include/asm/io.h:559:58: note: expanded from macro '__do_outsb'
   #define __do_outsb(p, b, n)     writesb((PCI_IO_ADDR)_IO_BASE+(p),(b),(n))
                                           ~~~~~~~~~~~~~~~~~~~~~^
   In file included from drivers/infiniband/core/cma.c:14:
   In file included from include/linux/igmp.h:13:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/powerpc/include/asm/io.h:619:
   arch/powerpc/include/asm/io-defs.h:51:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(outsw, (unsigned long p, const void *b, unsigned long c),
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
                   __do_##name al;                                 \
                   ^~~~~~~~~~~~~~
   <scratch space>:257:1: note: expanded from here
   __do_outsw
   ^
   arch/powerpc/include/asm/io.h:560:58: note: expanded from macro '__do_outsw'
   #define __do_outsw(p, b, n)     writesw((PCI_IO_ADDR)_IO_BASE+(p),(b),(n))
                                           ~~~~~~~~~~~~~~~~~~~~~^
   In file included from drivers/infiniband/core/cma.c:14:
   In file included from include/linux/igmp.h:13:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/powerpc/include/asm/io.h:619:
   arch/powerpc/include/asm/io-defs.h:53:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(outsl, (unsigned long p, const void *b, unsigned long c),
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
                   __do_##name al;                                 \
                   ^~~~~~~~~~~~~~
   <scratch space>:259:1: note: expanded from here
   __do_outsl
   ^
   arch/powerpc/include/asm/io.h:561:58: note: expanded from macro '__do_outsl'
   #define __do_outsl(p, b, n)     writesl((PCI_IO_ADDR)_IO_BASE+(p),(b),(n))
                                           ~~~~~~~~~~~~~~~~~~~~~^
>> drivers/infiniband/core/cma.c:948:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
           if (id->qp_type == IB_QPT_UD)
               ^~~~~~~~~~~~~~~~~~~~~~~~
   drivers/infiniband/core/cma.c:950:6: note: uninitialized use occurs here
           if (ret)
               ^~~
   drivers/infiniband/core/cma.c:948:2: note: remove the 'if' if its condition is always true
           if (id->qp_type == IB_QPT_UD)
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/infiniband/core/cma.c:933:9: note: initialize the variable 'ret' to silence this warning
           int ret;
                  ^
                   = 0
   14 warnings generated.


vim +948 drivers/infiniband/core/cma.c

e51060f08a6196 Sean Hefty        2006-06-17  927  
e51060f08a6196 Sean Hefty        2006-06-17  928  int rdma_create_qp(struct rdma_cm_id *id, struct ib_pd *pd,
e51060f08a6196 Sean Hefty        2006-06-17  929  		   struct ib_qp_init_attr *qp_init_attr)
e51060f08a6196 Sean Hefty        2006-06-17  930  {
e51060f08a6196 Sean Hefty        2006-06-17  931  	struct rdma_id_private *id_priv;
e51060f08a6196 Sean Hefty        2006-06-17  932  	struct ib_qp *qp;
e51060f08a6196 Sean Hefty        2006-06-17  933  	int ret;
e51060f08a6196 Sean Hefty        2006-06-17  934  
e51060f08a6196 Sean Hefty        2006-06-17  935  	id_priv = container_of(id, struct rdma_id_private, id);
ed999f820a6c57 Chuck Lever       2019-12-18  936  	if (id->device != pd->device) {
ed999f820a6c57 Chuck Lever       2019-12-18  937  		ret = -EINVAL;
ed999f820a6c57 Chuck Lever       2019-12-18  938  		goto out_err;
ed999f820a6c57 Chuck Lever       2019-12-18  939  	}
e51060f08a6196 Sean Hefty        2006-06-17  940  
0691a286d59183 Christoph Hellwig 2016-05-03  941  	qp_init_attr->port_num = id->port_num;
e51060f08a6196 Sean Hefty        2006-06-17  942  	qp = ib_create_qp(pd, qp_init_attr);
ed999f820a6c57 Chuck Lever       2019-12-18  943  	if (IS_ERR(qp)) {
ed999f820a6c57 Chuck Lever       2019-12-18  944  		ret = PTR_ERR(qp);
ed999f820a6c57 Chuck Lever       2019-12-18  945  		goto out_err;
ed999f820a6c57 Chuck Lever       2019-12-18  946  	}
e51060f08a6196 Sean Hefty        2006-06-17  947  
b26f9b9949013f Sean Hefty        2010-04-01 @948  	if (id->qp_type == IB_QPT_UD)
c8f6a362bf3eb2 Sean Hefty        2007-02-15  949  		ret = cma_init_ud_qp(id_priv, qp);
e51060f08a6196 Sean Hefty        2006-06-17  950  	if (ret)
ed999f820a6c57 Chuck Lever       2019-12-18  951  		goto out_destroy;
e51060f08a6196 Sean Hefty        2006-06-17  952  
e51060f08a6196 Sean Hefty        2006-06-17  953  	id->qp = qp;
e51060f08a6196 Sean Hefty        2006-06-17  954  	id_priv->qp_num = qp->qp_num;
e51060f08a6196 Sean Hefty        2006-06-17  955  	id_priv->srq = (qp->srq != NULL);
ed999f820a6c57 Chuck Lever       2019-12-18  956  	trace_cm_qp_create(id_priv, pd, qp_init_attr, 0);
e51060f08a6196 Sean Hefty        2006-06-17  957  	return 0;
ed999f820a6c57 Chuck Lever       2019-12-18  958  out_destroy:
e51060f08a6196 Sean Hefty        2006-06-17  959  	ib_destroy_qp(qp);
ed999f820a6c57 Chuck Lever       2019-12-18  960  out_err:
ed999f820a6c57 Chuck Lever       2019-12-18  961  	trace_cm_qp_create(id_priv, pd, qp_init_attr, ret);
e51060f08a6196 Sean Hefty        2006-06-17  962  	return ret;
e51060f08a6196 Sean Hefty        2006-06-17  963  }
e51060f08a6196 Sean Hefty        2006-06-17  964  EXPORT_SYMBOL(rdma_create_qp);
e51060f08a6196 Sean Hefty        2006-06-17  965  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 26823 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH for-next] RDMA/cma: Remove unnecessary INIT->INIT transition
  2021-06-17 15:46 [PATCH for-next] RDMA/cma: Remove unnecessary INIT->INIT transition Håkon Bugge
  2021-06-17 20:40   ` kernel test robot
@ 2021-06-20 12:19 ` Mark Zhang
  2021-06-21  8:44   ` Haakon Bugge
  2021-06-21 10:07 ` Leon Romanovsky
  2 siblings, 1 reply; 8+ messages in thread
From: Mark Zhang @ 2021-06-20 12:19 UTC (permalink / raw)
  To: Håkon Bugge, Doug Ledford, Jason Gunthorpe, Leon Romanovsky
  Cc: linux-rdma

On 6/17/2021 11:46 PM, Håkon Bugge wrote:
> External email: Use caution opening links or attachments
> 
> 
> In rdma_create_qp(), a connected QP will be transitioned to the INIT
> state.
> 
> Afterwards, the QP will be transitioned to the RTR state by the
> cma_modify_qp_rtr() function. But this function starts by performing
> an ib_modify_qp() to the INIT state again, before another
> ib_modify_qp() is performed to transition the QP to the RTR state.
> 
> Hence, there is no need to transition the QP to the INIT state in
> rdma_create_qp().

The comment in cma_modify_qp_rtr() says:
     /* Need to update QP attributes from default values. */

So maybe both are needed? E.g., qp_attr->qp_access_flags maybe updated 
in cm_init_qp_init_attr()?

> Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
> ---
>   drivers/infiniband/core/cma.c | 15 ---------------
>   1 file changed, 15 deletions(-)
> 
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index 2b9ffc2..937e44e 100644
> --- a/drivers/infiniband/core/cma.c
> +++ b/drivers/infiniband/core/cma.c
> @@ -925,19 +925,6 @@ static int cma_init_ud_qp(struct rdma_id_private *id_priv, struct ib_qp *qp)
>          return ret;
>   }
> 
> -static int cma_init_conn_qp(struct rdma_id_private *id_priv, struct ib_qp *qp)
> -{
> -       struct ib_qp_attr qp_attr;
> -       int qp_attr_mask, ret;
> -
> -       qp_attr.qp_state = IB_QPS_INIT;
> -       ret = rdma_init_qp_attr(&id_priv->id, &qp_attr, &qp_attr_mask);
> -       if (ret)
> -               return ret;
> -
> -       return ib_modify_qp(qp, &qp_attr, qp_attr_mask);
> -}
> -
>   int rdma_create_qp(struct rdma_cm_id *id, struct ib_pd *pd,
>                     struct ib_qp_init_attr *qp_init_attr)
>   {
> @@ -960,8 +947,6 @@ int rdma_create_qp(struct rdma_cm_id *id, struct ib_pd *pd,
> 
>          if (id->qp_type == IB_QPT_UD)
>                  ret = cma_init_ud_qp(id_priv, qp);
> -       else
> -               ret = cma_init_conn_qp(id_priv, qp);
>          if (ret)
>                  goto out_destroy;
> 
> --
> 1.8.3.1
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH for-next] RDMA/cma: Remove unnecessary INIT->INIT transition
  2021-06-20 12:19 ` Mark Zhang
@ 2021-06-21  8:44   ` Haakon Bugge
  2021-06-21 12:05     ` Mark Zhang
  0 siblings, 1 reply; 8+ messages in thread
From: Haakon Bugge @ 2021-06-21  8:44 UTC (permalink / raw)
  To: Mark Zhang
  Cc: Doug Ledford, Jason Gunthorpe, Leon Romanovsky, OFED mailing list



> On 20 Jun 2021, at 14:19, Mark Zhang <markzhang@nvidia.com> wrote:
> 
> On 6/17/2021 11:46 PM, Håkon Bugge wrote:
>> External email: Use caution opening links or attachments
>> In rdma_create_qp(), a connected QP will be transitioned to the INIT
>> state.
>> Afterwards, the QP will be transitioned to the RTR state by the
>> cma_modify_qp_rtr() function. But this function starts by performing
>> an ib_modify_qp() to the INIT state again, before another
>> ib_modify_qp() is performed to transition the QP to the RTR state.
>> Hence, there is no need to transition the QP to the INIT state in
>> rdma_create_qp().
> 
> The comment in cma_modify_qp_rtr() says:
>    /* Need to update QP attributes from default values. */
> 
> So maybe both are needed? E.g., qp_attr->qp_access_flags maybe updated in cm_init_qp_init_attr()?

I'll give you two reasons why that is not the case :-)

1. In cm_init_qp_init_attr(), which sets the mask both places, the mask is set hard to 

	IB_QP_STATE | IB_QP_ACCESS_FLAGS | IB_QP_PKEY_INDEX | IB_QP_PORT

If we do the old RESET -> INIT -> INIT, it is the last modify_qp to INIT which will persist. And therefore, the values will be the same if we skip RESET -> INIT.

2. I think the rationale behind modifying the QP to INIT in rdma_create_qp() was to enable ULPs to tweak some of the values. But no ULP calls modify_qp on a QP created by rdma_create_qp(). And if one did, the values would be overwritten when the state transitions to RTR.


Thxs, Håkon



> 
>> Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
>> ---
>>  drivers/infiniband/core/cma.c | 15 ---------------
>>  1 file changed, 15 deletions(-)
>> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
>> index 2b9ffc2..937e44e 100644
>> --- a/drivers/infiniband/core/cma.c
>> +++ b/drivers/infiniband/core/cma.c
>> @@ -925,19 +925,6 @@ static int cma_init_ud_qp(struct rdma_id_private *id_priv, struct ib_qp *qp)
>>         return ret;
>>  }
>> -static int cma_init_conn_qp(struct rdma_id_private *id_priv, struct ib_qp *qp)
>> -{
>> -       struct ib_qp_attr qp_attr;
>> -       int qp_attr_mask, ret;
>> -
>> -       qp_attr.qp_state = IB_QPS_INIT;
>> -       ret = rdma_init_qp_attr(&id_priv->id, &qp_attr, &qp_attr_mask);
>> -       if (ret)
>> -               return ret;
>> -
>> -       return ib_modify_qp(qp, &qp_attr, qp_attr_mask);
>> -}
>> -
>>  int rdma_create_qp(struct rdma_cm_id *id, struct ib_pd *pd,
>>                    struct ib_qp_init_attr *qp_init_attr)
>>  {
>> @@ -960,8 +947,6 @@ int rdma_create_qp(struct rdma_cm_id *id, struct ib_pd *pd,
>>         if (id->qp_type == IB_QPT_UD)
>>                 ret = cma_init_ud_qp(id_priv, qp);
>> -       else
>> -               ret = cma_init_conn_qp(id_priv, qp);
>>         if (ret)
>>                 goto out_destroy;
>> --
>> 1.8.3.1
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH for-next] RDMA/cma: Remove unnecessary INIT->INIT transition
  2021-06-21  8:44   ` Haakon Bugge
@ 2021-06-21 12:05     ` Mark Zhang
  0 siblings, 0 replies; 8+ messages in thread
From: Mark Zhang @ 2021-06-21 12:05 UTC (permalink / raw)
  To: Haakon Bugge
  Cc: Doug Ledford, Jason Gunthorpe, Leon Romanovsky, OFED mailing list

On 6/21/2021 4:44 PM, Haakon Bugge wrote:
> External email: Use caution opening links or attachments
> 
> 
>> On 20 Jun 2021, at 14:19, Mark Zhang <markzhang@nvidia.com> wrote:
>>
>> On 6/17/2021 11:46 PM, Håkon Bugge wrote:
>>> External email: Use caution opening links or attachments
>>> In rdma_create_qp(), a connected QP will be transitioned to the INIT
>>> state.
>>> Afterwards, the QP will be transitioned to the RTR state by the
>>> cma_modify_qp_rtr() function. But this function starts by performing
>>> an ib_modify_qp() to the INIT state again, before another
>>> ib_modify_qp() is performed to transition the QP to the RTR state.
>>> Hence, there is no need to transition the QP to the INIT state in
>>> rdma_create_qp().
>>
>> The comment in cma_modify_qp_rtr() says:
>>     /* Need to update QP attributes from default values. */
>>
>> So maybe both are needed? E.g., qp_attr->qp_access_flags maybe updated in cm_init_qp_init_attr()?
> 
> I'll give you two reasons why that is not the case :-)
> 
> 1. In cm_init_qp_init_attr(), which sets the mask both places, the mask is set hard to
> 
>          IB_QP_STATE | IB_QP_ACCESS_FLAGS | IB_QP_PKEY_INDEX | IB_QP_PORT
> 
> If we do the old RESET -> INIT -> INIT, it is the last modify_qp to INIT which will persist. And therefore, the values will be the same if we skip RESET -> INIT.
> 
> 2. I think the rationale behind modifying the QP to INIT in rdma_create_qp() was to enable ULPs to tweak some of the values. But no ULP calls modify_qp on a QP created by rdma_create_qp(). And if one did, the values would be overwritten when the state transitions to RTR.
> 

Got it, thanks Haakon:)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH for-next] RDMA/cma: Remove unnecessary INIT->INIT transition
  2021-06-17 15:46 [PATCH for-next] RDMA/cma: Remove unnecessary INIT->INIT transition Håkon Bugge
  2021-06-17 20:40   ` kernel test robot
  2021-06-20 12:19 ` Mark Zhang
@ 2021-06-21 10:07 ` Leon Romanovsky
  2021-06-21 12:14   ` Haakon Bugge
  2 siblings, 1 reply; 8+ messages in thread
From: Leon Romanovsky @ 2021-06-21 10:07 UTC (permalink / raw)
  To: Håkon Bugge; +Cc: Doug Ledford, Jason Gunthorpe, linux-rdma

On Thu, Jun 17, 2021 at 05:46:23PM +0200, Håkon Bugge wrote:
> In rdma_create_qp(), a connected QP will be transitioned to the INIT
> state.
> 
> Afterwards, the QP will be transitioned to the RTR state by the
> cma_modify_qp_rtr() function. But this function starts by performing
> an ib_modify_qp() to the INIT state again, before another
> ib_modify_qp() is performed to transition the QP to the RTR state.
> 
> Hence, there is no need to transition the QP to the INIT state in
> rdma_create_qp().
> 
> Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
> ---
>  drivers/infiniband/core/cma.c | 15 ---------------
>  1 file changed, 15 deletions(-)
> 

Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH for-next] RDMA/cma: Remove unnecessary INIT->INIT transition
  2021-06-21 10:07 ` Leon Romanovsky
@ 2021-06-21 12:14   ` Haakon Bugge
  0 siblings, 0 replies; 8+ messages in thread
From: Haakon Bugge @ 2021-06-21 12:14 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: Doug Ledford, Jason Gunthorpe, OFED mailing list



> On 21 Jun 2021, at 12:07, Leon Romanovsky <leon@kernel.org> wrote:
> 
> On Thu, Jun 17, 2021 at 05:46:23PM +0200, Håkon Bugge wrote:
>> In rdma_create_qp(), a connected QP will be transitioned to the INIT
>> state.
>> 
>> Afterwards, the QP will be transitioned to the RTR state by the
>> cma_modify_qp_rtr() function. But this function starts by performing
>> an ib_modify_qp() to the INIT state again, before another
>> ib_modify_qp() is performed to transition the QP to the RTR state.
>> 
>> Hence, there is no need to transition the QP to the INIT state in
>> rdma_create_qp().
>> 
>> Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
>> ---
>> drivers/infiniband/core/cma.c | 15 ---------------
>> 1 file changed, 15 deletions(-)
>> 
> 
> Thanks,
> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>

Thank you Leon!


Håkon



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-06-21 12:14 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-17 15:46 [PATCH for-next] RDMA/cma: Remove unnecessary INIT->INIT transition Håkon Bugge
2021-06-17 20:40 ` kernel test robot
2021-06-17 20:40   ` kernel test robot
2021-06-20 12:19 ` Mark Zhang
2021-06-21  8:44   ` Haakon Bugge
2021-06-21 12:05     ` Mark Zhang
2021-06-21 10:07 ` Leon Romanovsky
2021-06-21 12:14   ` Haakon Bugge

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.