diff mbox series

sunrpc: document locking rules for svc_exit_thread()

Message ID 172196639503.18529.16126598330625338469@noble.neil.brown.name (mailing list archive)
State New
Headers show
Series sunrpc: document locking rules for svc_exit_thread() | expand

Commit Message

NeilBrown July 26, 2024, 3:59 a.m. UTC
The locking required for svc_exit_thread() is not obvious, so document
it in a kdoc comment.

Signed-off-by: NeilBrown <neilb@suse.de>
---

Rather than repost patch 5/14 of my recent set for dynamic thread
management, I decided to provide this as an independent patch which
could usefully be inserted into the series before 5/14.  That will make
that patch easier to understand/review.

NeilBrown


 net/sunrpc/svc.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Jeff Layton July 26, 2024, 12:51 p.m. UTC | #1
On Fri, 2024-07-26 at 13:59 +1000, NeilBrown wrote:
> 
> The locking required for svc_exit_thread() is not obvious, so document
> it in a kdoc comment.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
> 
> Rather than repost patch 5/14 of my recent set for dynamic thread
> management, I decided to provide this as an independent patch which
> could usefully be inserted into the series before 5/14.  That will make
> that patch easier to understand/review.
> 
> NeilBrown
> 
> 
>  net/sunrpc/svc.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index 26b6e73fc0de..4c3df893c532 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -955,6 +955,20 @@ void svc_rqst_release_pages(struct svc_rqst *rqstp)
>  	}
>  }
>  
> +/**
> + * svc_exit_thread - finalise the termination of a sunrpc server thread
> + * @rqstp: the svc_rqst which represents the thread.
> + *
> + * When a thread started with svc_new_thread() exits it must call
> + * svc_exit_thread() as its last act.  This must be done with the
> + * service mutex held.  Normally this is held by a DIFFERENT thread, the
> + * one that is calling svc_set_num_threads() and which will wait for
> + * SP_VICTIM_REMAINS to be cleared before dropping the mutex.  If the
> + * thread exits for any reason other than svc_thread_should_stop()
> + * returning %true (which indicated that svc_set_num_threads() is
> + * waiting for it to exit), then it must take the service mutex itself,
> + * which can only safely be done using mutex_try_lock().
> + */
>  void
>  svc_exit_thread(struct svc_rqst *rqstp)
>  {

Nice to have this clearly stated.

Reviewed-by: Jeff Layton <jlayton@kernel.org>
diff mbox series

Patch

diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 26b6e73fc0de..4c3df893c532 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -955,6 +955,20 @@  void svc_rqst_release_pages(struct svc_rqst *rqstp)
 	}
 }
 
+/**
+ * svc_exit_thread - finalise the termination of a sunrpc server thread
+ * @rqstp: the svc_rqst which represents the thread.
+ *
+ * When a thread started with svc_new_thread() exits it must call
+ * svc_exit_thread() as its last act.  This must be done with the
+ * service mutex held.  Normally this is held by a DIFFERENT thread, the
+ * one that is calling svc_set_num_threads() and which will wait for
+ * SP_VICTIM_REMAINS to be cleared before dropping the mutex.  If the
+ * thread exits for any reason other than svc_thread_should_stop()
+ * returning %true (which indicated that svc_set_num_threads() is
+ * waiting for it to exit), then it must take the service mutex itself,
+ * which can only safely be done using mutex_try_lock().
+ */
 void
 svc_exit_thread(struct svc_rqst *rqstp)
 {