1. 08 Jan, 2022 1 commit
  2. 13 Dec, 2021 25 commits
    • Chuck Lever's avatar
      SUNRPC: Remove low signal-to-noise tracepoints · 5089f3d9
      Chuck Lever authored
      I'm about to add more information to the server-side SUNRPC
      tracepoints, so I'm going to offset the increased trace log
      consumption by getting rid of some tracepoints that fire frequently
      but don't offer much value.
      
      trace_svc_xprt_received() was useful for debugging, perhaps, but
      is not generally informative.
      
      trace_svc_handle_xprt() reports largely the same information as
      trace_svc_xdr_recvfrom().
      
      As a clean-up, rename trace_svc_xprt_do_enqueue() to match
      svc_xprt_dequeue().
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      5089f3d9
    • NeilBrown's avatar
      NFSD: simplify per-net file cache management · 1463b38e
      NeilBrown authored
      We currently have a 'laundrette' for closing cached files - a different
      work-item for each network-namespace.
      
      These 'laundrettes' (aka struct nfsd_fcache_disposal) are currently on a
      list, and are freed using rcu.
      
      The list is not necessary as we have a per-namespace structure (struct
      nfsd_net) which can hold a link to the nfsd_fcache_disposal.
      The use of kfree_rcu is also unnecessary as the cache is cleaned of all
      files associated with a given namespace, and no new files can be added,
      before the nfsd_fcache_disposal is freed.
      
      So add a '->fcache_disposal' link to nfsd_net, and discard the list
      management and rcu usage.
      Signed-off-by: default avatarNeilBrown <neilb@suse.de>
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      1463b38e
    • Jiapeng Chong's avatar
      NFSD: Fix inconsistent indenting · 1e37d0e5
      Jiapeng Chong authored
      Eliminate the follow smatch warning:
      
      fs/nfsd/nfs4xdr.c:4766 nfsd4_encode_read_plus_hole() warn: inconsistent
      indenting.
      Reported-by: default avatarAbaci Robot <abaci@linux.alibaba.com>
      Signed-off-by: default avatarJiapeng Chong <jiapeng.chong@linux.alibaba.com>
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      1e37d0e5
    • Chuck Lever's avatar
      NFSD: Remove be32_to_cpu() from DRC hash function · 7578b2f6
      Chuck Lever authored
      Commit 7142b98d ("nfsd: Clean up drc cache in preparation for
      global spinlock elimination"), billed as a clean-up, added
      be32_to_cpu() to the DRC hash function without explanation. That
      commit removed two comments that state that byte-swapping in the
      hash function is unnecessary without explaining whether there was
      a need for that change.
      
      On some Intel CPUs, the swab32 instruction is known to cause a CPU
      pipeline stall. be32_to_cpu() does not add extra randomness, since
      the hash multiplication is done /before/ shifting to the high-order
      bits of the result.
      
      As a micro-optimization, remove the unnecessary transform from the
      DRC hash function.
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      7578b2f6
    • NeilBrown's avatar
      NFS: switch the callback service back to non-pooled. · 23a1a573
      NeilBrown authored
      Now that thread management is consistent there is no need for
      nfs-callback to use svc_create_pooled() as introduced in Commit
      df807fff ("NFSv4.x/callback: Create the callback service through
      svc_create_pooled").  So switch back to svc_create().
      
      If service pools were configured, but the number of threads were left at
      '1', nfs callback may not work reliably when svc_create_pooled() is used.
      Signed-off-by: default avatarNeilBrown <neilb@suse.de>
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      23a1a573
    • NeilBrown's avatar
      lockd: use svc_set_num_threads() for thread start and stop · 6b044fba
      NeilBrown authored
      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: default avatarNeilBrown <neilb@suse.de>
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      6b044fba
    • NeilBrown's avatar
      SUNRPC: always treat sv_nrpools==1 as "not pooled" · 93aa619e
      NeilBrown authored
      Currently 'pooled' services hold a reference on the pool_map, and
      'unpooled' services do not.
      svc_destroy() uses the presence of ->svo_function (via
      svc_serv_is_pooled()) to determine if the reference should be dropped.
      There is no direct correlation between being pooled and the use of
      svo_function, though in practice, lockd is the only non-pooled service,
      and the only one not to use svo_function.
      
      This is untidy and would cause problems if we changed lockd to use
      svc_set_num_threads(), which requires the use of ->svo_function.
      
      So change the test for "is the service pooled" to "is sv_nrpools > 1".
      
      This means that when svc_pool_map_get() returns 1, it must NOT take a
      reference to the pool.
      
      We discard svc_serv_is_pooled(), and test sv_nrpools directly.
      Signed-off-by: default avatarNeilBrown <neilb@suse.de>
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      93aa619e
    • NeilBrown's avatar
      SUNRPC: move the pool_map definitions (back) into svc.c · cf0e124e
      NeilBrown authored
      These definitions are not used outside of svc.c, and there is no
      evidence that they ever have been.  So move them into svc.c
      and make the declarations 'static'.
      Signed-off-by: default avatarNeilBrown <neilb@suse.de>
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      cf0e124e
    • NeilBrown's avatar
      lockd: rename lockd_create_svc() to lockd_get() · ecd3ad68
      NeilBrown authored
      lockd_create_svc() already does an svc_get() if the service already
      exists, so it is more like a "get" than a "create".
      
      So:
       - Move the increment of nlmsvc_users into the function as well
       - rename to lockd_get().
      
      It is now the inverse of lockd_put().
      Signed-off-by: default avatarNeilBrown <neilb@suse.de>
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      ecd3ad68
    • NeilBrown's avatar
      lockd: introduce lockd_put() · 865b6740
      NeilBrown authored
      There is some cleanup that is duplicated in lockd_down() and the failure
      path of lockd_up().
      Factor these out into a new lockd_put() and call it from both places.
      
      lockd_put() does *not* take the mutex - that must be held by the caller.
      It decrements nlmsvc_users and if that reaches zero, it cleans up.
      Signed-off-by: default avatarNeilBrown <neilb@suse.de>
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      865b6740
    • NeilBrown's avatar
      lockd: move svc_exit_thread() into the thread · 6a4e2527
      NeilBrown authored
      The normal place to call svc_exit_thread() is from the thread itself
      just before it exists.
      Do this for lockd.
      
      This means that nlmsvc_rqst is not used out side of lockd_start_svc(),
      so it can be made local to that function, and renamed to 'rqst'.
      Signed-off-by: default avatarNeilBrown <neilb@suse.de>
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      6a4e2527
    • NeilBrown's avatar
      lockd: move lockd_start_svc() call into lockd_create_svc() · b73a2972
      NeilBrown authored
      lockd_start_svc() only needs to be called once, just after the svc is
      created.  If the start fails, the svc is discarded too.
      
      It thus makes sense to call lockd_start_svc() from lockd_create_svc().
      This allows us to remove the test against nlmsvc_rqst at the start of
      lockd_start_svc() - it must always be NULL.
      
      lockd_up() only held an extra reference on the svc until a thread was
      created - then it dropped it.  The thread - and thus the extra reference
      - will remain until kthread_stop() is called.
      Now that the thread is created in lockd_create_svc(), the extra
      reference can be dropped there.  So the 'serv' variable is no longer
      needed in lockd_up().
      Signed-off-by: default avatarNeilBrown <neilb@suse.de>
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      b73a2972
    • NeilBrown's avatar
      lockd: simplify management of network status notifiers · 5a8a7ff5
      NeilBrown authored
      Now that the network status notifiers use nlmsvc_serv rather then
      nlmsvc_rqst the management can be simplified.
      
      Notifier unregistration synchronises with any pending notifications so
      providing we unregister before nlm_serv is freed no further interlock
      is required.
      
      So we move the unregister call to just before the thread is killed
      (which destroys the service) and just before the service is destroyed in
      the failure-path of lockd_up().
      
      Then nlm_ntf_refcnt and nlm_ntf_wq can be removed.
      Signed-off-by: default avatarNeilBrown <neilb@suse.de>
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      5a8a7ff5
    • NeilBrown's avatar
      lockd: introduce nlmsvc_serv · 2840fe86
      NeilBrown authored
      lockd has two globals - nlmsvc_task and nlmsvc_rqst - but mostly it
      wants the 'struct svc_serv', and when it doesn't want it exactly it can
      get to what it wants from the serv.
      
      This patch is a first step to removing nlmsvc_task and nlmsvc_rqst.  It
      introduces nlmsvc_serv to store the 'struct svc_serv*'.  This is set as
      soon as the serv is created, and cleared only when it is destroyed.
      Signed-off-by: default avatarNeilBrown <neilb@suse.de>
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      2840fe86
    • NeilBrown's avatar
      NFSD: simplify locking for network notifier. · d057cfec
      NeilBrown authored
      nfsd currently maintains an open-coded read/write semaphore (refcount
      and wait queue) for each network namespace to ensure the nfs service
      isn't shut down while the notifier is running.
      
      This is excessive.  As there is unlikely to be contention between
      notifiers and they run without sleeping, a single spinlock is sufficient
      to avoid problems.
      Signed-off-by: default avatarNeilBrown <neilb@suse.de>
      [ cel: ensure nfsd_notifier_lock is static ]
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      d057cfec
    • NeilBrown's avatar
      SUNRPC: discard svo_setup and rename svc_set_num_threads_sync() · 3ebdbe52
      NeilBrown authored
      The ->svo_setup callback serves no purpose.  It is always called from
      within the same module that chooses which callback is needed.  So
      discard it and call the relevant function directly.
      
      Now that svc_set_num_threads() is no longer used remove it and rename
      svc_set_num_threads_sync() to remove the "_sync" suffix.
      Signed-off-by: default avatarNeilBrown <neilb@suse.de>
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      3ebdbe52
    • NeilBrown's avatar
      NFSD: Make it possible to use svc_set_num_threads_sync · 3409e4f1
      NeilBrown authored
      nfsd cannot currently use svc_set_num_threads_sync.  It instead
      uses svc_set_num_threads which does *not* wait for threads to all
      exit, and has a separate mechanism (nfsd_shutdown_complete) to wait
      for completion.
      
      The reason that nfsd is unlike other services is that nfsd threads can
      exit separately from svc_set_num_threads being called - they die on
      receipt of SIGKILL.  Also, when the last thread exits, the service must
      be shut down (sockets closed).
      
      For this, the nfsd_mutex needs to be taken, and as that mutex needs to
      be held while svc_set_num_threads is called, the one cannot wait for
      the other.
      
      This patch changes the nfsd thread so that it can drop the ref on the
      service without blocking on nfsd_mutex, so that svc_set_num_threads_sync
      can be used:
       - if it can drop a non-last reference, it does that.  This does not
         trigger shutdown and does not require a mutex.  This will likely
         happen for all but the last thread signalled, and for all threads
         being shut down by nfsd_shutdown_threads()
       - if it can get the mutex without blocking (trylock), it does that
         and then drops the reference.  This will likely happen for the
         last thread killed by SIGKILL
       - Otherwise there might be an unrelated task holding the mutex,
         possibly in another network namespace, or nfsd_shutdown_threads()
         might be just about to get a reference on the service, after which
         we can drop ours safely.
         We cannot conveniently get wakeup notifications on these events,
         and we are unlikely to need to, so we sleep briefly and check again.
      
      With this we can discard nfsd_shutdown_complete and
      nfsd_complete_shutdown(), and switch to svc_set_num_threads_sync.
      Signed-off-by: default avatarNeilBrown <neilb@suse.de>
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      3409e4f1
    • NeilBrown's avatar
      NFSD: narrow nfsd_mutex protection in nfsd thread · 9d3792ae
      NeilBrown authored
      There is nothing happening in the start of nfsd() that requires
      protection by the mutex, so don't take it until shutting down the thread
      - which does still require protection - but only for nfsd_put().
      Signed-off-by: default avatarNeilBrown <neilb@suse.de>
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      9d3792ae
    • NeilBrown's avatar
      SUNRPC: use sv_lock to protect updates to sv_nrthreads. · 2a36395f
      NeilBrown authored
      Using sv_lock means we don't need to hold the service mutex over these
      updates.
      
      In particular,  svc_exit_thread() no longer requires synchronisation, so
      threads can exit asynchronously.
      
      Note that we could use an atomic_t, but as there are many more read
      sites than writes, that would add unnecessary noise to the code.
      Some reads are already racy, and there is no need for them to not be.
      Signed-off-by: default avatarNeilBrown <neilb@suse.de>
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      2a36395f
    • NeilBrown's avatar
      nfsd: make nfsd_stats.th_cnt atomic_t · 9b6c8c9b
      NeilBrown authored
      This allows us to move the updates for th_cnt out of the mutex.
      This is a step towards reducing mutex coverage in nfsd().
      Signed-off-by: default avatarNeilBrown <neilb@suse.de>
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      9b6c8c9b
    • NeilBrown's avatar
      SUNRPC: stop using ->sv_nrthreads as a refcount · ec52361d
      NeilBrown authored
      The use of sv_nrthreads as a general refcount results in clumsy code, as
      is seen by various comments needed to explain the situation.
      
      This patch introduces a 'struct kref' and uses that for reference
      counting, leaving sv_nrthreads to be a pure count of threads.  The kref
      is managed particularly in svc_get() and svc_put(), and also nfsd_put();
      
      svc_destroy() now takes a pointer to the embedded kref, rather than to
      the serv.
      
      nfsd allows the svc_serv to exist with ->sv_nrhtreads being zero.  This
      happens when a transport is created before the first thread is started.
      To support this, a 'keep_active' flag is introduced which holds a ref on
      the svc_serv.  This is set when any listening socket is successfully
      added (unless there are running threads), and cleared when the number of
      threads is set.  So when the last thread exits, the nfs_serv will be
      destroyed.
      The use of 'keep_active' replaces previous code which checked if there
      were any permanent sockets.
      
      We no longer clear ->rq_server when nfsd() exits.  This was done
      to prevent svc_exit_thread() from calling svc_destroy().
      Instead we take an extra reference to the svc_serv to prevent
      svc_destroy() from being called.
      Signed-off-by: default avatarNeilBrown <neilb@suse.de>
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      ec52361d
    • NeilBrown's avatar
      SUNRPC/NFSD: clean up get/put functions. · 8c62d127
      NeilBrown authored
      svc_destroy() is poorly named - it doesn't necessarily destroy the svc,
      it might just reduce the ref count.
      nfsd_destroy() is poorly named for the same reason.
      
      This patch:
       - removes the refcount functionality from svc_destroy(), moving it to
         a new svc_put().  Almost all previous callers of svc_destroy() now
         call svc_put().
       - renames nfsd_destroy() to nfsd_put() and improves the code, using
         the new svc_destroy() rather than svc_put()
       - removes a few comments that explain the important for balanced
         get/put calls.  This should be obvious.
      
      The only non-trivial part of this is that svc_destroy() would call
      svc_sock_update() on a non-final decrement.  It can no longer do that,
      and svc_put() isn't really a good place of it.  This call is now made
      from svc_exit_thread() which seems like a good place.  This makes the
      call *before* sv_nrthreads is decremented rather than after.  This
      is not particularly important as the call just sets a flag which
      causes sv_nrthreads set be checked later.  A subsequent patch will
      improve the ordering.
      Signed-off-by: default avatarNeilBrown <neilb@suse.de>
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      8c62d127
    • NeilBrown's avatar
      SUNRPC: change svc_get() to return the svc. · df5e49c8
      NeilBrown authored
      It is common for 'get' functions to return the object that was 'got',
      and there are a couple of places where users of svc_get() would be a
      little simpler if svc_get() did that.
      
      Make it so.
      Signed-off-by: default avatarNeilBrown <neilb@suse.de>
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      df5e49c8
    • NeilBrown's avatar
      NFSD: handle errors better in write_ports_addfd() · 89b24336
      NeilBrown authored
      If write_ports_add() fails, we shouldn't destroy the serv, unless we had
      only just created it.  So if there are any permanent sockets already
      attached, leave the serv in place.
      Signed-off-by: default avatarNeilBrown <neilb@suse.de>
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      89b24336
    • Chuck Lever's avatar
      NFSD: Fix sparse warning · c2f1c4bd
      Chuck Lever authored
      /home/cel/src/linux/linux/fs/nfsd/nfs4proc.c:1539:24: warning: incorrect type in assignment (different base types)
      /home/cel/src/linux/linux/fs/nfsd/nfs4proc.c:1539:24:    expected restricted __be32 [usertype] status
      /home/cel/src/linux/linux/fs/nfsd/nfs4proc.c:1539:24:    got int
      Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
      c2f1c4bd
  3. 12 Dec, 2021 14 commits