[PATCH 00/15] IB/rxe patches for kernel v4.11

[PATCH 00/15] IB/rxe patches for kernel v4.11

[Bart Van Assche]

Hello Doug,

The patches in this series are what I came up with by reviewing and testing
the rdma_rxe driver. Please consider these patches for kernel v4.11.

Thanks,

Bart.

--

Bart Van Assche (15):
  IB/rxe: Suppress sparse warnings
  IB/rxe: Constify the pool name
  IB/rxe: Remove an unused function
  IB/rxe: Remove an unused variable
  IB/rxe: Remove superfluous casts
  IB/rxe: Enable type checking on SKB_TO_PKT() and PKT_TO_SKB() arguments
  IB/rxe: Let the compiler check the type of the cleanup functions
  IB/rxe: Issue warnings once
  IB/rxe: Add a runtime check in alloc_index()
  IB/rxe: Introduce functions for queue draining
  IB/rxe: Generate a completion for all failed work requests
  IB/rxe: Fix a MR reference leak in check_rkey()
  IB/rxe: Fix reference leaks in memory key invalidation code
  IB/rxe: Remove a pointless indirection layer
  IB/rxe: Fix an skb leak

 drivers/infiniband/sw/rxe/rxe.c       |  2 +-
 drivers/infiniband/sw/rxe/rxe_comp.c  | 83 +++++++++++++++++------------------
 drivers/infiniband/sw/rxe/rxe_cq.c    |  4 +-
 drivers/infiniband/sw/rxe/rxe_hdr.h   | 12 ++++-
 drivers/infiniband/sw/rxe/rxe_loc.h   | 29 +++++++++---
 drivers/infiniband/sw/rxe/rxe_mcast.c |  8 ++--
 drivers/infiniband/sw/rxe/rxe_mr.c    | 10 ++---
 drivers/infiniband/sw/rxe/rxe_net.c   | 51 +++++++--------------
 drivers/infiniband/sw/rxe/rxe_pool.c  | 14 ++----
 drivers/infiniband/sw/rxe/rxe_pool.h  |  8 ++--
 drivers/infiniband/sw/rxe/rxe_qp.c    |  4 +-
 drivers/infiniband/sw/rxe/rxe_recv.c  |  2 +-
 drivers/infiniband/sw/rxe/rxe_req.c   | 30 +++++--------
 drivers/infiniband/sw/rxe/rxe_resp.c  | 64 ++++++++++++++-------------
 drivers/infiniband/sw/rxe/rxe_verbs.c | 10 ++---
 drivers/infiniband/sw/rxe/rxe_verbs.h | 24 +---------
 16 files changed, 164 insertions(+), 191 deletions(-)

-- 
2.11.0

[PATCH 01/15] IB/rxe: Suppress sparse warnings

[Bart Van Assche]

Avoid that sparse complains about using 0 as a pointer. Avoid that
sparse complains about missing function declarations. Avoid that
sparse complains about
endianness.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Moni Shoua <monis@mellanox.com>
Cc: Andrew Boyer <andrew.boyer@dell.com>
---
 drivers/infiniband/sw/rxe/rxe_net.c  | 8 ++++----
 drivers/infiniband/sw/rxe/rxe_recv.c | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index 342e78163613..151f639abebf 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -243,8 +243,8 @@ static struct socket *rxe_setup_udp_tunnel(struct net *net, __be16 port,
 {
 	int err;
 	struct socket *sock;
-	struct udp_port_cfg udp_cfg = {0};
-	struct udp_tunnel_sock_cfg tnl_cfg = {0};
+	struct udp_port_cfg udp_cfg = { };
+	struct udp_tunnel_sock_cfg tnl_cfg = { };
 
 	if (ipv6) {
 		udp_cfg.family = AF_INET6;
@@ -658,7 +658,7 @@ struct notifier_block rxe_net_notifier = {
 	.notifier_call = rxe_notify,
 };
 
-int rxe_net_ipv4_init(void)
+static int rxe_net_ipv4_init(void)
 {
 	recv_sockets.sk4 = rxe_setup_udp_tunnel(&init_net,
 				htons(ROCE_V2_UDP_DPORT), false);
@@ -671,7 +671,7 @@ int rxe_net_ipv4_init(void)
 	return 0;
 }
 
-int rxe_net_ipv6_init(void)
+static int rxe_net_ipv6_init(void)
 {
 #if IS_ENABLED(CONFIG_IPV6)
 
diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c
index 252b4d637d45..50886031096f 100644
--- a/drivers/infiniband/sw/rxe/rxe_recv.c
+++ b/drivers/infiniband/sw/rxe/rxe_recv.c
@@ -389,7 +389,7 @@ int rxe_rcv(struct sk_buff *skb)
 	calc_icrc = rxe_icrc_hdr(pkt, skb);
 	calc_icrc = crc32_le(calc_icrc, (u8 *)payload_addr(pkt),
 			     payload_size(pkt));
-	calc_icrc = cpu_to_be32(~calc_icrc);
+	calc_icrc = (__force u32)cpu_to_be32(~calc_icrc);
 	if (unlikely(calc_icrc != pack_icrc)) {
 		if (skb->protocol == htons(ETH_P_IPV6))
 			pr_warn_ratelimited("bad ICRC from %pI6c\n",
-- 
2.11.0

Re: [PATCH 01/15] IB/rxe: Suppress sparse warnings

[Leon Romanovsky]

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

On Mon, Jan 02, 2017 at 10:39:05AM +0000, Bart Van Assche wrote:
> Avoid that sparse complains about using 0 as a pointer. Avoid that
> sparse complains about missing function declarations. Avoid that
> sparse complains about
> endianness.
>
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
> ---

Something wrong with commit message.

Thanks,
Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

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

Re: [PATCH 01/15] IB/rxe: Suppress sparse warnings

[Bart Van Assche]

On Thu, 2017-01-05 at 14:59 +0200, Leon Romanovsky wrote:
> On Mon, Jan 02, 2017 at 10:39:05AM +0000, Bart Van Assche wrote:
> > Avoid that sparse complains about using 0 as a pointer. Avoid that
> > sparse complains about missing function declarations. Avoid that
> > sparse complains about
> > endianness.
> > 
> > Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> > Cc: Moni Shoua <monis@mellanox.com>
> > Cc: Andrew Boyer <andrew.boyer@dell.com>
> > ---
> 
> Something wrong with commit message.

Hello Leon,

Are you referring to the formatting or to the contents of the commit
message?

Bart.

Re: [PATCH 01/15] IB/rxe: Suppress sparse warnings

[Leon Romanovsky]

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

On Mon, Jan 09, 2017 at 12:42:00PM +0000, Bart Van Assche wrote:
> On Thu, 2017-01-05 at 14:59 +0200, Leon Romanovsky wrote:
> > On Mon, Jan 02, 2017 at 10:39:05AM +0000, Bart Van Assche wrote:
> > > Avoid that sparse complains about using 0 as a pointer. Avoid that
> > > sparse complains about missing function declarations. Avoid that
> > > sparse complains about
> > > endianness.
> > >
> > > Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> > > Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
> > > ---
> >
> > Something wrong with commit message.
>
> Hello Leon,
>
> Are you referring to the formatting or to the contents of the commit
> message?

Both, it looks like the message was cut after last about and three
"avoid that sparse complains about ..." in the row was unpleasant to me.

Both these nitpicks don't worth to respin.

Thanks
>
> Bart.

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

Re: [PATCH 01/15] IB/rxe: Suppress sparse warnings

[Boyer, Andrew]

On 1/2/17, 5:39 AM, "Bart Van Assche" <Bart.VanAssche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> wrote:

>Avoid that sparse complains about using 0 as a pointer. Avoid that
>sparse complains about missing function declarations. Avoid that
>sparse complains about
>endianness.
>
>Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
>Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
>---
> drivers/infiniband/sw/rxe/rxe_net.c  | 8 ++++----
> drivers/infiniband/sw/rxe/rxe_recv.c | 2 +-
> 2 files changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@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 02/15] IB/rxe: Constify the pool name

[Bart Van Assche]

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Moni Shoua <monis@mellanox.com>
Cc: Andrew Boyer <andrew.boyer@dell.com>
---
 drivers/infiniband/sw/
rxe/rxe_pool.c | 2 +-
 drivers/infiniband/sw/rxe/rxe_pool.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_pool.c b/drivers/infiniband/sw/rxe/rxe_pool.c
index d723947a8542..238a6be00d46 100644
--- a/drivers/infiniband/sw/rxe/rxe_pool.c
+++ b/drivers/infiniband/sw/rxe/rxe_pool.c
@@ -102,7 +102,7 @@ struct rxe_type_info rxe_type_info[RXE_NUM_TYPES] = {
 	},
 };
 
-static inline char *pool_name(struct rxe_pool *pool)
+static inline const char *pool_name(struct rxe_pool *pool)
 {
 	return rxe_type_info[pool->type].name;
 }
diff --git a/drivers/infiniband/sw/rxe/rxe_pool.h b/drivers/infiniband/sw/rxe/rxe_pool.h
index 4d04830adcae..7846ccc58b25 100644
--- a/drivers/infiniband/sw/rxe/rxe_pool.h
+++ b/drivers/infiniband/sw/rxe/rxe_pool.h
@@ -58,7 +58,7 @@ enum rxe_elem_type {
 };
 
 struct rxe_type_info {
-	char			*name;
+	const char		*name;
 	size_t			size;
 	void			(*cleanup)(void *obj);
 	enum rxe_pool_flags	flags;
-- 
2.11.0

Re: [PATCH 02/15] IB/rxe: Constify the pool name

[Leon Romanovsky]

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

On Mon, Jan 02, 2017 at 10:39:22AM +0000, Bart Van Assche wrote:
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
> ---
>  drivers/infiniband/sw/
> rxe/rxe_pool.c | 2 +-
>  drivers/infiniband/sw/rxe/rxe_pool.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>

Thanks,
Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

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

Re: [PATCH 02/15] IB/rxe: Constify the pool name

[Boyer, Andrew]

On 1/2/17, 5:39 AM, "Bart Van Assche" <Bart.VanAssche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> wrote:

>Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
>Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
>---
> drivers/infiniband/sw/
>rxe/rxe_pool.c | 2 +-
> drivers/infiniband/sw/rxe/rxe_pool.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@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 03/15] IB/rxe: Remove an unused function

[Bart Van Assche]

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Moni Shoua <monis@mellanox.com>
Cc: Andrew Boyer <andrew.boyer@dell.com>
---
 drivers/infiniband/sw/
rxe/rxe_pool.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_pool.c b/drivers/infiniband/sw/rxe/rxe_pool.c
index 238a6be00d46..4fe11148b304 100644
--- a/drivers/infiniband/sw/rxe/rxe_pool.c
+++ b/drivers/infiniband/sw/rxe/rxe_pool.c
@@ -112,13 +112,6 @@ static inline struct kmem_cache *pool_cache(struct rxe_pool *pool)
 	return rxe_type_info[pool->type].cache;
 }
 
-static inline enum rxe_elem_type rxe_type(void *arg)
-{
-	struct rxe_pool_entry *elem = arg;
-
-	return elem->pool->type;
-}
-
 int rxe_cache_init(void)
 {
 	int err;
-- 
2.11.0

Re: [PATCH 03/15] IB/rxe: Remove an unused function

[Leon Romanovsky]

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

On Mon, Jan 02, 2017 at 10:39:39AM +0000, Bart Van Assche wrote:
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
> ---
>  drivers/infiniband/sw/
> rxe/rxe_pool.c | 7 -------
>  1 file changed, 7 deletions(-)
>

Thanks,
Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

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

Re: [PATCH 03/15] IB/rxe: Remove an unused function

[Boyer, Andrew]

On 1/2/17, 5:39 AM, "Bart Van Assche" <Bart.VanAssche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> wrote:

>Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
>Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
>---
> drivers/infiniband/sw/
>rxe/rxe_pool.c | 7 -------
> 1 file changed, 7 deletions(-)

Reviewed-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@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 04/15] IB/rxe: Remove an unused variable

[Bart Van Assche]

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

The variable 'av' is not used so remove it.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Moni Shoua <monis@mellanox.com>
Cc: Andrew Boyer <andrew.boyer@dell.com>
---
 drivers/infiniband/sw/rxe/rxe_req.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index 73d4a97603a1..b95b6035e988 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -364,16 +364,11 @@ static inline int check_init_depth(struct rxe_qp *qp, struct rxe_send_wqe *wqe)
 static inline int get_mtu(struct rxe_qp *qp, struct rxe_send_wqe *wqe)
 {
 	struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
-	struct rxe_port *port;
-	struct rxe_av *av;
 
 	if ((qp_type(qp) == IB_QPT_RC) || (qp_type(qp) == IB_QPT_UC))
 		return qp->mtu;
 
-	av = &wqe->av;
-	port = &rxe->port;
-
-	return port->mtu_cap;
+	return rxe->port.mtu_cap;
 }
 
 static struct sk_buff *init_req_packet(struct rxe_qp *qp,
-- 
2.11.0
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš

Re: [PATCH 04/15] IB/rxe: Remove an unused variable

[Leon Romanovsky]

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

On Mon, Jan 02, 2017 at 10:40:13AM +0000, Bart Van Assche wrote:
> The variable 'av' is not used so remove it.
>
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>

Thanks,
Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

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

Re: [PATCH 04/15] IB/rxe: Remove an unused variable

[Parav Pandit]

Hi Bart,

Additionally, since struct rxe_send_wqe is not used in get_mtu() function,
Can you please update signature to just have rxe_qp* as input argument?

Parav

On Jan 2, 2017 5:05 AM, "Bart Van Assche" <Bart.VanAssche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> wrote:

The variable 'av' is not used so remove it.

Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_req.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_req.c
b/drivers/infiniband/sw/rxe/rxe_req.c
index 73d4a97603a1..b95b6035e988 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -364,16 +364,11 @@ static inline int check_init_depth(struct rxe_qp
*qp, struct rxe_send_wqe *wqe)
 static inline int get_mtu(struct rxe_qp *qp, struct rxe_send_wqe *wqe)
 {
        struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
-       struct rxe_port *port;
-       struct rxe_av *av;

        if ((qp_type(qp) == IB_QPT_RC) || (qp_type(qp) == IB_QPT_UC))
                return qp->mtu;

-       av = &wqe->av;
-       port = &rxe->port;
-
-       return port->mtu_cap;
+       return rxe->port.mtu_cap;
 }

 static struct sk_buff *init_req_packet(struct rxe_qp *qp,
-- 
2.11.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 04/15] IB/rxe: Remove an unused variable

[Bart Van Assche]

On Tue, 2017-01-03 at 12:39 -0600, Parav Pandit wrote:
> Additionally, since struct rxe_send_wqe is not used in get_mtu() function,
> Can you please update signature to just have rxe_qp* as input argument?

Thanks for the feedback Leon and Parav. BTW, feedback for the other patches
of this series would also be welcome.

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 04/15] IB/rxe: Remove an unused variable

[Leon Romanovsky]

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

On Wed, Jan 04, 2017 at 10:37:07AM +0000, Bart Van Assche wrote:
> On Tue, 2017-01-03 at 12:39 -0600, Parav Pandit wrote:
> > Additionally, since struct rxe_send_wqe is not used in get_mtu() function,
> > Can you please update signature to just have rxe_qp* as input argument?
>
> Thanks for the feedback Leon and Parav. BTW, feedback for the other patches
> of this series would also be welcome.

I will try to do my best.
Thanks

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

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

Re: [PATCH 04/15] IB/rxe: Remove an unused variable

[Boyer, Andrew]

On 1/2/17, 5:40 AM, "Bart Van Assche" <Bart.VanAssche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> wrote:

>The variable 'av' is not used so remove it.
>
>Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
>Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
>---
> drivers/infiniband/sw/rxe/rxe_req.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)

Reviewed-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@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 05/15] IB/rxe: Remove superfluous casts

[Bart Van Assche]

Casting a pointer to 'void *' explicitly is not necessary in C code.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Moni Shoua <monis@mellanox.com>
Cc: Andrew Boyer <andrew.boyer@dell.com>
---
 drivers/infiniband/sw/rxe/rxe_pool.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_pool.c b/drivers/infiniband/sw/rxe/rxe_pool.c
index 4fe11148b304..7d1e2862b928 100644
--- a/drivers/infiniband/sw/rxe/rxe_pool.c
+++ b/drivers/infiniband/sw/rxe/rxe_pool.c
@@ -454,7 +454,7 @@ void *rxe_pool_get_index(struct rxe_pool *pool, u32 index)
 
 out:
 	spin_unlock_irqrestore(&pool->pool_lock, flags);
-	return node ? (void *)elem : NULL;
+	return node ? elem : NULL;
 }
 
 void *rxe_pool_get_key(struct rxe_pool *pool, void *key)
