These are not strongly related to each other, but there was a whole
collection such that I didn't feel like posting each individually.
---
Chuck Lever (7):
SUNRPC: Move initialization of rq_stime
NFSD: Add an nfsd4_encode_nfstime4() helper
svcrdma: Convert "might sleep" comment into a code annotation
svcrdma: trace cc_release calls
svcrdma: Remove an unused argument from __svc_rdma_put_rw_ctxt()
SUNRPC: Fix comments for transport class registration
SUNRPC: Remove transport class dprintk call sites
fs/nfsd/nfs4xdr.c | 46 +++++++++++++++------------
include/trace/events/rpcrdma.h | 8 +++++
net/sunrpc/svc_xprt.c | 18 ++++++++---
net/sunrpc/xprtrdma/svc_rdma_rw.c | 14 ++++----
net/sunrpc/xprtrdma/svc_rdma_sendto.c | 2 ++
5 files changed, 58 insertions(+), 30 deletions(-)
--
Chuck Lever
From: Chuck Lever <[email protected]>
Try to catch incorrect calling contexts mechanically rather than by
code review.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/xprtrdma/svc_rdma_rw.c | 5 +++--
net/sunrpc/xprtrdma/svc_rdma_sendto.c | 2 ++
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/sunrpc/xprtrdma/svc_rdma_rw.c b/net/sunrpc/xprtrdma/svc_rdma_rw.c
index 068c365e7812..59ea87b5f458 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_rw.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_rw.c
@@ -353,8 +353,7 @@ static void svc_rdma_wc_read_done(struct ib_cq *cq, struct ib_wc *wc)
return;
}
-/* This function sleeps when the transport's Send Queue is congested.
- *
+/*
* Assumptions:
* - If ib_post_send() succeeds, only one completion is expected,
* even if one or more WRs are flushed. This is true when posting
@@ -369,6 +368,8 @@ static int svc_rdma_post_chunk_ctxt(struct svc_rdma_chunk_ctxt *cc)
struct ib_cqe *cqe;
int ret;
+ might_sleep();
+
if (cc->cc_sqecount > rdma->sc_sq_depth)
return -EINVAL;
diff --git a/net/sunrpc/xprtrdma/svc_rdma_sendto.c b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
index 24228f3611e8..c6644cca52c5 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_sendto.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
@@ -315,6 +315,8 @@ int svc_rdma_send(struct svcxprt_rdma *rdma, struct svc_rdma_send_ctxt *ctxt)
struct ib_send_wr *wr = &ctxt->sc_send_wr;
int ret;
+ might_sleep();
+
/* Sync the transport header buffer */
ib_dma_sync_single_for_device(rdma->sc_pd->device,
wr->sg_list[0].addr,
From: Chuck Lever <[email protected]>
Clean up: de-duplicate some common code.
Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfsd/nfs4xdr.c | 46 ++++++++++++++++++++++++++--------------------
1 file changed, 26 insertions(+), 20 deletions(-)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 41234fc3b905..f3be2a6055f2 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2541,6 +2541,20 @@ static __be32 *encode_change(__be32 *p, struct kstat *stat, struct inode *inode,
return p;
}
+static __be32 nfsd4_encode_nfstime4(struct xdr_stream *xdr,
+ struct timespec64 *tv)
+{
+ __be32 *p;
+
+ p = xdr_reserve_space(xdr, XDR_UNIT * 3);
+ if (!p)
+ return nfserr_resource;
+
+ p = xdr_encode_hyper(p, (s64)tv->tv_sec);
+ *p = cpu_to_be32(tv->tv_nsec);
+ return nfs_ok;
+}
+
/*
* ctime (in NFSv4, time_metadata) is not writeable, and the client
* doesn't really care what resolution could theoretically be stored by
@@ -3348,11 +3362,9 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
p = xdr_encode_hyper(p, dummy64);
}
if (bmval1 & FATTR4_WORD1_TIME_ACCESS) {
- p = xdr_reserve_space(xdr, 12);
- if (!p)
- goto out_resource;
- p = xdr_encode_hyper(p, (s64)stat.atime.tv_sec);
- *p++ = cpu_to_be32(stat.atime.tv_nsec);
+ status = nfsd4_encode_nfstime4(xdr, &stat.atime);
+ if (status)
+ goto out;
}
if (bmval1 & FATTR4_WORD1_TIME_DELTA) {
p = xdr_reserve_space(xdr, 12);
@@ -3361,25 +3373,19 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
p = encode_time_delta(p, d_inode(dentry));
}
if (bmval1 & FATTR4_WORD1_TIME_METADATA) {
- p = xdr_reserve_space(xdr, 12);
- if (!p)
- goto out_resource;
- p = xdr_encode_hyper(p, (s64)stat.ctime.tv_sec);
- *p++ = cpu_to_be32(stat.ctime.tv_nsec);
+ status = nfsd4_encode_nfstime4(xdr, &stat.ctime);
+ if (status)
+ goto out;
}
if (bmval1 & FATTR4_WORD1_TIME_MODIFY) {
- p = xdr_reserve_space(xdr, 12);
- if (!p)
- goto out_resource;
- p = xdr_encode_hyper(p, (s64)stat.mtime.tv_sec);
- *p++ = cpu_to_be32(stat.mtime.tv_nsec);
+ status = nfsd4_encode_nfstime4(xdr, &stat.mtime);
+ if (status)
+ goto out;
}
if (bmval1 & FATTR4_WORD1_TIME_CREATE) {
- p = xdr_reserve_space(xdr, 12);
- if (!p)
- goto out_resource;
- p = xdr_encode_hyper(p, (s64)stat.btime.tv_sec);
- *p++ = cpu_to_be32(stat.btime.tv_nsec);
+ status = nfsd4_encode_nfstime4(xdr, &stat.btime);
+ if (status)
+ goto out;
}
if (bmval1 & FATTR4_WORD1_MOUNTED_ON_FILEID) {
u64 ino = stat.ino;
From: Chuck Lever <[email protected]>
This event brackets the svcrdma_post_* trace points. If this trace
event is enabled but does not appear as expected, that indicates a
chunk_ctxt leak.
Signed-off-by: Chuck Lever <[email protected]>
---
include/trace/events/rpcrdma.h | 8 ++++++++
net/sunrpc/xprtrdma/svc_rdma_rw.c | 2 ++
2 files changed, 10 insertions(+)
diff --git a/include/trace/events/rpcrdma.h b/include/trace/events/rpcrdma.h
index 8f461e04e5f0..f8069ef2ee0f 100644
--- a/include/trace/events/rpcrdma.h
+++ b/include/trace/events/rpcrdma.h
@@ -2112,6 +2112,14 @@ DEFINE_POST_CHUNK_EVENT(read);
DEFINE_POST_CHUNK_EVENT(write);
DEFINE_POST_CHUNK_EVENT(reply);
+DEFINE_EVENT(svcrdma_post_chunk_class, svcrdma_cc_release,
+ TP_PROTO(
+ const struct rpc_rdma_cid *cid,
+ int sqecount
+ ),
+ TP_ARGS(cid, sqecount)
+);
+
TRACE_EVENT(svcrdma_wc_read,
TP_PROTO(
const struct ib_wc *wc,
diff --git a/net/sunrpc/xprtrdma/svc_rdma_rw.c b/net/sunrpc/xprtrdma/svc_rdma_rw.c
index 59ea87b5f458..9836406cc41e 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_rw.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_rw.c
@@ -191,6 +191,8 @@ static void svc_rdma_cc_release(struct svc_rdma_chunk_ctxt *cc,
struct svc_rdma_rw_ctxt *ctxt;
LLIST_HEAD(free);
+ trace_svcrdma_cc_release(&cc->cc_cid, cc->cc_sqecount);
+
first = last = NULL;
while ((ctxt = svc_rdma_next_ctxt(&cc->cc_rwctxts)) != NULL) {
list_del(&ctxt->rw_list);
From: Chuck Lever <[email protected]>
Micro-optimization: Call ktime_get() only when ->xpo_recvfrom() has
given us a full RPC message to process. rq_stime isn't used
otherwise, so this avoids pointless work.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/svc_xprt.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 45bdc2e38631..4af83a0fd395 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -852,7 +852,6 @@ static int svc_handle_xprt(struct svc_rqst *rqstp, struct svc_xprt *xprt)
len = svc_deferred_recv(rqstp);
else
len = xprt->xpt_ops->xpo_recvfrom(rqstp);
- rqstp->rq_stime = ktime_get();
rqstp->rq_reserved = serv->sv_max_mesg;
atomic_add(rqstp->rq_reserved, &xprt->xpt_reserved);
} else
@@ -895,6 +894,7 @@ int svc_recv(struct svc_rqst *rqstp, long timeout)
err = -EAGAIN;
if (len <= 0)
goto out_release;
+
trace_svc_xdr_recvfrom(&rqstp->rq_arg);
clear_bit(XPT_OLD, &xprt->xpt_flags);
@@ -903,6 +903,7 @@ int svc_recv(struct svc_rqst *rqstp, long timeout)
if (serv->sv_stats)
serv->sv_stats->netcnt++;
+ rqstp->rq_stime = ktime_get();
return len;
out_release:
rqstp->rq_res.len = 0;
From: Chuck Lever <[email protected]>
Clean up.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/xprtrdma/svc_rdma_rw.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/net/sunrpc/xprtrdma/svc_rdma_rw.c b/net/sunrpc/xprtrdma/svc_rdma_rw.c
index 9836406cc41e..e460e25a1d6d 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_rw.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_rw.c
@@ -84,8 +84,7 @@ svc_rdma_get_rw_ctxt(struct svcxprt_rdma *rdma, unsigned int sges)
return NULL;
}
-static void __svc_rdma_put_rw_ctxt(struct svcxprt_rdma *rdma,
- struct svc_rdma_rw_ctxt *ctxt,
+static void __svc_rdma_put_rw_ctxt(struct svc_rdma_rw_ctxt *ctxt,
struct llist_head *list)
{
sg_free_table_chained(&ctxt->rw_sg_table, SG_CHUNK_SIZE);
@@ -95,7 +94,7 @@ static void __svc_rdma_put_rw_ctxt(struct svcxprt_rdma *rdma,
static void svc_rdma_put_rw_ctxt(struct svcxprt_rdma *rdma,
struct svc_rdma_rw_ctxt *ctxt)
{
- __svc_rdma_put_rw_ctxt(rdma, ctxt, &rdma->sc_rw_ctxts);
+ __svc_rdma_put_rw_ctxt(ctxt, &rdma->sc_rw_ctxts);
}
/**
@@ -200,7 +199,7 @@ static void svc_rdma_cc_release(struct svc_rdma_chunk_ctxt *cc,
rdma_rw_ctx_destroy(&ctxt->rw_ctx, rdma->sc_qp,
rdma->sc_port_num, ctxt->rw_sg_table.sgl,
ctxt->rw_nents, dir);
- __svc_rdma_put_rw_ctxt(rdma, ctxt, &free);
+ __svc_rdma_put_rw_ctxt(ctxt, &free);
ctxt->rw_node.next = first;
first = &ctxt->rw_node;
From: Chuck Lever <[email protected]>
Remove a couple of dprintk call sites that are of little value.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/svc_xprt.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 6d70278bd88d..ebdc2f70af90 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -86,8 +86,6 @@ int svc_reg_xprt_class(struct svc_xprt_class *xcl)
struct svc_xprt_class *cl;
int res = -EEXIST;
- dprintk("svc: Adding svc transport class '%s'\n", xcl->xcl_name);
-
INIT_LIST_HEAD(&xcl->xcl_list);
spin_lock(&svc_xprt_class_lock);
/* Make sure there isn't already a class with the same name */
@@ -110,7 +108,6 @@ EXPORT_SYMBOL_GPL(svc_reg_xprt_class);
*/
void svc_unreg_xprt_class(struct svc_xprt_class *xcl)
{
- dprintk("svc: Removing svc transport class '%s'\n", xcl->xcl_name);
spin_lock(&svc_xprt_class_lock);
list_del_init(&xcl->xcl_list);
spin_unlock(&svc_xprt_class_lock);
From: Chuck Lever <[email protected]>
The preceding block comment before svc_register_xprt_class() is
not related to that function.
While we're here, add proper documenting comments for these two
publicly-visible functions.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/svc_xprt.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 4af83a0fd395..6d70278bd88d 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -74,6 +74,13 @@ static LIST_HEAD(svc_xprt_class_list);
* that no other thread will be using the transport or will
* try to set XPT_DEAD.
*/
+
+/**
+ * svc_reg_xprt_class - Register a server-side RPC transport class
+ * @xcl: New transport class to be registered
+ *
+ * Returns zero on success; otherwise a negative errno is returned.
+ */
int svc_reg_xprt_class(struct svc_xprt_class *xcl)
{
struct svc_xprt_class *cl;
@@ -96,6 +103,11 @@ int svc_reg_xprt_class(struct svc_xprt_class *xcl)
}
EXPORT_SYMBOL_GPL(svc_reg_xprt_class);
+/**
+ * svc_unreg_xprt_class - Unregister a server-side RPC transport class
+ * @xcl: Transport class to be unregistered
+ *
+ */
void svc_unreg_xprt_class(struct svc_xprt_class *xcl)
{
dprintk("svc: Removing svc transport class '%s'\n", xcl->xcl_name);
On Mon, 2023-06-12 at 10:13 -0400, Chuck Lever wrote:
> These are not strongly related to each other, but there was a whole
> collection such that I didn't feel like posting each individually.
>
> ---
>
> Chuck Lever (7):
> SUNRPC: Move initialization of rq_stime
> NFSD: Add an nfsd4_encode_nfstime4() helper
> svcrdma: Convert "might sleep" comment into a code annotation
> svcrdma: trace cc_release calls
> svcrdma: Remove an unused argument from __svc_rdma_put_rw_ctxt()
> SUNRPC: Fix comments for transport class registration
> SUNRPC: Remove transport class dprintk call sites
>
>
> fs/nfsd/nfs4xdr.c | 46 +++++++++++++++------------
> include/trace/events/rpcrdma.h | 8 +++++
> net/sunrpc/svc_xprt.c | 18 ++++++++---
> net/sunrpc/xprtrdma/svc_rdma_rw.c | 14 ++++----
> net/sunrpc/xprtrdma/svc_rdma_sendto.c | 2 ++
> 5 files changed, 58 insertions(+), 30 deletions(-)
>
> --
> Chuck Lever
>
This all looks good to me:
Reviewed-by: Jeff Layton <[email protected]>
On 6/12/2023 10:27 AM, Jeff Layton wrote:
> On Mon, 2023-06-12 at 10:13 -0400, Chuck Lever wrote:
>> These are not strongly related to each other, but there was a whole
>> collection such that I didn't feel like posting each individually.
>>
>> ---
>>
>> Chuck Lever (7):
>> SUNRPC: Move initialization of rq_stime
>> NFSD: Add an nfsd4_encode_nfstime4() helper
>> svcrdma: Convert "might sleep" comment into a code annotation
>> svcrdma: trace cc_release calls
>> svcrdma: Remove an unused argument from __svc_rdma_put_rw_ctxt()
>> SUNRPC: Fix comments for transport class registration
>> SUNRPC: Remove transport class dprintk call sites
>>
>>
>> fs/nfsd/nfs4xdr.c | 46 +++++++++++++++------------
>> include/trace/events/rpcrdma.h | 8 +++++
>> net/sunrpc/svc_xprt.c | 18 ++++++++---
>> net/sunrpc/xprtrdma/svc_rdma_rw.c | 14 ++++----
>> net/sunrpc/xprtrdma/svc_rdma_sendto.c | 2 ++
>> 5 files changed, 58 insertions(+), 30 deletions(-)
>>
>> --
>> Chuck Lever
>>
>
> This all looks good to me:
>
> Reviewed-by: Jeff Layton <[email protected]>
>
Ditto.
Acked-by: Tom Talpey <[email protected]>
> On Jun 13, 2023, at 9:29 AM, Tom Talpey <[email protected]> wrote:
>
> On 6/12/2023 10:27 AM, Jeff Layton wrote:
>> On Mon, 2023-06-12 at 10:13 -0400, Chuck Lever wrote:
>>> These are not strongly related to each other, but there was a whole
>>> collection such that I didn't feel like posting each individually.
>>>
>>> ---
>>>
>>> Chuck Lever (7):
>>> SUNRPC: Move initialization of rq_stime
>>> NFSD: Add an nfsd4_encode_nfstime4() helper
>>> svcrdma: Convert "might sleep" comment into a code annotation
>>> svcrdma: trace cc_release calls
>>> svcrdma: Remove an unused argument from __svc_rdma_put_rw_ctxt()
>>> SUNRPC: Fix comments for transport class registration
>>> SUNRPC: Remove transport class dprintk call sites
>>>
>>>
>>> fs/nfsd/nfs4xdr.c | 46 +++++++++++++++------------
>>> include/trace/events/rpcrdma.h | 8 +++++
>>> net/sunrpc/svc_xprt.c | 18 ++++++++---
>>> net/sunrpc/xprtrdma/svc_rdma_rw.c | 14 ++++----
>>> net/sunrpc/xprtrdma/svc_rdma_sendto.c | 2 ++
>>> 5 files changed, 58 insertions(+), 30 deletions(-)
>>>
>>> --
>>> Chuck Lever
>>>
>> This all looks good to me:
>> Reviewed-by: Jeff Layton <[email protected]>
>
> Ditto.
>
> Acked-by: Tom Talpey <[email protected]>
Thanks, applied and pushed.
--
Chuck Lever