- 18 Jun, 2023 2 commits
-
-
Chuck Lever authored
I find the naming of nfsd_init_net() and nfsd_startup_net() to be confusingly similar. Rename the namespace initialization and tear- down ops and add comments to distinguish their separate purposes. Signed-off-by:Chuck Lever <chuck.lever@oracle.com>
-
Jeff Layton authored
Commit f5f9d4a3 ("nfsd: move reply cache initialization into nfsd startup") moved the initialization of the reply cache into nfsd startup, but didn't account for the stats counters, which can be accessed before nfsd is ever started. The result can be a NULL pointer dereference when someone accesses /proc/fs/nfsd/reply_cache_stats while nfsd is still shut down. This is a regression and a user-triggerable oops in the right situation: - non-x86_64 arch - /proc/fs/nfsd is mounted in the namespace - nfsd is not started in the namespace - unprivileged user calls "cat /proc/fs/nfsd/reply_cache_stats" Although this is easy to trigger on some arches (like aarch64), on x86_64, calling this_cpu_ptr(NULL) evidently returns a pointer to the fixed_percpu_data. That struct looks just enough like a newly initialized percpu var to allow nfsd_reply_cache_stats_show to access it without Oopsing. Move the initialization of the per-net+per-cpu reply-cache counters back into nfsd_init_net, while leaving the rest of the reply cache allocations to be done at nfsd startup time. Kudos to Eirik who did most of the legwork to track this down. Cc: stable@vger.kernel.org # v6.3+ Fixes: f5f9d4a3 ("nfsd: move reply cache initialization into nfsd startup") Reported-and-tested-by:
Eirik Fuller <efuller@redhat.com> Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2215429Signed-off-by:
Jeff Layton <jlayton@kernel.org> Signed-off-by:
-
- 17 Jun, 2023 11 commits
-
-
Chuck Lever authored
$ make C=1 W=1 net/sunrpc/svc.o make[1]: Entering directory 'linux/obj/manet.1015granger.net' GEN Makefile CALL linux/server-development/scripts/checksyscalls.sh DESCEND objtool INSTALL libsubcmd_headers DESCEND bpf/resolve_btfids INSTALL libsubcmd_headers CC [M] net/sunrpc/svc.o CHECK linux/server-development/net/sunrpc/svc.c linux/server-development/net/sunrpc/svc.c:1225:9: warning: incorrect type in argument 1 (different address spaces) linux/server-development/net/sunrpc/svc.c:1225:9: expected struct spinlock [usertype] *lock linux/server-development/net/sunrpc/svc.c:1225:9: got struct spinlock [noderef] __rcu * linux/server-development/net/sunrpc/svc.c:1227:40: warning: incorrect type in argument 1 (different address spaces) linux/server-development/net/sunrpc/svc.c:1227:40: expected struct spinlock [usertype] *lock linux/server-development/net/sunrpc/svc.c:1227:40: got struct spinlock [noderef] __rcu * make[1]: Leaving directory 'linux/obj/manet.1015granger.net' Warning introduced by commit 913292c9 ("sched.h: Annotate sighand_struct with __rcu"). Signed-off-by:
-
Azeem Shaikh authored
Part of an effort to remove strlcpy() tree-wide [1]. Direct replacement is safe here since the getter in kernel_params_ops handles -errno return [2]. [1] https://github.com/KSPP/linux/issues/89 [2] https://elixir.bootlin.com/linux/v6.4-rc6/source/include/linux/moduleparam.h#L52Signed-off-by:
Azeem Shaikh <azeemshaikh38@gmail.com> Signed-off-by:
-
Chuck Lever authored
Remove a couple of dprintk call sites that are of little value. Reviewed-by:
Tom Talpey <tom@talpey.com> Signed-off-by:
-
Chuck Lever authored
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. Reviewed-by:
-
Chuck Lever authored
Clean up. Reviewed-by:
-
Chuck Lever authored
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. Reviewed-by:
-
Chuck Lever authored
Try to catch incorrect calling contexts mechanically rather than by code review. Reviewed-by:
-
Chuck Lever authored
Clean up: de-duplicate some common code. Reviewed-by:
-
Chuck Lever authored
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. Reviewed-by:
-
Chuck Lever authored
Now that we have bulk page allocation and release APIs, it's more efficient to use those than it is for nfsd threads to wait for send completions. Previous patches have eliminated the calls to wait_for_completion() and complete(), in order to avoid scheduler overhead. Now release pages-under-I/O in the send completion handler using the efficient bulk release API. I've measured a 7% reduction in cumulative CPU utilization in svc_rdma_sendto(), svc_rdma_wc_send(), and svc_xprt_release(). In particular, using release_pages() instead of complete() cuts the time per svc_rdma_wc_send() call by two-thirds. This helps improve scalability because svc_rdma_wc_send() is single-threaded per connection. Reviewed-by:
-
Chuck Lever authored
I noticed that svc_rqst_release_pages() was still unnecessarily releasing a page when svc_rdma_recvfrom() returns zero. Fixes: a53d5cb0 ("svcrdma: Avoid releasing a page in svc_xprt_release()") Reviewed-by:
-
- 12 Jun, 2023 11 commits
-
-
Chuck Lever authored
Get rid of the completion wait in svc_rdma_sendto(), and release pages in the send completion handler again. A subsequent patch will handle releasing those pages more efficiently. Reverted by hand: patch -R would not apply cleanly. Reviewed-by:
-
Chuck Lever authored
Pre-requisite for releasing pages in the send completion handler. Reverted by hand: patch -R would not apply cleanly. Reviewed-by:
-
Chuck Lever authored
Pre-requisite for releasing pages in the send completion handler. Reverted by hand: patch -R would not apply cleanly. Reviewed-by:
-
Dai Ngo authored
Modified nfsd4_encode_open to encode the op_recall flag properly for OPEN result with write delegation granted. Signed-off-by:
Dai Ngo <dai.ngo@oracle.com> Reviewed-by:
-
Chuck Lever authored
At LFS 2023, it was suggested we should publicly document the name and email of reviewers who new contributors can trust. This also gives them some recognition for their work as reviewers. Acked-by:
-
Chuck Lever authored
Ensure that Bruce's old e-mail addresses map to his current one so he doesn't miss out on all the fun. Signed-off-by: -
Chuck Lever authored
The physical device's favored NUMA node ID is available when allocating a rw_ctxt. Use that value instead of relying on the assumption that the memory allocation happens to be running on a node close to the device. Signed-off-by: -
Chuck Lever authored
The physical device's favored NUMA node ID is available when allocating a send_ctxt. Use that value instead of relying on the assumption that the memory allocation happens to be running on a node close to the device. Signed-off-by: -
Chuck Lever authored
The physical device's favored NUMA node ID is available when allocating a recv_ctxt. Use that value instead of relying on the assumption that the memory allocation happens to be running on a node close to the device. This clean up eliminates the hack of destroying recv_ctxts that were not created by the receive CQ thread -- recv_ctxts are now always allocated on a "good" node. Signed-off-by: -
Chuck Lever authored
The physical device's NUMA node ID is available when allocating an svc_xprt for an incoming connection. Use that value to ensure the svc_xprt structure is allocated on the NUMA node closest to the device. Signed-off-by: -
NeilBrown authored
The below-mentioned patch was intended to simplify refcounting on the svc_serv used by locked. The goal was to only ever have a single reference from the single thread. To that end we dropped a call to lockd_start_svc() (except when creating thread) which would take a reference, and dropped the svc_put(serv) that would drop that reference. Unfortunately we didn't also remove the svc_get() from lockd_create_svc() in the case where the svc_serv already existed. So after the patch: - on the first call the svc_serv was allocated and the one reference was given to the thread, so there are no extra references - on subsequent calls svc_get() was called so there is now an extra reference. This is clearly not consistent. The inconsistency is also clear in the current code in lockd_get() takes *two* references, one on nlmsvc_serv and one by incrementing nlmsvc_users. This clearly does not match lockd_put(). So: drop that svc_get() from lockd_get() (which used to be in lockd_create_svc(). Reported-by:
Ido Schimmel <idosch@idosch.org> Closes: https://lore.kernel.org/linux-nfs/ZHsI%2FH16VX9kJQX1@shredder/T/#u Fixes: b73a2972 ("lockd: move lockd_start_svc() call into lockd_create_svc()") Signed-off-by:
NeilBrown <neilb@suse.de> Tested-by:
Ido Schimmel <idosch@nvidia.com> Signed-off-by:
-
- 11 Jun, 2023 6 commits
-
-
Jeff Layton authored
nfsd calls fh_getattr to get the latest inode attrs for pre/post-op info. In the event that fh_getattr fails, it resorts to scraping cached values out of the inode directly. Since these attributes are optional, we can just skip providing them altogether when this happens. Signed-off-by:
-
Chuck Lever authored
nfsd_readv()'s consumers now use nfsd_iter_read(). Signed-off-by: -
Chuck Lever authored
Now that the preparation of an rq_vec has been removed from the generic read path, nfsd_splice_read() no longer needs to reset rq_next_page. nfsd4_encode_read() calls nfsd_splice_read() directly. As far as I can ascertain, resetting rq_next_page for NFSv4 splice reads is unnecessary because rq_next_page is already set correctly. Moreover, resetting it might even be incorrect if previous operations in the COMPOUND have already consumed at least a page of the send buffer. I would expect that the result would be encoding the READ payload over previously-encoded results. Signed-off-by: -
Chuck Lever authored
Accrue the following benefits: a) Deduplicate this common bit of code. b) Don't prepare rq_vec for NFSv2 and NFSv3 spliced reads, which don't use rq_vec. This is already the case for nfsd4_encode_read(). c) Eventually, converting NFSD's read path to use a bvec iterator will be simpler. In the next patch, nfsd_iter_read() will replace nfsd_readv() for all NFS versions. Signed-off-by: -
Chuck Lever authored
A GETATTR with a large result can advance xdr->page_ptr without updating rq_next_page. If a splice READ follows that GETATTR in the COMPOUND, nfsd_splice_actor can start splicing at the wrong page. I've also seen READLINK and READDIR leave rq_next_page in an unmodified state. There are potentially a myriad of combinations like this, so play it safe: move the rq_next_page update to nfsd4_encode_operation. Signed-off-by: -
Chuck Lever authored
Commit 15b23ef5 ("nfsd4: fix corruption of NFSv4 read data") encountered exactly the same issue: after a splice read, a filesystem-owned page is left in rq_pages[]; the symptoms are the same as described there. If the computed number of pages in nfsd4_encode_splice_read() is not exactly the same as the actual number of pages that were consumed by nfsd_splice_actor() (say, because of a bug) then hilarity ensues. Instead of recomputing the page offset based on the size of the payload, use rq_next_page, which is already properly updated by nfsd_splice_actor(), to cause svc_rqst_release_pages() to operate correctly in every instance. This is a defensive change since we believe that after commit 27c934dd ("nfsd: don't replace page in rq_pages if it's a continuation of last page") has been applied, there are no known opportunities for nfsd_splice_actor() to screw up. So I'm not marking it for stable backport. Reported-by:
Andy Zlotek <andy.zlotek@oracle.com> Suggested-by:
Calum Mackay <calum.mackay@oracle.com> Signed-off-by:
-
- 05 Jun, 2023 10 commits
-
-
Chuck Lever authored
All other NFSv[23] procedures manage to keep page_ptr and rq_next_page in lock step. Signed-off-by: -
Chuck Lever authored
De-duplicate "reserve_space; encode_cinfo". Signed-off-by: -
Chuck Lever authored
Deduplicate some common code. Signed-off-by: -
Chuck Lever authored
Clean up: Use the bulk page allocator when filling a server thread's buffer page array. Signed-off-by: -
Chuck Lever authored
svc_init_buffer() is careful to allocate the initial set of server thread buffer pages from memory on the local NUMA node. svc_alloc_arg() should also be that careful. Signed-off-by: -
Chuck Lever authored
Add trace log eye-catchers that record the arguments used to configure NFSD. This helps when troubleshooting the NFSD administrative interfaces. These tracepoints can capture NFSD start-up and shutdown times and parameters, changes in lease time and thread count, and a request to end the namespace's NFSv4 grace period, in addition to the set of NFS versions that are enabled. Reviewed-by:
-
Chuck Lever authored
For easier readability, follow the common convention: if (error) handle_error; continue_normally; No behavior change is expected. Reviewed-by: -
Chuck Lever authored
Reviewed-by:
-
Chuck Lever authored
Capture a timestamp and pointer address during the creation and destruction of struct svc_sock to record its lifetime. This helps to diagnose transport reference counting issues. Reviewed-by:
-
Chuck Lever authored
The -ENOMEM arm could fire repeatedly if the system runs low on memory, so remove it. Don't bother to trace -EAGAIN error events, since those fire after a listener is created (with no work done) and once again after an accept has been handled successfully (again, with no work done). Reviewed-by:
-
