2022-01-13 17:20:21

by Chuck Lever

[permalink] [raw]
Subject: [PATCH 0/3] Minor clean-ups for v5.17

Sorry, these seem to have gotten lost in the shuffle. They've been
in my client-miscellany branch on git.kernel.org since late
November. I must have forgotten to repost them for review.

Trond rejected "SUNRPC: Don't dereference xprt->snd_task if it's
a cookie" for v5.16, so I've recrafted it to hopefully address
his concerns.

---

Chuck Lever (3):
xprtrdma: Remove final dprintk call sites from xprtrdma
xprtrdma: Remove definitions of RPCDBG_FACILITY
SUNRPC: Don't dereference xprt->snd_task if it's a cookie


include/trace/events/sunrpc.h | 18 +++++++++++++-----
net/sunrpc/xprtrdma/backchannel.c | 4 ----
net/sunrpc/xprtrdma/frwr_ops.c | 4 ----
net/sunrpc/xprtrdma/rpc_rdma.c | 4 ----
net/sunrpc/xprtrdma/transport.c | 4 ----
net/sunrpc/xprtrdma/verbs.c | 23 -----------------------
6 files changed, 13 insertions(+), 44 deletions(-)

--
Chuck Lever


2022-01-13 17:20:27

by Chuck Lever

[permalink] [raw]
Subject: [PATCH 1/3] xprtrdma: Remove final dprintk call sites from xprtrdma

Deprecated. This information is available via tracepoints.

Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/xprtrdma/verbs.c | 12 ------------
1 file changed, 12 deletions(-)

diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
index 3d3673ba9e1e..1d6e85fe3133 100644
--- a/net/sunrpc/xprtrdma/verbs.c
+++ b/net/sunrpc/xprtrdma/verbs.c
@@ -274,8 +274,6 @@ rpcrdma_cm_event_handler(struct rdma_cm_id *id, struct rdma_cm_event *event)
ep->re_connect_status = -ENETUNREACH;
goto wake_connect_worker;
case RDMA_CM_EVENT_REJECTED:
- dprintk("rpcrdma: connection to %pISpc rejected: %s\n",
- sap, rdma_reject_msg(id, event->status));
ep->re_connect_status = -ECONNREFUSED;
if (event->status == IB_CM_REJ_STALE_CONN)
ep->re_connect_status = -ENOTCONN;
@@ -291,8 +289,6 @@ rpcrdma_cm_event_handler(struct rdma_cm_id *id, struct rdma_cm_event *event)
break;
}

- dprintk("RPC: %s: %pISpc on %s/frwr: %s\n", __func__, sap,
- ep->re_id->device->name, rdma_event_msg(event->event));
return 0;
}

@@ -419,14 +415,6 @@ static int rpcrdma_ep_create(struct rpcrdma_xprt *r_xprt)
ep->re_attr.qp_type = IB_QPT_RC;
ep->re_attr.port_num = ~0;

- dprintk("RPC: %s: requested max: dtos: send %d recv %d; "
- "iovs: send %d recv %d\n",
- __func__,
- ep->re_attr.cap.max_send_wr,
- ep->re_attr.cap.max_recv_wr,
- ep->re_attr.cap.max_send_sge,
- ep->re_attr.cap.max_recv_sge);
-
ep->re_send_batch = ep->re_max_requests >> 3;
ep->re_send_count = ep->re_send_batch;
init_waitqueue_head(&ep->re_connect_wait);


2022-01-13 17:20:33

by Chuck Lever

[permalink] [raw]
Subject: [PATCH 2/3] xprtrdma: Remove definitions of RPCDBG_FACILITY

Deprecated. dprintk is no longer used in xprtrdma.

Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/xprtrdma/backchannel.c | 4 ----
net/sunrpc/xprtrdma/frwr_ops.c | 4 ----
net/sunrpc/xprtrdma/rpc_rdma.c | 4 ----
net/sunrpc/xprtrdma/transport.c | 4 ----
net/sunrpc/xprtrdma/verbs.c | 11 -----------
5 files changed, 27 deletions(-)

diff --git a/net/sunrpc/xprtrdma/backchannel.c b/net/sunrpc/xprtrdma/backchannel.c
index 17f174d6ea3b..faba7136dd9a 100644
--- a/net/sunrpc/xprtrdma/backchannel.c
+++ b/net/sunrpc/xprtrdma/backchannel.c
@@ -13,10 +13,6 @@
#include "xprt_rdma.h"
#include <trace/events/rpcrdma.h>

-#if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
-# define RPCDBG_FACILITY RPCDBG_TRANS
-#endif
-
#undef RPCRDMA_BACKCHANNEL_DEBUG

