Jiri Slaby says:
> I bisected to this ... as it breaks nfs3-only servers in 6.3.
> I.e. /etc/nfs.conf containing:
> [nfsd]
> vers4=no
>
> The client sees:
>  mount("10.0.2.15:/tmp", "/mnt", "nfs", 0, "vers=4.2,addr=10.0.2.15,clientad"...) = -1 EIO (Input/output error)
>  write(2, "mount.nfs: mount system call fai"..., 45
>  mount.nfs: mount system call failed for /mnt
>
> And the kernel says:
>  nfs4_discover_server_trunking unhandled error -5. Exiting with error EIO
Reported-by: NJiri Slaby <jirislaby@kernel.org>
Link: https://bugzilla.suse.com/show_bug.cgi?id=1210995
Fixes: 4bcf0343 ("SUNRPC: Set rq_accept_statp inside ->accept methods")
Tested-by: NJiri Slaby <jirislaby@kernel.org>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

A single RPC transaction that touches only a couple of pages means
rq_pvec will not be even close to full in svc_xpt_release(). This is
a common case.

Instead, just leave the pages in rq_pvec until it is completely
full. This improves the efficiency of the batch release mechanism
on workloads that involve small RPC messages.

The rq_pvec is also fully emptied just before thread exit.
Reviewed-by: NCalum Mackay <calum.mackay@oracle.com>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Instead of invoking put_page() one-at-a-time, pass the "response"
portion of rq_pages directly to release_pages() to reduce the number
of times each nfsd thread invokes a page allocator API.

Since svc_xprt_release() is not invoked while a client is waiting
for an RPC Reply, this is not expected to directly impact mean
request latencies on a lightly or moderately loaded server. However
as workload intensity increases, I expect somewhat better
scalability: the same number of server threads should be able to
handle more work.
Reviewed-by: NCalum Mackay <calum.mackay@oracle.com>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Clean-up: There doesn't seem to be a reason why this function is
stuck in a header. One thing it prevents is the convenient addition
of tracing. Moving it to a source file also makes the rq_respages
clean-up logic easier to find.
Reviewed-by: NCalum Mackay <calum.mackay@oracle.com>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Clean up: All callers of svc_process() ignore its return value, so
svc_process() can safely be converted to return void. Ditto for
svc_send().

The return value of ->xpo_sendto() is now used only as part of a
trace event.
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

There have been several bugs over the years where the NFSD splice
actor has attempted to write outside the rq_pages array.

This is a "should never happen" condition, but if for some reason
the pipe splice actor should attempt to walk past the end of
rq_pages, it needs to terminate the READ operation to prevent
corruption of the pointer addresses in the fields just beyond the
array.

A server crash is thus prevented. Since the code is not behaving,
the READ operation returns -EIO to the client. None of the READ
payload data can be trusted if the splice actor isn't operating as
expected.
Suggested-by: NJeff Layton <jlayton@kernel.org>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>
Reviewed-by: NJeff Layton <jlayton@kernel.org>

Fix a race where kthread_stop() may prevent the threadfn from ever getting
called.  If that happens the svc_rqst will not be cleaned up.

Fixes: ed6473dd ("NFSv4: Fix callback server shutdown")
Signed-off-by: NBenjamin Coddington <bcodding@redhat.com>
Reviewed-by: NJeff Layton <jlayton@kernel.org>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Eliminate the use of bus-locked operations in svc_xprt_enqueue(),
which is a hot path. Replace them with per-cpu variables to reduce
cross-CPU memory bus traffic.
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

 - Improves counting accuracy
 - Reduces cross-CPU memory traffic
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

To navigate around the space that svcauth_gss_accept() reserves
for the RPC payload body length and sequence number fields,
svcauth_gss_release() does a little dance with the reply's
accept_stat, moving the accept_stat value in the response buffer
down by two words.

Instead, let's have the ->accept() methods each set the proper
final location of the accept_stat to avoid having to move
things.
Reviewed-by: NJeff Layton <jlayton@kernel.org>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Currently, svcauth_gss_accept() pre-reserves response buffer space
for the RPC payload length and GSS sequence number before returning
to the dispatcher, which then adds the header's accept_stat field.

The problem is the accept_stat field is supposed to go before the
length and seq_num fields. So svcauth_gss_release() has to relocate
the accept_stat value (see svcauth_gss_prepare_to_wrap()).

To enable these fields to be added to the response buffer in the
correct (final) order, the pointer to the accept_stat has to be made
available to svcauth_gss_accept() so that it can set it before
reserving space for the length and seq_num fields.

As a first step, move the pointer to the location of the accept_stat
field into struct svc_rqst.
Reviewed-by: NJeff Layton <jlayton@kernel.org>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

The @resv parameter is no longer used.
Reviewed-by: NJeff Layton <jlayton@kernel.org>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

The main part of RPC header encoding and the formation of error
responses are now done using the xdr_stream helpers. Bounds checking
before each XDR data item is encoded makes the server's encoding
path safer against accidental buffer overflows.
Reviewed-by: NJeff Layton <jlayton@kernel.org>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Now that each ->accept method has been converted, the
svcxdr_init_encode() calls can be hoisted back up into the generic
RPC server code.
Reviewed-by: NJeff Layton <jlayton@kernel.org>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Refactor: So that the overhaul of each ->accept method can be done
in separate smaller patches, temporarily move the
svcxdr_init_encode() call into those methods.
Reviewed-by: NJeff Layton <jlayton@kernel.org>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Now that all vs_dispatch functions invoke svcxdr_init_encode(), it
is common code and can be pushed down into the generic RPC server.
Reviewed-by: NJeff Layton <jlayton@kernel.org>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

There's no RPC header field called rpc_stat; more precisely, the
variable appears to be recording an accept_stat value. But it looks
like we don't need to preserve this value at all, actually, so
simply remove the variable.
Reviewed-by: NJeff Layton <jlayton@kernel.org>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Now the entire RPC Call header parsing path is handled via struct
xdr_stream-based decoders.
Reviewed-by: NJeff Layton <jlayton@kernel.org>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Clean up: With xdr_stream decoding, the @argv parameter is no longer
used.
Reviewed-by: NJeff Layton <jlayton@kernel.org>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Done as part of hardening the server-side RPC header decoding path.
Reviewed-by: NJeff Layton <jlayton@kernel.org>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Clean up: Saving the RPC program number in two places is
unnecessary.
Reviewed-by: NJeff Layton <jlayton@kernel.org>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Clean up: Group these together for legibility.
Reviewed-by: NJeff Layton <jlayton@kernel.org>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Now that each ->accept method has been converted to use xdr_stream,
the svcxdr_init_decode() calls can be hoisted back up into the
generic RPC server code.

The dprintk in svc_authenticate() is removed, since
trace_svc_authenticate() reports the same information.
Reviewed-by: NJeff Layton <jlayton@kernel.org>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Refactor: So that the overhaul of each ->accept method can be done
in separate smaller patches, temporarily move the
svcxdr_init_decode() call into those methods.
Reviewed-by: NJeff Layton <jlayton@kernel.org>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Now that all vs_dispatch functions invoke svcxdr_init_decode(), it
is common code and can be pushed down into the generic RPC server.
Reviewed-by: NJeff Layton <jlayton@kernel.org>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

The premise that "Once an svc thread is scheduled and executing an
RPC, no other processes will touch svc_rqst::rq_flags" is false.
svc_xprt_enqueue() examines the RQ_BUSY flag in scheduled nfsd
threads when determining which thread to wake up next.

Found via KCSAN.

Fixes: 28df0988 ("SUNRPC: Use RMW bitops in single-threaded hot paths")
Reviewed-by: NJeff Layton <jlayton@kernel.org>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Due to several bugs caused by timers being re-armed after they are
shutdown and just before they are freed, a new state of timers was added
called "shutdown".  After a timer is set to this state, then it can no
longer be re-armed.

The following script was run to find all the trivial locations where
del_timer() or del_timer_sync() is called in the same function that the
object holding the timer is freed.  It also ignores any locations where
the timer->function is modified between the del_timer*() and the free(),
as that is not considered a "trivial" case.

This was created by using a coccinelle script and the following
commands:

    $ cat timer.cocci
    @@
    expression ptr, slab;
    identifier timer, rfield;
    @@
    (
    -       del_timer(&ptr->timer);
    +       timer_shutdown(&ptr->timer);
    |
    -       del_timer_sync(&ptr->timer);
    +       timer_shutdown_sync(&ptr->timer);
    )
      ... when strict
          when != ptr->timer
    (
            kfree_rcu(ptr, rfield);
    |
            kmem_cache_free(slab, ptr);
    |
            kfree(ptr);
    )

    $ spatch timer.cocci . > /tmp/t.patch
    $ patch -p1 < /tmp/t.patch

Link: https://lore.kernel.org/lkml/20221123201306.823305113@linutronix.de/Signed-off-by: NSteven Rostedt (Google) <rostedt@goodmis.org>
Acked-by: Pavel Machek <pavel@ucw.cz> [ LED ]
Acked-by: Kalle Valo <kvalo@kernel.org> [ wireless ]
Acked-by: Paolo Abeni <pabeni@redhat.com> [ networking ]
Signed-off-by: NLinus Torvalds <torvalds@linux-foundation.org>

Clean up: Simplify the tracepoint's only call site.

Also, I noticed that when svc_authenticate() returns SVC_COMPLETE,
it leaves rq_auth_stat set to an error value. That doesn't need to
be recorded in the trace log.
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>
Reviewed-by: NJeff Layton <jlayton@kernel.org>

Clean up after commit 22700f3c ("SUNRPC: Improve ordering of
transport processing").
Reviewed-by: NJeff Layton <jlayton@kernel.org>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Note the function returns a per-transport value, not a per-request
value (eg, one that is related to the size of the available send or
receive buffer space).
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Currently, SUNRPC clears the whole of .pc_argsize before processing
each incoming RPC transaction. Add an extra parameter to struct
svc_procedure to enable upper layers to reduce the amount of each
operation's argument structure that is zeroed by SUNRPC.

The size of struct nfsd4_compoundargs, in particular, is a lot to
clear on each incoming RPC Call. A subsequent patch will cut this
down to something closer to what NFSv2 and NFSv3 uses.

This patch should cause no behavior changes.
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Move exception handling code out of the hot path, and avoid the need
for a bswap of a non-constant.
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

I noticed CPU pipeline stalls while using perf.

Once an svc thread is scheduled and executing an RPC, no other
processes will touch svc_rqst::rq_flags. Thus bus-locked atomics are
not needed outside the svc thread scheduler.
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Clean up: There is one caller. The @cpu argument can be made
implicit now that a get_cpu/put_cpu pair is no longer needed.
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Hoist svo_function back into svc_serv and remove struct
svc_serv_ops, since the struct is now devoid of fields.
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

struct svc_serv_ops is about to be removed.

Neil Brown says:
> I suspect svo_module can go as well - I don't think the thread is
> ever the thing that primarily keeps a module active.

A random sample of kthread_create() callers shows sunrpc is the only
one that manages module reference count in this way.
Suggested-by: NNeil Brown <neilb@suse.de>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Clean up: svc_shutdown_net() now does nothing but call
svc_close_net(). Replace all external call sites.

svc_close_net() is renamed to be the inverse of svc_xprt_create().
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Clean up: Use the "svc_xprt_<task>" function naming convention as
is used for other external APIs.
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>

Clean up. Neil observed that "any code that calls svc_shutdown_net()
knows what the shutdown function should be, and so can call it
directly."
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>
Reviewed-by: NNeilBrown <neilb@suse.de>

svc_set_num_threads() does everything that lockd_start_svc() does, except
set sv_maxconn.  It also (when passed 0) finds the threads and
stops them with kthread_stop().

So move the setting for sv_maxconn, and use svc_set_num_thread()

We now don't need nlmsvc_task.

Now that we use svc_set_num_threads() it makes sense to set svo_module.
This request that the thread exists with module_put_and_exit().
Also fix the documentation for svo_module to make this explicit.

svc_prepare_thread is now only used where it is defined, so it can be
made static.
Signed-off-by: NNeilBrown <neilb@suse.de>
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>