@@ -490,5 +490,5 @@ void *rxe_pool_get_key(struct rxe_pool *pool, void *key)
 
 out:
 	spin_unlock_irqrestore(&pool->pool_lock, flags);
-	return node ? ((void *)elem) : NULL;
+	return node ? elem : NULL;
 }
-- 
2.11.0

Re: [PATCH 05/15] IB/rxe: Remove superfluous casts

[Leon Romanovsky]

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

On Mon, Jan 02, 2017 at 10:40:30AM +0000, Bart Van Assche wrote:
> Casting a pointer to 'void *' explicitly is not necessary in C code.
>
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>

Thanks,
Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

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

Re: [PATCH 05/15] IB/rxe: Remove superfluous casts

[Boyer, Andrew]

On 1/2/17, 5:40 AM, "Bart Van Assche" <Bart.VanAssche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> wrote:

>Casting a pointer to 'void *' explicitly is not necessary in C code.
>
>Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
>Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
>---
> drivers/infiniband/sw/rxe/rxe_pool.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@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 06/15] IB/rxe: Enable type checking on SKB_TO_PKT() and PKT_TO_SKB() arguments

[Bart Van Assche]

Let the compiler check the type of the arguments passed to SKB_TO_PKT()
and PKT_TO_SKB().

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Moni Shoua <monis@mellanox.com>
Cc: Andrew Boyer <andrew.boyer@dell.com>
---
 drivers/infiniband/sw/rxe/rxe_hdr.h | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_hdr.h b/drivers/infiniband/sw/rxe/rxe_hdr.h
index d57b5e956ceb..6cb18406f5b8 100644
--- a/drivers/infiniband/sw/rxe/rxe_hdr.h
+++ b/drivers/infiniband/sw/rxe/rxe_hdr.h
@@ -53,8 +53,16 @@ struct rxe_pkt_info {
 };
 
 /* Macros should be used only for received skb */
-#define SKB_TO_PKT(skb) ((struct rxe_pkt_info *)(skb)->cb)
-#define PKT_TO_SKB(pkt) container_of((void *)(pkt), struct sk_buff, cb)
+static inline struct rxe_pkt_info *SKB_TO_PKT(struct sk_buff *skb)
+{
+	BUILD_BUG_ON(sizeof(struct rxe_pkt_info) > sizeof(skb->cb));
+	return (void *)skb->cb;
+}
+
+static inline struct sk_buff *PKT_TO_SKB(struct rxe_pkt_info *pkt)
+{
+	return container_of((void *)pkt, struct sk_buff, cb);
+}
 
 /*
  * IBA header types and methods
-- 
2.11.0

Re: [PATCH 06/15] IB/rxe: Enable type checking on SKB_TO_PKT() and PKT_TO_SKB() arguments

[Leon Romanovsky]

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

On Mon, Jan 02, 2017 at 10:40:49AM +0000, Bart Van Assche wrote:
> Let the compiler check the type of the arguments passed to SKB_TO_PKT()
> and PKT_TO_SKB().
>
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
> ---
>  drivers/infiniband/sw/rxe/rxe_hdr.h | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_hdr.h b/drivers/infiniband/sw/rxe/rxe_hdr.h
> index d57b5e956ceb..6cb18406f5b8 100644
> --- a/drivers/infiniband/sw/rxe/rxe_hdr.h
> +++ b/drivers/infiniband/sw/rxe/rxe_hdr.h
> @@ -53,8 +53,16 @@ struct rxe_pkt_info {
>  };
>  
>  /* Macros should be used only for received skb */
> -#define SKB_TO_PKT(skb) ((struct rxe_pkt_info *)(skb)->cb)
> -#define PKT_TO_SKB(pkt) container_of((void *)(pkt), struct sk_buff, cb)
> +static inline struct rxe_pkt_info *SKB_TO_PKT(struct sk_buff *skb)
> +{
> +	BUILD_BUG_ON(sizeof(struct rxe_pkt_info) > sizeof(skb->cb));

It is very good check, thanks for catching it.

> +	return (void *)skb->cb;
> +}
> +

Thanks,
Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

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

Re: [PATCH 06/15] IB/rxe: Enable type checking on SKB_TO_PKT() and PKT_TO_SKB() arguments

[Boyer, Andrew]

On 1/2/17, 5:40 AM, "linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org on behalf of Bart
Van Assche" <linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org on behalf of
Bart.VanAssche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> wrote:

>Let the compiler check the type of the arguments passed to SKB_TO_PKT()
>and PKT_TO_SKB().
>
>Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
>Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
>---
> drivers/infiniband/sw/rxe/rxe_hdr.h | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)

Reviewed-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@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 07/15] IB/rxe: Let the compiler check the type of the cleanup functions

[Bart Van Assche]

Change the argument type of these functions from void * into
struct rxe_pool_entry *.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Moni Shoua <monis@mellanox.com>
Cc: Andrew Boyer <andrew.boyer@dell.com>
---
 drivers/infiniband/sw/rxe/rxe_cq.c    | 4 ++--
 drivers/infiniband/sw/rxe/rxe_loc.h   | 8 ++++----
 drivers/infiniband/sw/rxe/rxe_mcast.c | 4 ++--
 drivers/infiniband/sw/rxe/rxe_mr.c    | 4 ++--
 drivers/infiniband/sw/rxe/rxe_pool.h  | 6 ++++--
 drivers/infiniband/sw/rxe/rxe_qp.c    | 4 ++--
 drivers/infiniband/sw/rxe/rxe_verbs.h | 2 +-
 7 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_cq.c b/drivers/infiniband/sw/rxe/rxe_cq.c
index e5e6a5e7dee9..49fe42c23f4d 100644
--- a/drivers/infiniband/sw/rxe/rxe_cq.c
+++ b/drivers/infiniband/sw/rxe/rxe_cq.c
@@ -156,9 +156,9 @@ int rxe_cq_post(struct rxe_cq *cq, struct rxe_cqe *cqe, int solicited)
 	return 0;
 }
 
