Re: [PATCH 5/5] staging: vc04_services: vchiq_core: Stop kthreads on shutdown

[Kieran Bingham <kieran.bingham@ideasonboard.com>]

Quoting Umang Jain (2024-03-15 10:56:59)
> The various kthreads thread functions (slot_handler_func, sync_func,
> recycle_func) in vchiq_core and vchiq_keepalive_thread_func in
> vchiq_arm should be stopped on vchiq_shutdown().
> 
> This also address the following TODO item:
>  * Fix kernel module support
> 
> hence drop it from the TODO item list.


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  drivers/staging/vc04_services/interface/TODO           |  7 -------
>  .../vc04_services/interface/vchiq_arm/vchiq_arm.c      |  8 ++++++--
>  .../vc04_services/interface/vchiq_arm/vchiq_core.c     | 10 +++++++---
>  3 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/vc04_services/interface/TODO b/drivers/staging/vc04_services/interface/TODO
> index 05eb5140d096..57a2d6761f9b 100644
> --- a/drivers/staging/vc04_services/interface/TODO
> +++ b/drivers/staging/vc04_services/interface/TODO
> @@ -16,13 +16,6 @@ some of the ones we want:
>    to manage these buffers as dmabufs so that we can zero-copy import
>    camera images into vc4 for rendering/display.
>  
> -* Fix kernel module support
> -
> -Even the VPU firmware doesn't support a VCHI re-connect, the driver
> -should properly handle a module unload. This also includes that all
> -resources must be freed (kthreads, debugfs entries, ...) and global
> -variables avoided.
> -
>  * Documentation
>  
>  A short top-down description of this driver's architecture (function of
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index 1151cb4a84f9..60b4874049b4 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -726,8 +726,9 @@ void free_bulk_waiter(struct vchiq_instance *instance)
>  
>  int vchiq_shutdown(struct vchiq_instance *instance)
>  {
> -       int status = 0;
>         struct vchiq_state *state = instance->state;
> +       struct vchiq_arm_state *arm_state;
> +       int status = 0;
>  
>         if (mutex_lock_killable(&state->mutex))
>                 return -EAGAIN;
> @@ -739,6 +740,9 @@ int vchiq_shutdown(struct vchiq_instance *instance)
>  
>         dev_dbg(state->dev, "core: (%p): returning %d\n", instance, status);
>  
> +       arm_state = vchiq_platform_get_arm_state(state);
> +       kthread_stop(arm_state->ka_thread);
> +
>         free_bulk_waiter(instance);
>         kfree(instance);
>  
> @@ -1311,7 +1315,7 @@ vchiq_keepalive_thread_func(void *v)
>                 goto shutdown;
>         }
>  
> -       while (1) {
> +       while (!kthread_should_stop()) {
>                 long rc = 0, uc = 0;
>  
>                 if (wait_for_completion_interruptible(&arm_state->ka_evt)) {
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> index 76c27778154a..953ccd87f425 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> @@ -1936,7 +1936,7 @@ slot_handler_func(void *v)
>  
>         DEBUG_INITIALISE(local);
>  
> -       while (1) {
> +       while (!kthread_should_stop()) {
>                 DEBUG_COUNT(SLOT_HANDLER_COUNT);
>                 DEBUG_TRACE(SLOT_HANDLER_LINE);
>                 remote_event_wait(&state->trigger_event, &local->trigger);
> @@ -1978,7 +1978,7 @@ recycle_func(void *v)
>         if (!found)
>                 return -ENOMEM;
>  
> -       while (1) {
> +       while (!kthread_should_stop()) {
>                 remote_event_wait(&state->recycle_event, &local->recycle);
>  
>                 process_free_queue(state, found, length);
> @@ -1997,7 +1997,7 @@ sync_func(void *v)
>                         state->remote->slot_sync);
>         int svc_fourcc;
>  
> -       while (1) {
> +       while (!kthread_should_stop()) {
>                 struct vchiq_service *service;
>                 int msgid, size;
>                 int type;
> @@ -2844,6 +2844,10 @@ vchiq_shutdown_internal(struct vchiq_state *state, struct vchiq_instance *instan
>                 (void)vchiq_remove_service(instance, service->handle);
>                 vchiq_service_put(service);
>         }
> +
> +       kthread_stop(state->sync_thread);
> +       kthread_stop(state->recycle_thread);
> +       kthread_stop(state->slot_handler_thread);
>  }
>  
>  int
> -- 
> 2.43.0
>