/**
diff --git a/net/sunrpc/xprtrdma/frwr_ops.c b/net/sunrpc/xprtrdma/frwr_ops.c
index ff699307e820..515dd7a66a04 100644
--- a/net/sunrpc/xprtrdma/frwr_ops.c
+++ b/net/sunrpc/xprtrdma/frwr_ops.c
@@ -45,10 +45,6 @@
#include "xprt_rdma.h"
#include <trace/events/rpcrdma.h>

-#if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
-# define RPCDBG_FACILITY RPCDBG_TRANS
-#endif
-
static void frwr_cid_init(struct rpcrdma_ep *ep,
struct rpcrdma_mr *mr)
{
diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c b/net/sunrpc/xprtrdma/rpc_rdma.c
index 8035a983c8ce..281ddb87ac8d 100644
--- a/net/sunrpc/xprtrdma/rpc_rdma.c
+++ b/net/sunrpc/xprtrdma/rpc_rdma.c
@@ -54,10 +54,6 @@
#include "xprt_rdma.h"
#include <trace/events/rpcrdma.h>

-#if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
-# define RPCDBG_FACILITY RPCDBG_TRANS
-#endif
-
/* Returns size of largest RPC-over-RDMA header in a Call message
*
* The largest Call header contains a full-size Read list and a
diff --git a/net/sunrpc/xprtrdma/transport.c b/net/sunrpc/xprtrdma/transport.c
index 16e5696314a4..42e375dbdadb 100644
--- a/net/sunrpc/xprtrdma/transport.c
+++ b/net/sunrpc/xprtrdma/transport.c
@@ -60,10 +60,6 @@
#include "xprt_rdma.h"
#include <trace/events/rpcrdma.h>

-#if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
-# define RPCDBG_FACILITY RPCDBG_TRANS
-#endif
-
/*
* tunables
*/
diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
index 1d6e85fe3133..f172d1298013 100644
--- a/net/sunrpc/xprtrdma/verbs.c
+++ b/net/sunrpc/xprtrdma/verbs.c
@@ -63,17 +63,6 @@
#include "xprt_rdma.h"
#include <trace/events/rpcrdma.h>

-/*
- * Globals/Macros
- */
-
-#if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
-# define RPCDBG_FACILITY RPCDBG_TRANS
-#endif
-
-/*
- * internal functions
- */
static int rpcrdma_sendctxs_create(struct rpcrdma_xprt *r_xprt);
static void rpcrdma_sendctxs_destroy(struct rpcrdma_xprt *r_xprt);
static void rpcrdma_sendctx_put_locked(struct rpcrdma_xprt *r_xprt,


2022-01-13 17:20:41

by Chuck Lever

[permalink] [raw]
Subject: [PATCH 3/3] SUNRPC: Don't dereference xprt->snd_task if it's a cookie

Fixes: e26d9972720e ("SUNRPC: Clean up scheduling of autoclose")
Signed-off-by: Chuck Lever <[email protected]>
---
include/trace/events/sunrpc.h | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
index 3a99358c262b..28433c9f3306 100644
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -953,7 +953,8 @@ TRACE_EVENT(rpc_socket_nospace,
{ BIT(XPRT_REMOVE), "REMOVE" }, \
{ BIT(XPRT_CONGESTED), "CONGESTED" }, \
{ BIT(XPRT_CWND_WAIT), "CWND_WAIT" }, \
- { BIT(XPRT_WRITE_SPACE), "WRITE_SPACE" })
+ { BIT(XPRT_WRITE_SPACE), "WRITE_SPACE" }, \
+ { BIT(XPRT_SND_IS_COOKIE), "SND_IS_COOKIE" })

DECLARE_EVENT_CLASS(rpc_xprt_lifetime_class,
TP_PROTO(
@@ -1150,8 +1151,11 @@ DECLARE_EVENT_CLASS(xprt_writelock_event,
__entry->task_id = -1;
__entry->client_id = -1;
}
- __entry->snd_task_id = xprt->snd_task ?
- xprt->snd_task->tk_pid : -1;
+ if (xprt->snd_task &&
+ !test_bit(XPRT_SND_IS_COOKIE, &xprt->state))
+ __entry->snd_task_id = xprt->snd_task->tk_pid;
+ else
+ __entry->snd_task_id = -1;
),

TP_printk(SUNRPC_TRACE_TASK_SPECIFIER
@@ -1196,8 +1200,12 @@ DECLARE_EVENT_CLASS(xprt_cong_event,
__entry->task_id = -1;
__entry->client_id = -1;
}
- __entry->snd_task_id = xprt->snd_task ?
- xprt->snd_task->tk_pid : -1;
+ if (xprt->snd_task &&
+ !test_bit(XPRT_SND_IS_COOKIE, &xprt->state))
+ __entry->snd_task_id = xprt->snd_task->tk_pid;
+ else
+ __entry->snd_task_id = -1;
+
__entry->cong = xprt->cong;
__entry->cwnd = xprt->cwnd;
__entry->wait = test_bit(XPRT_CWND_WAIT, &xprt->state);


2022-01-14 23:09:36

by Anna Schumaker

[permalink] [raw]
Subject: Re: [PATCH 0/3] Minor clean-ups for v5.17

Hi Chuck,

On Fri, Jan 14, 2022 at 3:19 AM Chuck Lever <[email protected]> wrote:
>
> Sorry, these seem to have gotten lost in the shuffle. They've been
> in my client-miscellany branch on git.kernel.org since late
> November. I must have forgotten to repost them for review.

These look fairly uncontroversial, so I've added them to my linux-next for 5.17.

Anna

>
> Trond rejected "SUNRPC: Don't dereference xprt->snd_task if it's
> a cookie" for v5.16, so I've recrafted it to hopefully address
> his concerns.
>
> ---
>
> Chuck Lever (3):
> xprtrdma: Remove final dprintk call sites from xprtrdma
> xprtrdma: Remove definitions of RPCDBG_FACILITY
> SUNRPC: Don't dereference xprt->snd_task if it's a cookie
>
>
> include/trace/events/sunrpc.h | 18 +++++++++++++-----
> net/sunrpc/xprtrdma/backchannel.c | 4 ----
> net/sunrpc/xprtrdma/frwr_ops.c | 4 ----
> net/sunrpc/xprtrdma/rpc_rdma.c | 4 ----
> net/sunrpc/xprtrdma/transport.c | 4 ----
> net/sunrpc/xprtrdma/verbs.c | 23 -----------------------
> 6 files changed, 13 insertions(+), 44 deletions(-)
>
> --
> Chuck Lever