-void rxe_cq_cleanup(void *arg)
+void rxe_cq_cleanup(struct rxe_pool_entry *arg)
 {
-	struct rxe_cq *cq = arg;
+	struct rxe_cq *cq = container_of(arg, typeof(*cq), pelem);
 
 	if (cq->queue)
 		rxe_queue_cleanup(cq->queue);
diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
index efe4c6a35442..da191d7acb6f 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -64,7 +64,7 @@ int rxe_cq_resize_queue(struct rxe_cq *cq, int new_cqe, struct ib_udata *udata);
 
 int rxe_cq_post(struct rxe_cq *cq, struct rxe_cqe *cqe, int solicited);
 
-void rxe_cq_cleanup(void *arg);
+void rxe_cq_cleanup(struct rxe_pool_entry *arg);
 
 /* rxe_mcast.c */
 int rxe_mcast_get_grp(struct rxe_dev *rxe, union ib_gid *mgid,
@@ -78,7 +78,7 @@ int rxe_mcast_drop_grp_elem(struct rxe_dev *rxe, struct rxe_qp *qp,
 
 void rxe_drop_all_mcast_groups(struct rxe_qp *qp);
 
-void rxe_mc_cleanup(void *arg);
+void rxe_mc_cleanup(struct rxe_pool_entry *arg);
 
 /* rxe_mmap.c */
 struct rxe_mmap_info {
@@ -137,7 +137,7 @@ int mem_check_range(struct rxe_mem *mem, u64 iova, size_t length);
 int rxe_mem_map_pages(struct rxe_dev *rxe, struct rxe_mem *mem,
 		      u64 *page, int num_pages, u64 iova);
 
-void rxe_mem_cleanup(void *arg);
+void rxe_mem_cleanup(struct rxe_pool_entry *arg);
 
 int advance_dma_data(struct rxe_dma_info *dma, unsigned int length);
 
@@ -162,7 +162,7 @@ void rxe_qp_error(struct rxe_qp *qp);
 
 void rxe_qp_destroy(struct rxe_qp *qp);
 
-void rxe_qp_cleanup(void *arg);
+void rxe_qp_cleanup(struct rxe_pool_entry *arg);
 
 static inline int qp_num(struct rxe_qp *qp)
 {
diff --git a/drivers/infiniband/sw/rxe/rxe_mcast.c b/drivers/infiniband/sw/rxe/rxe_mcast.c
index fa95544ca7e0..e0fb6752f90e 100644
--- a/drivers/infiniband/sw/rxe/rxe_mcast.c
+++ b/drivers/infiniband/sw/rxe/rxe_mcast.c
@@ -180,9 +180,9 @@ void rxe_drop_all_mcast_groups(struct rxe_qp *qp)
 	}
 }
 
-void rxe_mc_cleanup(void *arg)
+void rxe_mc_cleanup(struct rxe_pool_entry *arg)
 {
-	struct rxe_mc_grp *grp = arg;
+	struct rxe_mc_grp *grp = container_of(arg, typeof(*grp), pelem);
 	struct rxe_dev *rxe = grp->rxe;
 
 	rxe_drop_key(grp);
diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
index d0faca294006..8ca3acd327b3 100644
--- a/drivers/infiniband/sw/rxe/rxe_mr.c
+++ b/drivers/infiniband/sw/rxe/rxe_mr.c
@@ -89,9 +89,9 @@ static void rxe_mem_init(int access, struct rxe_mem *mem)
 	mem->map_shift		= ilog2(RXE_BUF_PER_MAP);
 }
 
-void rxe_mem_cleanup(void *arg)
+void rxe_mem_cleanup(struct rxe_pool_entry *arg)
 {
-	struct rxe_mem *mem = arg;
+	struct rxe_mem *mem = container_of(arg, typeof(*mem), pelem);
 	int i;
 
 	if (mem->umem)
diff --git a/drivers/infiniband/sw/rxe/rxe_pool.h b/drivers/infiniband/sw/rxe/rxe_pool.h
index 7846ccc58b25..47df28e43acf 100644
--- a/drivers/infiniband/sw/rxe/rxe_pool.h
+++ b/drivers/infiniband/sw/rxe/rxe_pool.h
@@ -57,10 +57,12 @@ enum rxe_elem_type {
 	RXE_NUM_TYPES,		/* keep me last */
 };
 
+struct rxe_pool_entry;
+
 struct rxe_type_info {
 	const char		*name;
 	size_t			size;
-	void			(*cleanup)(void *obj);
+	void			(*cleanup)(struct rxe_pool_entry *obj);
 	enum rxe_pool_flags	flags;
 	u32			max_index;
 	u32			min_index;
@@ -91,7 +93,7 @@ struct rxe_pool {
 	spinlock_t              pool_lock; /* pool spinlock */
 	size_t			elem_size;
 	struct kref		ref_cnt;
-	void			(*cleanup)(void *obj);
+	void			(*cleanup)(struct rxe_pool_entry *obj);
 	enum rxe_pool_state	state;
 	enum rxe_pool_flags	flags;
 	enum rxe_elem_type	type;
diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
index 486d576e55bc..917147ce4cf9 100644
--- a/drivers/infiniband/sw/rxe/rxe_qp.c
+++ b/drivers/infiniband/sw/rxe/rxe_qp.c
@@ -825,9 +825,9 @@ void rxe_qp_destroy(struct rxe_qp *qp)
 }
 
 /* called when the last reference to the qp is dropped */
-void rxe_qp_cleanup(void *arg)
+void rxe_qp_cleanup(struct rxe_pool_entry *arg)
 {
-	struct rxe_qp *qp = arg;
+	struct rxe_qp *qp = container_of(arg, typeof(*qp), pelem);
 
 	rxe_drop_all_mcast_groups(qp);
 
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
index cac1d52a08f0..536974b69ed9 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.h
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
@@ -475,6 +475,6 @@ static inline struct rxe_mem *to_rmw(struct ib_mw *mw)
 int rxe_register_device(struct rxe_dev *rxe);
 int rxe_unregister_device(struct rxe_dev *rxe);
 
-void rxe_mc_cleanup(void *arg);
+void rxe_mc_cleanup(struct rxe_pool_entry *arg);
 
 #endif /* RXE_VERBS_H */
-- 
2.11.0

Re: [PATCH 07/15] IB/rxe: Let the compiler check the type of the cleanup functions

[Leon Romanovsky]

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

On Mon, Jan 02, 2017 at 10:41:23AM +0000, Bart Van Assche wrote:
> Change the argument type of these functions from void * into
> struct rxe_pool_entry *.
>
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
> ---
>  drivers/infiniband/sw/rxe/rxe_cq.c    | 4 ++--
>  drivers/infiniband/sw/rxe/rxe_loc.h   | 8 ++++----
>  drivers/infiniband/sw/rxe/rxe_mcast.c | 4 ++--
>  drivers/infiniband/sw/rxe/rxe_mr.c    | 4 ++--
>  drivers/infiniband/sw/rxe/rxe_pool.h  | 6 ++++--
>  drivers/infiniband/sw/rxe/rxe_qp.c    | 4 ++--
>  drivers/infiniband/sw/rxe/rxe_verbs.h | 2 +-
>  7 files changed, 17 insertions(+), 15 deletions(-)
>

Thanks,
Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

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

Re: [PATCH 07/15] IB/rxe: Let the compiler check the type of the cleanup functions

[Boyer, Andrew]

On 1/2/17, 5:41 AM, "Bart Van Assche" <Bart.VanAssche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> wrote:

>Change the argument type of these functions from void * into
>struct rxe_pool_entry *.
>
>Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
>Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
>---
> drivers/infiniband/sw/rxe/rxe_cq.c    | 4 ++--
> drivers/infiniband/sw/rxe/rxe_loc.h   | 8 ++++----
> drivers/infiniband/sw/rxe/rxe_mcast.c | 4 ++--
> drivers/infiniband/sw/rxe/rxe_mr.c    | 4 ++--
> drivers/infiniband/sw/rxe/rxe_pool.h  | 6 ++++--
> drivers/infiniband/sw/rxe/rxe_qp.c    | 4 ++--
> drivers/infiniband/sw/rxe/rxe_verbs.h | 2 +-
> 7 files changed, 17 insertions(+), 15 deletions(-)

Reviewed-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@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 08/15] IB/rxe: Issue warnings once

[Bart Van Assche]

It is strongly recommended to report kernel warnings once instead
of every time a condition is hit. Hence change WARN_ON() into
WARN_ON_ONCE() / BUILD_BUG_ON() as
appropriate.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Moni Shoua <monis@mellanox.com>
Cc: Andrew Boyer <andrew.boyer@dell.com>
---
 drivers/infiniband/sw/rxe/rxe_comp.c |  2 +-
 drivers/infiniband/sw/rxe/rxe_mr.c   |  6 +++---
 drivers/infiniband/sw/rxe/rxe_resp.c | 11 ++++++-----
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c
index d369f24425f9..e912e5396e8c 100644
--- a/drivers/infiniband/sw/rxe/rxe_comp.c
+++ b/drivers/infiniband/sw/rxe/rxe_comp.c
@@ -254,7 +254,7 @@ static inline enum comp_state check_ack(struct rxe_qp *qp,
 		}
 		break;
 	default:
-		WARN_ON(1);
+		WARN_ON_ONCE(1);
 	}
 
 	/* Check operation validity. */
diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
index 8ca3acd327b3..8cf38b253c37 100644
--- a/drivers/infiniband/sw/rxe/rxe_mr.c
+++ b/drivers/infiniband/sw/rxe/rxe_mr.c
@@ -123,7 +123,7 @@ static int rxe_mem_alloc(struct rxe_dev *rxe, struct rxe_mem *mem, int num_buf)
 			goto err2;
 	}
 
-	WARN_ON(!is_power_of_2(RXE_BUF_PER_MAP));
+	BUILD_BUG_ON(!is_power_of_2(RXE_BUF_PER_MAP));
 
 	mem->map_shift	= ilog2(RXE_BUF_PER_MAP);
 	mem->map_mask	= RXE_BUF_PER_MAP - 1;
@@ -189,7 +189,7 @@ int rxe_mem_init_user(struct rxe_dev *rxe, struct rxe_pd *pd, u64 start,
 		goto err1;
 	}
 
-	WARN_ON(!is_power_of_2(umem->page_size));
+	WARN_ON_ONCE(!is_power_of_2(umem->page_size));
 
 	mem->page_shift		= ilog2(umem->page_size);
 	mem->page_mask		= umem->page_size - 1;
@@ -375,7 +375,7 @@ int rxe_mem_copy(struct rxe_mem *mem, u64 iova, void *addr, int length,
 		return 0;
 	}
 
-	WARN_ON(!mem->map);
+	WARN_ON_ONCE(!mem->map);
 
 	err = mem_check_range(mem, iova, length);
 	if (err) {
diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index 3435efff8799..6dbd069689fc 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -307,7 +307,7 @@ static enum resp_states check_op_valid(struct rxe_qp *qp,
 		break;
 
 	default:
-		WARN_ON(1);
+		WARN_ON_ONCE(1);
 		break;
 	}
 
@@ -495,7 +495,7 @@ static enum resp_states check_rkey(struct rxe_qp *qp,
 		}
 	}
 
-	WARN_ON(qp->resp.mr);
+	WARN_ON_ONCE(qp->resp.mr);
 
 	qp->resp.mr = mem;
 	return RESPST_EXECUTE;
@@ -808,9 +808,10 @@ static enum resp_states execute(struct rxe_qp *qp, struct rxe_pkt_info *pkt)
 		err = process_atomic(qp, pkt);
 		if (err)
 			return err;
-	} else
+	} else {
 		/* Unreachable */
-		WARN_ON(1);
+		WARN_ON_ONCE(1);
+	}
 
 	/* We successfully processed this new request. */
 	qp->resp.msn++;
@@ -1396,7 +1397,7 @@ int rxe_responder(void *arg)
 			goto exit;
 
 		default:
-			WARN_ON(1);
+			WARN_ON_ONCE(1);
 		}
 	}
 
-- 
2.11.0

Re: [PATCH 08/15] IB/rxe: Issue warnings once

[Leon Romanovsky]

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

On Mon, Jan 02, 2017 at 10:41:40AM +0000, Bart Van Assche wrote:
> It is strongly recommended to report kernel warnings once instead
> of every time a condition is hit. Hence change WARN_ON() into
> WARN_ON_ONCE() / BUILD_BUG_ON() as
> appropriate.
>
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
> ---
>  drivers/infiniband/sw/rxe/rxe_comp.c |  2 +-
>  drivers/infiniband/sw/rxe/rxe_mr.c   |  6 +++---
>  drivers/infiniband/sw/rxe/rxe_resp.c | 11 ++++++-----
>  3 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c
> index d369f24425f9..e912e5396e8c 100644
> --- a/drivers/infiniband/sw/rxe/rxe_comp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_comp.c
> @@ -254,7 +254,7 @@ static inline enum comp_state check_ack(struct rxe_qp *qp,
>  		}
>  		break;
>  	default:
> -		WARN_ON(1);
> +		WARN_ON_ONCE(1);
>  	}
>  
>  	/* Check operation validity. */
> diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
> index 8ca3acd327b3..8cf38b253c37 100644
> --- a/drivers/infiniband/sw/rxe/rxe_mr.c
> +++ b/drivers/infiniband/sw/rxe/rxe_mr.c
> @@ -123,7 +123,7 @@ static int rxe_mem_alloc(struct rxe_dev *rxe, struct rxe_mem *mem, int num_buf)
>  			goto err2;
>  	}
>  
> -	WARN_ON(!is_power_of_2(RXE_BUF_PER_MAP));
> +	BUILD_BUG_ON(!is_power_of_2(RXE_BUF_PER_MAP));
>  
>  	mem->map_shift	= ilog2(RXE_BUF_PER_MAP);
>  	mem->map_mask	= RXE_BUF_PER_MAP - 1;
> @@ -189,7 +189,7 @@ int rxe_mem_init_user(struct rxe_dev *rxe, struct rxe_pd *pd, u64 start,
>  		goto err1;
>  	}
>  
> -	WARN_ON(!is_power_of_2(umem->page_size));
> +	WARN_ON_ONCE(!is_power_of_2(umem->page_size));
>  
>  	mem->page_shift		= ilog2(umem->page_size);
>  	mem->page_mask		= umem->page_size - 1;
> @@ -375,7 +375,7 @@ int rxe_mem_copy(struct rxe_mem *mem, u64 iova, void *addr, int length,
>  		return 0;
>  	}
>  
> -	WARN_ON(!mem->map);
> +	WARN_ON_ONCE(!mem->map);
>  
>  	err = mem_check_range(mem, iova, length);
>  	if (err) {
> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
> index 3435efff8799..6dbd069689fc 100644
> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
> @@ -307,7 +307,7 @@ static enum resp_states check_op_valid(struct rxe_qp *qp,
>  		break;
>  
>  	default:
> -		WARN_ON(1);
> +		WARN_ON_ONCE(1);
>  		break;
>  	}
>  
> @@ -495,7 +495,7 @@ static enum resp_states check_rkey(struct rxe_qp *qp,
>  		}
>  	}
>  
> -	WARN_ON(qp->resp.mr);
> +	WARN_ON_ONCE(qp->resp.mr);
>  
>  	qp->resp.mr = mem;
>  	return RESPST_EXECUTE;
> @@ -808,9 +808,10 @@ static enum resp_states execute(struct rxe_qp *qp, struct rxe_pkt_info *pkt)
>  		err = process_atomic(qp, pkt);
>  		if (err)
>  			return err;
> -	} else
> +	} else {
>  		/* Unreachable */
> -		WARN_ON(1);
> +		WARN_ON_ONCE(1);
> +	}

This function (execute(..)) and the comment looks very suspicious to me.
It seems that the orginal author had intention to check if mask has
specific bit on, but current implmentation allows multiple bits and
has execution priority between bits in mask.

Moni,
Did you do it on purpose?

>  
>  	/* We successfully processed this new request. */
>  	qp->resp.msn++;
> @@ -1396,7 +1397,7 @@ int rxe_responder(void *arg)
>  			goto exit;
>  
>  		default:
> -			WARN_ON(1);
> +			WARN_ON_ONCE(1);
>  		}
>  	}
>  
> -- 
> 2.11.0

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

[PATCH 09/15] IB/rxe: Add a runtime check in alloc_index()

[Bart Van Assche]

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

Since index values equal to or above 'range' can trigger memory
corruption, complain if index >= range.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Moni Shoua <monis@mellanox.com>
Cc: Andrew Boyer <andrew.boyer@dell.com>
---
 drivers/infiniband/sw/rxe/rxe_pool.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/infiniband/sw/rxe/rxe_pool.c b/drivers/infiniband/sw/rxe/rxe_pool.c
index 7d1e2862b928..75d11ee635ec 100644
--- a/drivers/infiniband/sw/rxe/rxe_pool.c
+++ b/drivers/infiniband/sw/rxe/rxe_pool.c
@@ -266,6 +266,7 @@ static u32 alloc_index(struct rxe_pool *pool)
 	if (index >= range)
 		index = find_first_zero_bit(pool->table, range);
 
+	WARN_ON_ONCE(index >= range);
 	set_bit(index, pool->table);
 	pool->last = index;
 	return index + pool->min_index;
-- 
2.11.0
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš

Re: [PATCH 09/15] IB/rxe: Add a runtime check in alloc_index()

[Leon Romanovsky]

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

On Mon, Jan 02, 2017 at 10:41:58AM +0000, Bart Van Assche wrote:
> Since index values equal to or above 'range' can trigger memory
> corruption, complain if index >= range.

It looks like severe bug and the proper error folding is needed, if it
is a real situation.

