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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Umang,

Am 21.03.24 um 14:07 schrieb Umang Jain:
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.

Signed-off-by: Umang Jain <umang.jain@xxxxxxxxxxxxxxxx>
Reviewed-by: Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx>
---
  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 ad506016fc93..1d21f9cfbfe5 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);

@@ -1310,7 +1314,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);

i had a little bit time to look at the corrupt issue, i think this
caused by this change. This function is called by vchiq_shutdown which
is exported and used by VCHIQ user drivers like bcm2835-audio or
mmal-vchiq. So this is absolutely not the right place to stop the kernel
threads and the issue is not specific to the Raspberry Pi 3.

  }

  int





[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux