Hi Umang,
Am 27.03.24 um 21:00 schrieb Umang Jain:
Hi Stefan
On 27/03/24 11:49 pm, Stefan Wahren wrote:
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.
Can you try building with ARCH=64-bit and testing it please? Please
refer to the cover section where I have tested 64-bit on rpi4.
sorry, i don't understand the point behind this. As i mention it's clear
that we cannot call kthread_stop() from vchiq_shutdown_internal because
this function is also used by different drivers. The calls to create the
kthreads should be symmetric to the stop calls. From my understanding
the kthreads are created from vchiq_probe(), so the kthreads should
stopped directly or indirectly from vchiq_remove().
This has nothing to do with Raspberry Pi 3 vs 4 or 32 bit vs 64 bit. It
was just luck that it worked in your case.
Regarding 32-bit build for my Pi4 - everything gets broken for me RPi4
(if I try to test via SD-card) ... and I am also having problems with
setting up a NFS-mounted 32-bit rootfs today so can't boot the pi
through NFS with 32-bit kernel :(
My Pi3 power cable/microUSB went missing, so I have ordered a new one.
It will take a couple of days to arrive before I can test it there.
}
int