>
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
> ---
>  drivers/infiniband/sw/rxe/rxe_pool.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_pool.c b/drivers/infiniband/sw/rxe/rxe_pool.c
> index 7d1e2862b928..75d11ee635ec 100644
> --- a/drivers/infiniband/sw/rxe/rxe_pool.c
> +++ b/drivers/infiniband/sw/rxe/rxe_pool.c
> @@ -266,6 +266,7 @@ static u32 alloc_index(struct rxe_pool *pool)
>  	if (index >= range)
>  		index = find_first_zero_bit(pool->table, range);
>  
> +	WARN_ON_ONCE(index >= range);
>  	set_bit(index, pool->table);
>  	pool->last = index;
>  	return index + pool->min_index;
> -- 
> 2.11.0
> N?????r??y????b?X??ǧv?^?)޺{.n?+????{??ٚ?{ay?\x1dʇڙ?,j\a??f???h???z?\x1e?w???\f???j:+v???w?j?m????\a????zZ+?????ݢj"??!

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

Re: [PATCH 09/15] IB/rxe: Add a runtime check in alloc_index()

[Bart Van Assche]

On Wed, 2017-01-04 at 19:27 +0200, Leon Romanovsky wrote:
> On Mon, Jan 02, 2017 at 10:41:58AM +0000, Bart Van Assche wrote:
> > Since index values equal to or above 'range' can trigger memory
> > corruption, complain if index >= range.
> 
> It looks like severe bug and the proper error folding is needed, if it
> is a real situation.

Code in other functions guarantees that this condition won't be hit. The warning
I added is intended to catch bugs if that other code would ever be modified.

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 09/15] IB/rxe: Add a runtime check in alloc_index()

[Leon Romanovsky]

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

On Wed, Jan 04, 2017 at 06:38:45PM +0000, Bart Van Assche wrote:
> On Wed, 2017-01-04 at 19:27 +0200, Leon Romanovsky wrote:
> > On Mon, Jan 02, 2017 at 10:41:58AM +0000, Bart Van Assche wrote:
> > > Since index values equal to or above 'range' can trigger memory
> > > corruption, complain if index >= range.
> >
> > It looks like severe bug and the proper error folding is needed, if it
> > is a real situation.
>
> Code in other functions guarantees that this condition won't be hit. The warning
> I added is intended to catch bugs if that other code would ever be modified.

It sounds reasonable.
Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Thanks.

>
> Bart.

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

Re: [PATCH 09/15] IB/rxe: Add a runtime check in alloc_index()

[Boyer, Andrew]

On 1/2/17, 5:41 AM, "Bart Van Assche" <Bart.VanAssche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> wrote:

>Since index values equal to or above 'range' can trigger memory
>corruption, complain if index >= range.
>
>Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
>Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
>---
> drivers/infiniband/sw/rxe/rxe_pool.c | 1 +
> 1 file changed, 1 insertion(+)

Reviewed-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@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 10/15] IB/rxe: Introduce functions for queue draining

[Bart Van Assche]

This change makes the code easier to read and avoids that code is
duplicated.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Moni Shoua <monis@mellanox.com>
Cc: Andrew Boyer <andrew.boyer@dell.com>
---
 drivers/infiniband/sw/rxe/rxe_comp.c | 63 +++++++++++++-----------------------
 drivers/infiniband/sw/rxe/rxe_resp.c | 28 ++++++++--------
 2 files changed, 38 insertions(+), 53 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c
index e912e5396e8c..6769a075501e 100644
--- a/drivers/infiniband/sw/rxe/rxe_comp.c
+++ b/drivers/infiniband/sw/rxe/rxe_comp.c
@@ -503,57 +503,40 @@ static inline enum comp_state complete_wqe(struct rxe_qp *qp,
 	return COMPST_GET_WQE;
 }
 
-int rxe_completer(void *arg)
+static void rxe_drain_resp_pkts(struct rxe_qp *qp, bool notify)
 {
-	struct rxe_qp *qp = (struct rxe_qp *)arg;
-	struct rxe_send_wqe *wqe = wqe;
-	struct sk_buff *skb = NULL;
-	struct rxe_pkt_info *pkt = NULL;
-	enum comp_state state;
-
-	rxe_add_ref(qp);
-
-	if (!qp->valid) {
-		while ((skb = skb_dequeue(&qp->resp_pkts))) {
-			rxe_drop_ref(qp);
-			kfree_skb(skb);
-		}
-		skb = NULL;
-		pkt = NULL;
-
-		while (queue_head(qp->sq.queue))
-			advance_consumer(qp->sq.queue);
+	struct sk_buff *skb;
+	struct rxe_send_wqe *wqe;
 
-		goto exit;
+	while ((skb = skb_dequeue(&qp->resp_pkts))) {
+		rxe_drop_ref(qp);
+		kfree_skb(skb);
 	}
 
-	if (qp->req.state == QP_STATE_ERROR) {
-		while ((skb = skb_dequeue(&qp->resp_pkts))) {
-			rxe_drop_ref(qp);
-			kfree_skb(skb);
-		}
-		skb = NULL;
-		pkt = NULL;
-
-		while ((wqe = queue_head(qp->sq.queue))) {
+	while ((wqe = queue_head(qp->sq.queue))) {
+		if (notify) {
 			wqe->status = IB_WC_WR_FLUSH_ERR;
 			do_complete(qp, wqe);
+		} else {
+			advance_consumer(qp->sq.queue);
 		}
-
-		goto exit;
 	}
+}
 
-	if (qp->req.state == QP_STATE_RESET) {
-		while ((skb = skb_dequeue(&qp->resp_pkts))) {
-			rxe_drop_ref(qp);
-			kfree_skb(skb);
-		}
-		skb = NULL;
-		pkt = NULL;
+int rxe_completer(void *arg)
+{
+	struct rxe_qp *qp = (struct rxe_qp *)arg;
+	struct rxe_send_wqe *wqe = wqe;
+	struct sk_buff *skb = NULL;
+	struct rxe_pkt_info *pkt = NULL;
+	enum comp_state state;
 
-		while (queue_head(qp->sq.queue))
-			advance_consumer(qp->sq.queue);
+	rxe_add_ref(qp);
 
+	if (!qp->valid || qp->req.state == QP_STATE_ERROR ||
+	    qp->req.state == QP_STATE_RESET) {
+		rxe_drain_resp_pkts(qp, qp->valid &&
+				    qp->req.state == QP_STATE_ERROR);
 		goto exit;
 	}
 
diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index 6dbd069689fc..25951e9413b8 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -1207,6 +1207,19 @@ static enum resp_states do_class_d1e_error(struct rxe_qp *qp)
 	}
 }
 
+static void rxe_drain_req_pkts(struct rxe_qp *qp, bool notify)
+{
+	struct sk_buff *skb;
+
+	while ((skb = skb_dequeue(&qp->req_pkts))) {
+		rxe_drop_ref(qp);
+		kfree_skb(skb);
+	}
+
+	while (!qp->srq && qp->rq.queue && queue_head(qp->rq.queue))
+		advance_consumer(qp->rq.queue);
+}
+
 int rxe_responder(void *arg)
 {
 	struct rxe_qp *qp = (struct rxe_qp *)arg;
@@ -1374,21 +1387,10 @@ int rxe_responder(void *arg)
 
 			goto exit;
 
-		case RESPST_RESET: {
-			struct sk_buff *skb;
-
-			while ((skb = skb_dequeue(&qp->req_pkts))) {
-				rxe_drop_ref(qp);
-				kfree_skb(skb);
-			}
-
-			while (!qp->srq && qp->rq.queue &&
-			       queue_head(qp->rq.queue))
-				advance_consumer(qp->rq.queue);
-
+		case RESPST_RESET:
+			rxe_drain_req_pkts(qp, false);
 			qp->resp.wqe = NULL;
 			goto exit;
-		}
 
 		case RESPST_ERROR:
 			qp->resp.goto_error = 0;
-- 
2.11.0

Re: [PATCH 10/15] IB/rxe: Introduce functions for queue draining

[Boyer, Andrew]

On 1/2/17, 5:42 AM, "Bart Van Assche" <Bart.VanAssche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> wrote:

>This change makes the code easier to read and avoids that code is
>duplicated.
>
>Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
>Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
>---
> drivers/infiniband/sw/rxe/rxe_comp.c | 63
>+++++++++++++-----------------------
> drivers/infiniband/sw/rxe/rxe_resp.c | 28 ++++++++--------
> 2 files changed, 38 insertions(+), 53 deletions(-)
>
>diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c
>b/drivers/infiniband/sw/rxe/rxe_comp.c
>index e912e5396e8c..6769a075501e 100644
>--- a/drivers/infiniband/sw/rxe/rxe_comp.c
>+++ b/drivers/infiniband/sw/rxe/rxe_comp.c
>@@ -503,57 +503,40 @@ static inline enum comp_state complete_wqe(struct
>rxe_qp *qp,
> 	return COMPST_GET_WQE;
> }
> 
>-int rxe_completer(void *arg)
>+static void rxe_drain_resp_pkts(struct rxe_qp *qp, bool notify)
> {
>-	struct rxe_qp *qp = (struct rxe_qp *)arg;
>-	struct rxe_send_wqe *wqe = wqe;
>-	struct sk_buff *skb = NULL;
>-	struct rxe_pkt_info *pkt = NULL;
>-	enum comp_state state;
>-
>-	rxe_add_ref(qp);
>-
>-	if (!qp->valid) {
>-		while ((skb = skb_dequeue(&qp->resp_pkts))) {
>-			rxe_drop_ref(qp);
>-			kfree_skb(skb);
>-		}
>-		skb = NULL;
>-		pkt = NULL;
>-
>-		while (queue_head(qp->sq.queue))
>-			advance_consumer(qp->sq.queue);
>+	struct sk_buff *skb;
>+	struct rxe_send_wqe *wqe;
> 
>-		goto exit;
>+	while ((skb = skb_dequeue(&qp->resp_pkts))) {
>+		rxe_drop_ref(qp);
>+		kfree_skb(skb);
> 	}
> 
>-	if (qp->req.state == QP_STATE_ERROR) {
>-		while ((skb = skb_dequeue(&qp->resp_pkts))) {
>-			rxe_drop_ref(qp);
>-			kfree_skb(skb);
>-		}
>-		skb = NULL;
>-		pkt = NULL;
>-
>-		while ((wqe = queue_head(qp->sq.queue))) {
>+	while ((wqe = queue_head(qp->sq.queue))) {
>+		if (notify) {
> 			wqe->status = IB_WC_WR_FLUSH_ERR;
> 			do_complete(qp, wqe);
>+		} else {
>+			advance_consumer(qp->sq.queue);
> 		}
>-
>-		goto exit;
> 	}
>+}
> 
>-	if (qp->req.state == QP_STATE_RESET) {
>-		while ((skb = skb_dequeue(&qp->resp_pkts))) {
>-			rxe_drop_ref(qp);
>-			kfree_skb(skb);
>-		}
>-		skb = NULL;
>-		pkt = NULL;
>+int rxe_completer(void *arg)
>+{
>+	struct rxe_qp *qp = (struct rxe_qp *)arg;
>+	struct rxe_send_wqe *wqe = wqe;
>+	struct sk_buff *skb = NULL;
>+	struct rxe_pkt_info *pkt = NULL;
>+	enum comp_state state;
> 
>-		while (queue_head(qp->sq.queue))
>-			advance_consumer(qp->sq.queue);
>+	rxe_add_ref(qp);
> 
>+	if (!qp->valid || qp->req.state == QP_STATE_ERROR ||
>+	    qp->req.state == QP_STATE_RESET) {
>+		rxe_drain_resp_pkts(qp, qp->valid &&
>+				    qp->req.state == QP_STATE_ERROR);
> 		goto exit;
> 	}
> 
>diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c
>b/drivers/infiniband/sw/rxe/rxe_resp.c
>index 6dbd069689fc..25951e9413b8 100644
>--- a/drivers/infiniband/sw/rxe/rxe_resp.c
>+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
>@@ -1207,6 +1207,19 @@ static enum resp_states do_class_d1e_error(struct
>rxe_qp *qp)
> 	}
> }
> 
>+static void rxe_drain_req_pkts(struct rxe_qp *qp, bool notify)

Is notify unused? Is it here just for symmetry with the responder?

>+{
>+	struct sk_buff *skb;
>+
>+	while ((skb = skb_dequeue(&qp->req_pkts))) {
>+		rxe_drop_ref(qp);
>+		kfree_skb(skb);
>+	}
>+
>+	while (!qp->srq && qp->rq.queue && queue_head(qp->rq.queue))
>+		advance_consumer(qp->rq.queue);
>+}
>+
> int rxe_responder(void *arg)
> {
> 	struct rxe_qp *qp = (struct rxe_qp *)arg;
>@@ -1374,21 +1387,10 @@ int rxe_responder(void *arg)
> 
> 			goto exit;
> 
>-		case RESPST_RESET: {
>-			struct sk_buff *skb;
>-
>-			while ((skb = skb_dequeue(&qp->req_pkts))) {
>-				rxe_drop_ref(qp);
>-				kfree_skb(skb);
>-			}
>-
>-			while (!qp->srq && qp->rq.queue &&
>-			       queue_head(qp->rq.queue))
>-				advance_consumer(qp->rq.queue);
>-
>+		case RESPST_RESET:
>+			rxe_drain_req_pkts(qp, false);
> 			qp->resp.wqe = NULL;
> 			goto exit;
>-		}
> 
> 		case RESPST_ERROR:
> 			qp->resp.goto_error = 0;
>-- 
>2.11.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 10/15] IB/rxe: Introduce functions for queue draining

[Boyer, Andrew]

On 1/9/17, 10:06 AM, "linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org on behalf of Boyer,
Andrew" <linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org on behalf of
Andrew.Boyer-8PEkshWhKlo@public.gmane.org> wrote:
>On 1/2/17, 5:42 AM, "Bart Van Assche" <Bart.VanAssche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> wrote:
>
>>This change makes the code easier to read and avoids that code is
>>duplicated.
>>
>>Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
>>Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
>>---
>> drivers/infiniband/sw/rxe/rxe_comp.c | 63
>>+++++++++++++-----------------------
>> drivers/infiniband/sw/rxe/rxe_resp.c | 28 ++++++++--------
>> 2 files changed, 38 insertions(+), 53 deletions(-)
...
>>
>> }
>> 
>>+static void rxe_drain_req_pkts(struct rxe_qp *qp, bool notify)
>
>Is notify unused? Is it here just for symmetry with the responder?

Never mind, I see it used in patch 11.

Reviewed-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@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 11/15] IB/rxe: Generate a completion for all failed work requests

[Bart Van Assche]

Change do_complete() such that an error completion is not only
generated if a QP is in the error state but also if a work request
failed.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Moni Shoua <monis@mellanox.com>
Cc: Andrew Boyer <andrew.boyer@dell.com>
---
 drivers/infiniband/sw/rxe/rxe_comp.c |  3 ++-
 drivers/infiniband/sw/rxe/rxe_loc.h  |  1 +
 drivers/infiniband/sw/rxe/rxe_req.c  | 18 +++++++-----------
 drivers/infiniband/sw/rxe/rxe_resp.c |  2 +-
 4 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c
index 6769a075501e..d1c3411c5478 100644
--- a/drivers/infiniband/sw/rxe/rxe_comp.c
+++ b/drivers/infiniband/sw/rxe/rxe_comp.c
@@ -418,7 +418,7 @@ static void do_complete(struct rxe_qp *qp, struct rxe_send_wqe *wqe)
 
 	if ((qp->sq_sig_type == IB_SIGNAL_ALL_WR) ||
 	    (wqe->wr.send_flags & IB_SEND_SIGNALED) ||
-	    (qp->req.state == QP_STATE_ERROR)) {
+	    wqe->status != IB_WC_SUCCESS) {
 		make_send_cqe(qp, wqe, &cqe);
 		advance_consumer(qp->sq.queue);
 		rxe_cq_post(qp->scq, &cqe, 0);
@@ -709,6 +709,7 @@ int rxe_completer(void *arg)
 			break;
 
 		case COMPST_ERROR:
+			WARN_ON_ONCE(wqe->status == IB_WC_SUCCESS);
 			do_complete(qp, wqe);
 			rxe_qp_error(qp);
 
diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
index da191d7acb6f..bdec460f1fce 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -225,6 +225,7 @@ extern struct ib_dma_mapping_ops rxe_dma_mapping_ops;
 
 void rxe_release(struct kref *kref);
 
+void rxe_drain_req_pkts(struct rxe_qp *qp, bool notify);
 int rxe_completer(void *arg);
 int rxe_requester(void *arg);
 int rxe_responder(void *arg);
diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index b95b6035e988..64999f529981 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -594,9 +594,14 @@ int rxe_requester(void *arg)
 	rxe_add_ref(qp);
 
 next_wqe:
-	if (unlikely(!qp->valid || qp->req.state == QP_STATE_ERROR))
+	if (unlikely(!qp->valid))
 		goto exit;
 
+	if (unlikely(qp->req.state == QP_STATE_ERROR)) {
+		rxe_drain_req_pkts(qp, true);
+		goto exit;
+	}
+
 	if (unlikely(qp->req.state == QP_STATE_RESET)) {
 		qp->req.wqe_index = consumer_index(qp->sq.queue);
 		qp->req.opcode = -1;
@@ -743,17 +748,8 @@ int rxe_requester(void *arg)
 	kfree_skb(skb);
 	wqe->status = IB_WC_LOC_PROT_ERR;
 	wqe->state = wqe_state_error;
-
-	/*
-	 * IBA Spec. Section 10.7.3.1 SIGNALED COMPLETIONS
-	 * ---------8<---------8<-------------
-	 * ...Note that if a completion error occurs, a Work Completion
-	 * will always be generated, even if the signaling
-	 * indicator requests an Unsignaled Completion.
-	 * ---------8<---------8<-------------
-	 */
-	wqe->wr.send_flags |= IB_SEND_SIGNALED;
 	__rxe_do_task(&qp->comp.task);
+
 exit:
 	rxe_drop_ref(qp);
 	return -EAGAIN;
diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index 25951e9413b8..33defaddc000 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -1207,7 +1207,7 @@ static enum resp_states do_class_d1e_error(struct rxe_qp *qp)
 	}
 }
 
-static void rxe_drain_req_pkts(struct rxe_qp *qp, bool notify)
+void rxe_drain_req_pkts(struct rxe_qp *qp, bool notify)
 {
 	struct sk_buff *skb;
 
-- 
2.11.0

Re: [PATCH 11/15] IB/rxe: Generate a completion for all failed work requests

[Boyer, Andrew]

On 1/2/17, 5:42 AM, "Bart Van Assche" <Bart.VanAssche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> wrote:

>Change do_complete() such that an error completion is not only
>generated if a QP is in the error state but also if a work request
>failed.
>
>Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
>Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
>---
> drivers/infiniband/sw/rxe/rxe_comp.c |  3 ++-
> drivers/infiniband/sw/rxe/rxe_loc.h  |  1 +
> drivers/infiniband/sw/rxe/rxe_req.c  | 18 +++++++-----------
> drivers/infiniband/sw/rxe/rxe_resp.c |  2 +-
> 4 files changed, 11 insertions(+), 13 deletions(-)
>
>diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c
>b/drivers/infiniband/sw/rxe/rxe_comp.c
>index 6769a075501e..d1c3411c5478 100644
>--- a/drivers/infiniband/sw/rxe/rxe_comp.c
>+++ b/drivers/infiniband/sw/rxe/rxe_comp.c
>@@ -418,7 +418,7 @@ static void do_complete(struct rxe_qp *qp, struct
>rxe_send_wqe *wqe)
> 
> 	if ((qp->sq_sig_type == IB_SIGNAL_ALL_WR) ||
> 	    (wqe->wr.send_flags & IB_SEND_SIGNALED) ||
>-	    (qp->req.state == QP_STATE_ERROR)) {
>+	    wqe->status != IB_WC_SUCCESS) {
> 		make_send_cqe(qp, wqe, &cqe);
> 		advance_consumer(qp->sq.queue);
> 		rxe_cq_post(qp->scq, &cqe, 0);
>@@ -709,6 +709,7 @@ int rxe_completer(void *arg)
> 			break;
> 
> 		case COMPST_ERROR:
>+			WARN_ON_ONCE(wqe->status == IB_WC_SUCCESS);
> 			do_complete(qp, wqe);
> 			rxe_qp_error(qp);
> 
>diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h
>b/drivers/infiniband/sw/rxe/rxe_loc.h
>index da191d7acb6f..bdec460f1fce 100644
>--- a/drivers/infiniband/sw/rxe/rxe_loc.h
>+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
>@@ -225,6 +225,7 @@ extern struct ib_dma_mapping_ops rxe_dma_mapping_ops;
> 
> void rxe_release(struct kref *kref);
> 
>+void rxe_drain_req_pkts(struct rxe_qp *qp, bool notify);
> int rxe_completer(void *arg);
> int rxe_requester(void *arg);
> int rxe_responder(void *arg);
>diff --git a/drivers/infiniband/sw/rxe/rxe_req.c
>b/drivers/infiniband/sw/rxe/rxe_req.c
>index b95b6035e988..64999f529981 100644
>--- a/drivers/infiniband/sw/rxe/rxe_req.c
>+++ b/drivers/infiniband/sw/rxe/rxe_req.c
>@@ -594,9 +594,14 @@ int rxe_requester(void *arg)
> 	rxe_add_ref(qp);
> 
> next_wqe:
>-	if (unlikely(!qp->valid || qp->req.state == QP_STATE_ERROR))
>+	if (unlikely(!qp->valid))
> 		goto exit;
> 
>+	if (unlikely(qp->req.state == QP_STATE_ERROR)) {
>+		rxe_drain_req_pkts(qp, true);
>+		goto exit;
>+	}
>+
> 	if (unlikely(qp->req.state == QP_STATE_RESET)) {
> 		qp->req.wqe_index = consumer_index(qp->sq.queue);
> 		qp->req.opcode = -1;
>@@ -743,17 +748,8 @@ int rxe_requester(void *arg)
> 	kfree_skb(skb);
> 	wqe->status = IB_WC_LOC_PROT_ERR;
> 	wqe->state = wqe_state_error;
>-
>-	/*
>-	 * IBA Spec. Section 10.7.3.1 SIGNALED COMPLETIONS
>-	 * ---------8<---------8<-------------
>-	 * ...Note that if a completion error occurs, a Work Completion
>-	 * will always be generated, even if the signaling
>-	 * indicator requests an Unsignaled Completion.
>-	 * ---------8<---------8<-------------
>-	 */

It wouldn¹t hurt to move this comment over to the top of do_complete() to
explain the wqe->status != IB_WC_SUCCESS test.

Reviewed-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>


>-	wqe->wr.send_flags |= IB_SEND_SIGNALED;
> 	__rxe_do_task(&qp->comp.task);
>+
> exit:
> 	rxe_drop_ref(qp);
> 	return -EAGAIN;
>diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c
>b/drivers/infiniband/sw/rxe/rxe_resp.c
>index 25951e9413b8..33defaddc000 100644
>--- a/drivers/infiniband/sw/rxe/rxe_resp.c
>+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
>@@ -1207,7 +1207,7 @@ static enum resp_states do_class_d1e_error(struct
>rxe_qp *qp)
> 	}
> }
> 
>-static void rxe_drain_req_pkts(struct rxe_qp *qp, bool notify)
>+void rxe_drain_req_pkts(struct rxe_qp *qp, bool notify)
> {
> 	struct sk_buff *skb;
> 
>-- 
>2.11.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

[PATCH 12/15] IB/rxe: Fix a MR reference leak in check_rkey()

[Bart Van Assche]

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

Avoid that calling check_rkey() for mem->state == RXE_MEM_STATE_FREE
triggers an MR reference leak.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Moni Shoua <monis@mellanox.com>
Cc: Andrew Boyer <andrew.boyer@dell.com>
---
 drivers/infiniband/sw/rxe/rxe_resp.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index 33defaddc000..60d78f45aa04 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -418,7 +418,7 @@ static enum resp_states check_length(struct rxe_qp *qp,
 static enum resp_states check_rkey(struct rxe_qp *qp,
 				   struct rxe_pkt_info *pkt)
 {
-	struct rxe_mem *mem;
+	struct rxe_mem *mem = NULL;
 	u64 va;
 	u32 rkey;
 	u32 resid;
@@ -459,38 +459,38 @@ static enum resp_states check_rkey(struct rxe_qp *qp,
 	mem = lookup_mem(qp->pd, access, rkey, lookup_remote);
 	if (!mem) {
 		state = RESPST_ERR_RKEY_VIOLATION;
-		goto err1;
+		goto err;
 	}
 
 	if (unlikely(mem->state == RXE_MEM_STATE_FREE)) {
 		state = RESPST_ERR_RKEY_VIOLATION;
-		goto err1;
+		goto err;
 	}
 
 	if (mem_check_range(mem, va, resid)) {
 		state = RESPST_ERR_RKEY_VIOLATION;
-		goto err2;
+		goto err;
 	}
 
 	if (pkt->mask & RXE_WRITE_MASK)	 {
 		if (resid > mtu) {
 			if (pktlen != mtu || bth_pad(pkt)) {
 				state = RESPST_ERR_LENGTH;
-				goto err2;
+				goto err;
 			}
 
 			resid = mtu;
 		} else {
 			if (pktlen != resid) {
 				state = RESPST_ERR_LENGTH;
-				goto err2;
+				goto err;
 			}
 			if ((bth_pad(pkt) != (0x3 & (-resid)))) {
 				/* This case may not be exactly that
 				 * but nothing else fits.
 				 */
 				state = RESPST_ERR_LENGTH;
-				goto err2;
+				goto err;
 			}
 		}
 	}
@@ -500,9 +500,9 @@ static enum resp_states check_rkey(struct rxe_qp *qp,
 	qp->resp.mr = mem;
 	return RESPST_EXECUTE;
 
-err2:
-	rxe_drop_ref(mem);
-err1:
+err:
+	if (mem)
+		rxe_drop_ref(mem);
 	return state;
 }
 
-- 
2.11.0
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš

Re: [PATCH 12/15] IB/rxe: Fix a MR reference leak in check_rkey()

[Boyer, Andrew]

On 1/2/17, 5:43 AM, "Bart Van Assche" <Bart.VanAssche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> wrote:

>Avoid that calling check_rkey() for mem->state == RXE_MEM_STATE_FREE
>triggers an MR reference leak.
>
>Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
>Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
>---
> drivers/infiniband/sw/rxe/rxe_resp.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
>diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c
>b/drivers/infiniband/sw/rxe/rxe_resp.c
>index 33defaddc000..60d78f45aa04 100644
>--- a/drivers/infiniband/sw/rxe/rxe_resp.c
>+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
>@@ -418,7 +418,7 @@ static enum resp_states check_length(struct rxe_qp
>*qp,
> static enum resp_states check_rkey(struct rxe_qp *qp,
> 				   struct rxe_pkt_info *pkt)
> {
>-	struct rxe_mem *mem;
>+	struct rxe_mem *mem = NULL;

I like having this extra load for clarity, but the general sentiment
around here seems to be that it should be avoided. There¹s no path I can
see that touches mem before lookup_mem() sets it.

Otherwise
Reviewed-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>


> 	u64 va;
> 	u32 rkey;
> 	u32 resid;
>@@ -459,38 +459,38 @@ static enum resp_states check_rkey(struct rxe_qp
>*qp,
> 	mem = lookup_mem(qp->pd, access, rkey, lookup_remote);
> 	if (!mem) {
> 		state = RESPST_ERR_RKEY_VIOLATION;
>-		goto err1;
>+		goto err;
> 	}
> 
> 	if (unlikely(mem->state == RXE_MEM_STATE_FREE)) {
> 		state = RESPST_ERR_RKEY_VIOLATION;
>-		goto err1;
>+		goto err;
> 	}
> 
> 	if (mem_check_range(mem, va, resid)) {
> 		state = RESPST_ERR_RKEY_VIOLATION;
>-		goto err2;
>+		goto err;
> 	}
> 
> 	if (pkt->mask & RXE_WRITE_MASK)	 {
> 		if (resid > mtu) {
> 			if (pktlen != mtu || bth_pad(pkt)) {
> 				state = RESPST_ERR_LENGTH;
>-				goto err2;
>+				goto err;
> 			}
> 
> 			resid = mtu;
> 		} else {
> 			if (pktlen != resid) {
> 				state = RESPST_ERR_LENGTH;
>-				goto err2;
>+				goto err;
> 			}
> 			if ((bth_pad(pkt) != (0x3 & (-resid)))) {
> 				/* This case may not be exactly that
> 				 * but nothing else fits.
> 				 */
> 				state = RESPST_ERR_LENGTH;
>-				goto err2;
>+				goto err;
> 			}
> 		}
> 	}
>@@ -500,9 +500,9 @@ static enum resp_states check_rkey(struct rxe_qp *qp,
> 	qp->resp.mr = mem;
> 	return RESPST_EXECUTE;
> 
>-err2:
>-	rxe_drop_ref(mem);
>-err1:
>+err:
>+	if (mem)
>+		rxe_drop_ref(mem);
> 	return state;
> }
> 
>-- 
>2.11.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

[PATCH 13/15] IB/rxe: Fix reference leaks in memory key invalidation code

[Bart Van Assche]

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Moni Shoua <monis@mellanox.com>
Cc: Andrew Boyer <andrew.boyer@dell.com>
---
 drivers/infiniband/sw/
rxe/rxe_req.c  | 1 +
 drivers/infiniband/sw/rxe/rxe_resp.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index 64999f529981..1bba4a734e73 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -635,6 +635,7 @@ int rxe_requester(void *arg)
 				goto exit;
 			}
 			rmr->state = RXE_MEM_STATE_FREE;
+			rxe_drop_ref(rmr);
 			wqe->state = wqe_state_done;
 			wqe->status = IB_WC_SUCCESS;
 		} else if (wqe->wr.opcode == IB_WR_REG_MR) {
diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index 60d78f45aa04..05f374986cea 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -907,6 +907,7 @@ static enum resp_states do_complete(struct rxe_qp *qp,
 					return RESPST_ERROR;
 				}
 				rmr->state = RXE_MEM_STATE_FREE;
+				rxe_drop_ref(rmr);
 			}
 
 			wc->qp			= &qp->ibqp;
-- 
2.11.0

Re: [PATCH 13/15] IB/rxe: Fix reference leaks in memory key invalidation code

[Boyer, Andrew]

On 1/2/17, 5:43 AM, "Bart Van Assche" <Bart.VanAssche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> wrote:

>Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
>Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
>---
> drivers/infiniband/sw/
>rxe/rxe_req.c  | 1 +
> drivers/infiniband/sw/rxe/rxe_resp.c | 1 +
> 2 files changed, 2 insertions(+)

Reviewed-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@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 14/15] IB/rxe: Remove a pointless indirection layer

[Bart Van Assche]

Neither rxe->ifc_ops nor any of the function pointers in struct
struct rxe_ifc_ops ever change. Hence remove the rxe->ifc_ops
indirection mechanism.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Moni Shoua <monis@mellanox.com>
Cc: Andrew Boyer <andrew.boyer@dell.com>
---
 drivers/infiniband/sw/rxe/rxe.c       |  2 +-
 drivers/infiniband/sw/rxe/rxe_loc.h   | 20 ++++++++++++++--
 drivers/infiniband/sw/rxe/rxe_mcast.c |  4 ++--
 drivers/infiniband/sw/rxe/rxe_net.c   | 43 +++++++++++------------------------
 drivers/infiniband/sw/rxe/rxe_req.c   |  4 ++--
 drivers/infiniband/sw/rxe/rxe_resp.c  |  4 ++--
 drivers/infiniband/sw/rxe/rxe_verbs.c | 10 ++++----
 drivers/infiniband/sw/rxe/rxe_verbs.h | 22 ------------------
 8 files changed, 42 insertions(+), 67 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c
index ab6c3c25d7ff..b12dd9b5a89d 100644
--- a/drivers/infiniband/sw/rxe/rxe.c
+++ b/drivers/infiniband/sw/rxe/rxe.c
@@ -178,7 +178,7 @@ static int rxe_init_ports(struct rxe_dev *rxe)
 		return -ENOMEM;
 
 	port->pkey_tbl[0] = 0xffff;
-	port->port_guid = rxe->ifc_ops->port_guid(rxe);
+	port->port_guid = rxe_port_guid(rxe);
 
 	spin_lock_init(&port->port_lock);
 
diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
index bdec460f1fce..272337e5e948 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -141,6 +141,22 @@ void rxe_mem_cleanup(struct rxe_pool_entry *arg);
 
 int advance_dma_data(struct rxe_dma_info *dma, unsigned int length);
 
+/* rxe_net.c */
+int rxe_loopback(struct sk_buff *skb);
+int rxe_send(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
+	     struct sk_buff *skb);
+__be64 rxe_port_guid(struct rxe_dev *rxe);
+struct sk_buff *rxe_init_packet(struct rxe_dev *rxe, struct rxe_av *av,
+				int paylen, struct rxe_pkt_info *pkt);
+int rxe_prepare(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
+		struct sk_buff *skb, u32 *crc);
+enum rdma_link_layer rxe_link_layer(struct rxe_dev *rxe, unsigned int port_num);
+const char *rxe_parent_name(struct rxe_dev *rxe, unsigned int port_num);
+struct device *rxe_dma_device(struct rxe_dev *rxe);
+__be64 rxe_node_guid(struct rxe_dev *rxe);
+int rxe_mcast_add(struct rxe_dev *rxe, union ib_gid *mgid);
+int rxe_mcast_delete(struct rxe_dev *rxe, union ib_gid *mgid);
+
 /* rxe_qp.c */
 int rxe_qp_chk_init(struct rxe_dev *rxe, struct ib_qp_init_attr *init);
 
@@ -257,9 +273,9 @@ static inline int rxe_xmit_packet(struct rxe_dev *rxe, struct rxe_qp *qp,
 
 	if (pkt->mask & RXE_LOOPBACK_MASK) {
 		memcpy(SKB_TO_PKT(skb), pkt, sizeof(*pkt));
-		err = rxe->ifc_ops->loopback(skb);
+		err = rxe_loopback(skb);
 	} else {
-		err = rxe->ifc_ops->send(rxe, pkt, skb);
+		err = rxe_send(rxe, pkt, skb);
 	}
 
 	if (err) {
diff --git a/drivers/infiniband/sw/rxe/rxe_mcast.c b/drivers/infiniband/sw/rxe/rxe_mcast.c
index e0fb6752f90e..522a7942c56c 100644
--- a/drivers/infiniband/sw/rxe/rxe_mcast.c
+++ b/drivers/infiniband/sw/rxe/rxe_mcast.c
@@ -61,7 +61,7 @@ int rxe_mcast_get_grp(struct rxe_dev *rxe, union ib_gid *mgid,
 
 	rxe_add_key(grp, mgid);
 
-	err = rxe->ifc_ops->mcast_add(rxe, mgid);
+	err = rxe_mcast_add(rxe, mgid);
 	if (err)
 		goto err2;
 
@@ -186,5 +186,5 @@ void rxe_mc_cleanup(struct rxe_pool_entry *arg)
 	struct rxe_dev *rxe = grp->rxe;
 
 	rxe_drop_key(grp);
-	rxe->ifc_ops->mcast_delete(rxe, &grp->mgid);
+	rxe_mcast_delete(rxe, &grp->mgid);
 }
diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index 151f639abebf..50144c307eb4 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -102,17 +102,17 @@ static __be64 rxe_mac_to_eui64(struct net_device *ndev)
 	return eui64;
 }
 
-static __be64 node_guid(struct rxe_dev *rxe)
+__be64 rxe_node_guid(struct rxe_dev *rxe)
 {
 	return rxe_mac_to_eui64(rxe->ndev);
 }
 
-static __be64 port_guid(struct rxe_dev *rxe)
+__be64 rxe_port_guid(struct rxe_dev *rxe)
 {
 	return rxe_mac_to_eui64(rxe->ndev);
 }
 
-static struct device *dma_device(struct rxe_dev *rxe)
+struct device *rxe_dma_device(struct rxe_dev *rxe)
 {
 	struct net_device *ndev;
 
@@ -124,7 +124,7 @@ static struct device *dma_device(struct rxe_dev *rxe)
 	return ndev->dev.parent;
 }
 
-static int mcast_add(struct rxe_dev *rxe, union ib_gid *mgid)
+int rxe_mcast_add(struct rxe_dev *rxe, union ib_gid *mgid)
 {
 	int err;
 	unsigned char ll_addr[ETH_ALEN];
@@ -135,7 +135,7 @@ static int mcast_add(struct rxe_dev *rxe, union ib_gid *mgid)
 	return err;
 }
 
-static int mcast_delete(struct rxe_dev *rxe, union ib_gid *mgid)
+int rxe_mcast_delete(struct rxe_dev *rxe, union ib_gid *mgid)
 {
 	int err;
 	unsigned char ll_addr[ETH_ALEN];
@@ -397,8 +397,8 @@ static int prepare6(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
 	return 0;
 }
 
-static int prepare(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
-		   struct sk_buff *skb, u32 *crc)
+int rxe_prepare(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
+		struct sk_buff *skb, u32 *crc)
 {
 	int err = 0;
 	struct rxe_av *av = rxe_get_av(pkt);
@@ -424,8 +424,7 @@ static void rxe_skb_tx_dtor(struct sk_buff *skb)
 		rxe_run_task(&qp->req.task, 1);
 }
 
-static int send(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
-		struct sk_buff *skb)
+int rxe_send(struct rxe_dev *rxe, struct rxe_pkt_info *pkt, struct sk_buff *skb)
 {
 	struct sk_buff *nskb;
 	struct rxe_av *av;
@@ -461,7 +460,7 @@ static int send(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
 	return 0;
 }
 
-static int loopback(struct sk_buff *skb)
+int rxe_loopback(struct sk_buff *skb)
 {
 	return rxe_rcv(skb);
 }
@@ -471,8 +470,8 @@ static inline int addr_same(struct rxe_dev *rxe, struct rxe_av *av)
 	return rxe->port.port_guid == av->grh.dgid.global.interface_id;
 }
 
-static struct sk_buff *init_packet(struct rxe_dev *rxe, struct rxe_av *av,
-				   int paylen, struct rxe_pkt_info *pkt)
+struct sk_buff *rxe_init_packet(struct rxe_dev *rxe, struct rxe_av *av,
+				int paylen, struct rxe_pkt_info *pkt)
 {
 	unsigned int hdr_len;
 	struct sk_buff *skb;
@@ -511,31 +510,16 @@ static struct sk_buff *init_packet(struct rxe_dev *rxe, struct rxe_av *av,
  * this is required by rxe_cfg to match rxe devices in
  * /sys/class/infiniband up with their underlying ethernet devices
  */
-static char *parent_name(struct rxe_dev *rxe, unsigned int port_num)
+const char *rxe_parent_name(struct rxe_dev *rxe, unsigned int port_num)
 {
 	return rxe->ndev->name;
 }
 
-static enum rdma_link_layer link_layer(struct rxe_dev *rxe,
-				       unsigned int port_num)
+enum rdma_link_layer rxe_link_layer(struct rxe_dev *rxe, unsigned int port_num)
 {
 	return IB_LINK_LAYER_ETHERNET;
 }
 
-static struct rxe_ifc_ops ifc_ops = {
-	.node_guid	= node_guid,
-	.port_guid	= port_guid,
-	.dma_device	= dma_device,
-	.mcast_add	= mcast_add,
-	.mcast_delete	= mcast_delete,
-	.prepare	= prepare,
-	.send		= send,
-	.loopback	= loopback,
-	.init_packet	= init_packet,
-	.parent_name	= parent_name,
-	.link_layer	= link_layer,
-};
-
 struct rxe_dev *rxe_net_add(struct net_device *ndev)
 {
 	int err;
@@ -545,7 +529,6 @@ struct rxe_dev *rxe_net_add(struct net_device *ndev)
 	if (!rxe)
 		return NULL;
 
-	rxe->ifc_ops = &ifc_ops;
 	rxe->ndev = ndev;
 
 	err = rxe_add(rxe, ndev->mtu);
diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index 1bba4a734e73..522a4e4a21ce 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -404,7 +404,7 @@ static struct sk_buff *init_req_packet(struct rxe_qp *qp,
 
 	/* init skb */
 	av = rxe_get_av(pkt);
-	skb = rxe->ifc_ops->init_packet(rxe, av, paylen, pkt);
+	skb = rxe_init_packet(rxe, av, paylen, pkt);
 	if (unlikely(!skb))
 		return NULL;
 
@@ -475,7 +475,7 @@ static int fill_packet(struct rxe_qp *qp, struct rxe_send_wqe *wqe,
 	u32 *p;
 	int err;
 
-	err = rxe->ifc_ops->prepare(rxe, pkt, skb, &crc);
+	err = rxe_prepare(rxe, pkt, skb, &crc);
 	if (err)
 		return err;
 
diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index 05f374986cea..7bf20ced2078 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -608,7 +608,7 @@ static struct sk_buff *prepare_ack_packet(struct rxe_qp *qp,
 	pad = (-payload) & 0x3;
 	paylen = rxe_opcode[opcode].length + payload + pad + RXE_ICRC_SIZE;
 
-	skb = rxe->ifc_ops->init_packet(rxe, &qp->pri_av, paylen, ack);
+	skb = rxe_init_packet(rxe, &qp->pri_av, paylen, ack);
 	if (!skb)
 		return NULL;
 
@@ -637,7 +637,7 @@ static struct sk_buff *prepare_ack_packet(struct rxe_qp *qp,
 	if (ack->mask & RXE_ATMACK_MASK)
 		atmack_set_orig(ack, qp->resp.atomic_orig);
 
-	err = rxe->ifc_ops->prepare(rxe, ack, skb, &crc);
+	err = rxe_prepare(rxe, ack, skb, &crc);
 	if (err) {
 		kfree_skb(skb);
 		return NULL;
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
index beb7021ff18a..e4de37fb9aab 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
@@ -234,7 +234,7 @@ static enum rdma_link_layer rxe_get_link_layer(struct ib_device *dev,
 {
 	struct rxe_dev *rxe = to_rdev(dev);
 
-	return rxe->ifc_ops->link_layer(rxe, port_num);
+	return rxe_link_layer(rxe, port_num);
 }
 
 static struct ib_ucontext *rxe_alloc_ucontext(struct ib_device *dev,
@@ -1209,10 +1209,8 @@ static ssize_t rxe_show_parent(struct device *device,
 {
 	struct rxe_dev *rxe = container_of(device, struct rxe_dev,
 					   ib_dev.dev);
-	char *name;
 
-	name = rxe->ifc_ops->parent_name(rxe, 1);
-	return snprintf(buf, 16, "%s\n", name);
+	return snprintf(buf, 16, "%s\n", rxe_parent_name(rxe, 1));
 }
 
 static DEVICE_ATTR(parent, S_IRUGO, rxe_show_parent, NULL);
@@ -1234,9 +1232,9 @@ int rxe_register_device(struct rxe_dev *rxe)
 	dev->node_type = RDMA_NODE_IB_CA;
 	dev->phys_port_cnt = 1;
 	dev->num_comp_vectors = RXE_NUM_COMP_VECTORS;
-	dev->dma_device = rxe->ifc_ops->dma_device(rxe);
+	dev->dma_device = rxe_dma_device(rxe);
 	dev->local_dma_lkey = 0;
-	dev->node_guid = rxe->ifc_ops->node_guid(rxe);
+	dev->node_guid = rxe_node_guid(rxe);
 	dev->dma_ops = &rxe_dma_mapping_ops;
 
 	dev->uverbs_abi_ver = RXE_UVERBS_ABI_VERSION;
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
index 536974b69ed9..e100c500ae85 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.h
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
@@ -372,26 +372,6 @@ struct rxe_port {
 	u32			qp_gsi_index;
 };
 
-/* callbacks from rdma_rxe to network interface layer */
-struct rxe_ifc_ops {
-	void (*release)(struct rxe_dev *rxe);
-	__be64 (*node_guid)(struct rxe_dev *rxe);
-	__be64 (*port_guid)(struct rxe_dev *rxe);
-	struct device *(*dma_device)(struct rxe_dev *rxe);
-	int (*mcast_add)(struct rxe_dev *rxe, union ib_gid *mgid);
-	int (*mcast_delete)(struct rxe_dev *rxe, union ib_gid *mgid);
-	int (*prepare)(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
-		       struct sk_buff *skb, u32 *crc);
-	int (*send)(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
-		    struct sk_buff *skb);
-	int (*loopback)(struct sk_buff *skb);
-	struct sk_buff *(*init_packet)(struct rxe_dev *rxe, struct rxe_av *av,
-				       int paylen, struct rxe_pkt_info *pkt);
-	char *(*parent_name)(struct rxe_dev *rxe, unsigned int port_num);
-	enum rdma_link_layer (*link_layer)(struct rxe_dev *rxe,
-					   unsigned int port_num);
-};
-
 struct rxe_dev {
 	struct ib_device	ib_dev;
 	struct ib_device_attr	attr;
@@ -400,8 +380,6 @@ struct rxe_dev {
 	struct kref		ref_cnt;
 	struct mutex	usdev_lock;
 
-	struct rxe_ifc_ops	*ifc_ops;
-
 	struct net_device	*ndev;
 
 	int			xmit_errors;
-- 
2.11.0

Re: [PATCH 14/15] IB/rxe: Remove a pointless indirection layer

[Leon Romanovsky]

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

On Mon, Jan 02, 2017 at 10:43:57AM +0000, Bart Van Assche wrote:
> Neither rxe->ifc_ops nor any of the function pointers in struct
> struct rxe_ifc_ops ever change. Hence remove the rxe->ifc_ops
> indirection mechanism.
>
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
> ---
>  drivers/infiniband/sw/rxe/rxe.c       |  2 +-
>  drivers/infiniband/sw/rxe/rxe_loc.h   | 20 ++++++++++++++--
>  drivers/infiniband/sw/rxe/rxe_mcast.c |  4 ++--
>  drivers/infiniband/sw/rxe/rxe_net.c   | 43 +++++++++++------------------------
>  drivers/infiniband/sw/rxe/rxe_req.c   |  4 ++--
>  drivers/infiniband/sw/rxe/rxe_resp.c  |  4 ++--
>  drivers/infiniband/sw/rxe/rxe_verbs.c | 10 ++++----
>  drivers/infiniband/sw/rxe/rxe_verbs.h | 22 ------------------
>  8 files changed, 42 insertions(+), 67 deletions(-)
>

Thanks,
Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

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

Re: [PATCH 14/15] IB/rxe: Remove a pointless indirection layer

[Boyer, Andrew]

On 1/2/17, 5:43 AM, "Bart Van Assche" <Bart.VanAssche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> wrote:

>Neither rxe->ifc_ops nor any of the function pointers in struct
>struct rxe_ifc_ops ever change. Hence remove the rxe->ifc_ops
>indirection mechanism.
>
>Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
>Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
>---
> drivers/infiniband/sw/rxe/rxe.c       |  2 +-
> drivers/infiniband/sw/rxe/rxe_loc.h   | 20 ++++++++++++++--
> drivers/infiniband/sw/rxe/rxe_mcast.c |  4 ++--
> drivers/infiniband/sw/rxe/rxe_net.c   | 43
>+++++++++++------------------------
> drivers/infiniband/sw/rxe/rxe_req.c   |  4 ++--
> drivers/infiniband/sw/rxe/rxe_resp.c  |  4 ++--
> drivers/infiniband/sw/rxe/rxe_verbs.c | 10 ++++----
> drivers/infiniband/sw/rxe/rxe_verbs.h | 22 ------------------
> 8 files changed, 42 insertions(+), 67 deletions(-)

Reviewed-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@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/15] IB/rxe: Fix an skb leak

[Bart Van Assche]

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

Additionally, make it easier to detect skb leaks by issuing a warning
if a leak occurs.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Moni Shoua <monis@mellanox.com>
Cc: Andrew Boyer <andrew.boyer@dell.com>
---
 drivers/infiniband/sw/rxe/rxe_comp.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c
index d1c3411c5478..bbc7cd39f978 100644
--- a/drivers/infiniband/sw/rxe/rxe_comp.c
+++ b/drivers/infiniband/sw/rxe/rxe_comp.c
@@ -622,6 +622,7 @@ int rxe_completer(void *arg)
 			if (pkt) {
 				rxe_drop_ref(pkt->qp);
 				kfree_skb(skb);
+				skb = NULL;
 			}
 			goto done;
 
@@ -645,6 +646,7 @@ int rxe_completer(void *arg)
 			    qp->qp_timeout_jiffies)
 				mod_timer(&qp->retrans_timer,
 					  jiffies + qp->qp_timeout_jiffies);
+			WARN_ON_ONCE(skb);
 			goto exit;
 
 		case COMPST_ERROR_RETRY:
@@ -657,8 +659,10 @@ int rxe_completer(void *arg)
 			 */
 
 			/* there is nothing to retry in this case */
-			if (!wqe || (wqe->state == wqe_state_posted))
+			if (!wqe || (wqe->state == wqe_state_posted)) {
+				WARN_ON_ONCE(skb);
 				goto exit;
+			}
 
 			if (qp->comp.retry_cnt > 0) {
 				if (qp->comp.retry_cnt != 7)
@@ -680,8 +684,10 @@ int rxe_completer(void *arg)
 				if (pkt) {
 					rxe_drop_ref(pkt->qp);
 					kfree_skb(skb);
+					skb = NULL;
 				}
 
+				WARN_ON_ONCE(skb);
 				goto exit;
 
 			} else {
@@ -701,6 +707,9 @@ int rxe_completer(void *arg)
 				mod_timer(&qp->rnr_nak_timer,
 					  jiffies + rnrnak_jiffies(aeth_syn(pkt)
 						& ~AETH_TYPE_MASK));
+				rxe_drop_ref(pkt->qp);
+				kfree_skb(skb);
+				skb = NULL;
 				goto exit;
 			} else {
 				wqe->status = IB_WC_RNR_RETRY_EXC_ERR;
@@ -716,8 +725,10 @@ int rxe_completer(void *arg)
 			if (pkt) {
 				rxe_drop_ref(pkt->qp);
 				kfree_skb(skb);
+				skb = NULL;
 			}
 
+			WARN_ON_ONCE(skb);
 			goto exit;
 		}
 	}
@@ -726,6 +737,7 @@ int rxe_completer(void *arg)
 	/* we come here if we are done with processing and want the task to
 	 * exit from the loop calling us
 	 */
+	WARN_ON_ONCE(skb);
 	rxe_drop_ref(qp);
 	return -EAGAIN;
 
@@ -733,6 +745,7 @@ int rxe_completer(void *arg)
 	/* we come here if we have processed a packet we want the task to call
 	 * us again to see if there is anything else to do
 	 */
+	WARN_ON_ONCE(skb);
 	rxe_drop_ref(qp);
 	return 0;
 }
-- 
2.11.0
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš

Re: [PATCH 15/15] IB/rxe: Fix an skb leak

[Leon Romanovsky]

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

On Mon, Jan 02, 2017 at 10:44:15AM +0000, Bart Van Assche wrote:
> Additionally, make it easier to detect skb leaks by issuing a warning
> if a leak occurs.
>
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
> ---
>  drivers/infiniband/sw/rxe/rxe_comp.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>

Thanks,
Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

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

Re: [PATCH 15/15] IB/rxe: Fix an skb leak

[Boyer, Andrew]

On 1/2/17, 5:44 AM, "Bart Van Assche" <Bart.VanAssche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> wrote:

>Additionally, make it easier to detect skb leaks by issuing a warning
>if a leak occurs.
>
>Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
>Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>Cc: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
>---
> drivers/infiniband/sw/rxe/rxe_comp.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)

I liked this better when it was doing the rxe_drop_ref() and kfree_skb()
after exit: and done:, if pkt was set.
Maybe that was just in our private patchset, I don¹t remember, but it
would be fewer lines and fewer chances to miss something.

>
>diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c
>b/drivers/infiniband/sw/rxe/rxe_comp.c
>index d1c3411c5478..bbc7cd39f978 100644
>--- a/drivers/infiniband/sw/rxe/rxe_comp.c
>+++ b/drivers/infiniband/sw/rxe/rxe_comp.c
>@@ -622,6 +622,7 @@ int rxe_completer(void *arg)
> 			if (pkt) {
> 				rxe_drop_ref(pkt->qp);
> 				kfree_skb(skb);
>+				skb = NULL;
> 			}
> 			goto done;
> 
>@@ -645,6 +646,7 @@ int rxe_completer(void *arg)
> 			    qp->qp_timeout_jiffies)
> 				mod_timer(&qp->retrans_timer,
> 					  jiffies + qp->qp_timeout_jiffies);
>+			WARN_ON_ONCE(skb);
> 			goto exit;
> 
> 		case COMPST_ERROR_RETRY:
>@@ -657,8 +659,10 @@ int rxe_completer(void *arg)
> 			 */
> 
> 			/* there is nothing to retry in this case */
>-			if (!wqe || (wqe->state == wqe_state_posted))
>+			if (!wqe || (wqe->state == wqe_state_posted)) {
>+				WARN_ON_ONCE(skb);
> 				goto exit;
>+			}
> 
> 			if (qp->comp.retry_cnt > 0) {
> 				if (qp->comp.retry_cnt != 7)
>@@ -680,8 +684,10 @@ int rxe_completer(void *arg)
> 				if (pkt) {
> 					rxe_drop_ref(pkt->qp);
> 					kfree_skb(skb);
>+					skb = NULL;
> 				}
> 
>+				WARN_ON_ONCE(skb);
> 				goto exit;
> 
> 			} else {
>@@ -701,6 +707,9 @@ int rxe_completer(void *arg)
> 				mod_timer(&qp->rnr_nak_timer,
> 					  jiffies + rnrnak_jiffies(aeth_syn(pkt)
> 						& ~AETH_TYPE_MASK));
>+				rxe_drop_ref(pkt->qp);
>+				kfree_skb(skb);
>+				skb = NULL;
> 				goto exit;
> 			} else {
> 				wqe->status = IB_WC_RNR_RETRY_EXC_ERR;
>@@ -716,8 +725,10 @@ int rxe_completer(void *arg)
> 			if (pkt) {
> 				rxe_drop_ref(pkt->qp);
> 				kfree_skb(skb);
>+				skb = NULL;
> 			}
> 
>+			WARN_ON_ONCE(skb);
> 			goto exit;
> 		}
> 	}
>@@ -726,6 +737,7 @@ int rxe_completer(void *arg)
> 	/* we come here if we are done with processing and want the task to
> 	 * exit from the loop calling us
> 	 */
>+	WARN_ON_ONCE(skb);
> 	rxe_drop_ref(qp);
> 	return -EAGAIN;
> 
>@@ -733,6 +745,7 @@ int rxe_completer(void *arg)
> 	/* we come here if we have processed a packet we want the task to call
> 	 * us again to see if there is anything else to do
> 	 */
>+	WARN_ON_ONCE(skb);
> 	rxe_drop_ref(qp);
> 	return 0;
> }
>-- 
>2.11.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 00/15] IB/rxe patches for kernel v4.11

[Doug Ledford]

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

On Mon, 2017-01-02 at 10:35 +0000, Bart Van Assche wrote:
> Hello Doug,
> 
> The patches in this series are what I came up with by reviewing and
> testing
> the rdma_rxe driver. Please consider these patches for kernel v4.11.
> 

Hi Bart,

This patch set, as a whole, did not make it to patchworks properly.
 The patches themselves are in DOS file format and require a run of
dos2unix to fix them up, and patchworks didn't even grab 4 of the
patches out of the series.  Can you please gather your Reviewed-by:
tags, then resubmit the series with Unix formatting?  That should help
ensure they all get captured by patchworks.

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]