linux
linux copied to clipboard
raspberrypi
vc_sm_cma: BUG: sleeping function called from invalid context
Describe the bug
vc_sm.c:get_kernel_id() calls idr_alloc() with GFP_KERNEL under a spinlock.
(Also, does lookup_kernel_id() need to take the lock as well?)
Steps to reproduce the behaviour
Start capturing from a camera with e.g. cam (from libcamera).
Device (s)
Raspberry Pi 4 Mod. B
System
Linux <hostname> 6.15.0-rc3-v8+ #2 SMP PREEMPT Thu Apr 24 18:47:39 CEST 2025 aarch64 GNU/Linux
Logs
# cam -c1 -C32
[0:00:34.572735259] [311] INFO Camera camera_manager.cpp:326 libcamera v0.5.0+155-94e7d330-dirty (2025-04-25)
[0:00:34.780727131] [312] WARN CameraSensorProperties camera_sensor_properties.cpp:463 No static properties available for 'imx708_wide'
[0:00:34.780877790] [312] WARN CameraSensorProperties camera_sensor_properties.cpp:465 Please consider updating the camera sensor properties database
[0:00:35.064659868] [312] WARN RPiSdn sdn.cpp:40 Using legacy SDN tuning - please consider moving SDN inside rpi.denoise
[0:00:35.067119191] [312] WARN CameraSensor camera_sensor_legacy.cpp:501 'imx708_wide': No sensor delays found in static properties. Assuming unverified defaults.
[0:00:35.070015603] [312] INFO RPI vc4.cpp:407 Registered camera /base/soc/i2c0mux/i2c@1/imx708@1a to Unicam device /dev/media4 and ISP device /dev/media0
Using camera /base/soc/i2c0mux/i2c@1/imx708@1a as cam0
[0:00:35.071686595] [311] INFO Camera camera.cpp:1258 configuring streams: (0) 800x600-XRGB8888
[0:00:35.074298429] [312] INFO RPI vc4.cpp:576 Sensor: /base/soc/i2c0mux/i2c@1/imx708@1a - Selected sensor format: 1536x864-SBGGR10_1X10 - Selected unicam format: 1536x864-pBAA
[ 35.406296] BUG: sleeping function called from invalid context at ./include/linux/sched/mm.h:321
[ 35.406348] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 312, name: cam
[ 35.406388] preempt_count: 1, expected: 0
[ 35.406409] RCU nest depth: 0, expected: 0
cam0: Capture 32 frames
[ 35.406614] 2 locks held by cam/312:
[ 35.406625] #0: ffff0cdc08021858 (&node->queue_lock){+.+.}-{4:4}, at: __video_do_ioctl+0xf8/0x4a0 [videodev]
[ 35.406788] #1: ffff0cdc0654b8f8 (&sm_state->kernelid_map_lock){+.+.}-{3:3}, at: vc_sm_cma_import_dmabuf_internal+0x248/0x448 [vc_sm_cma]
[ 35.406850] CPU: 0 UID: 0 PID: 312 Comm: cam Tainted: G C 6.15.0-rc3-v8+ #2 PREEMPT
[ 35.406867] Tainted: [C]=CRAP
[ 35.406872] Hardware name: Raspberry Pi 4 Model B Rev 1.5 (DT)
[ 35.406879] Call trace:
[ 35.406884] show_stack+0x20/0x40 (C)
[ 35.406907] dump_stack_lvl+0x70/0x98
[ 35.406923] dump_stack+0x18/0x24
[ 35.406935] __might_resched+0x164/0x240
[ 35.406950] __might_sleep+0x50/0x90
[ 35.406963] kmem_cache_alloc_noprof+0x21c/0x450
[ 35.406979] radix_tree_node_alloc.constprop.0+0x54/0xf0
[ 35.406997] idr_get_free+0x224/0x2e0
[ 35.407012] idr_alloc_u32+0x78/0x108
[ 35.407029] idr_alloc+0x44/0x90
[ 35.407043] vc_sm_cma_import_dmabuf_internal+0x264/0x448 [vc_sm_cma]
[ 35.407063] vc_sm_cma_import_dmabuf+0x60/0xf8 [vc_sm_cma]
[ 35.407081] vchiq_mmal_submit_buffer+0xc4/0x190 [bcm2835_mmal_vchiq]
[ 35.407105] bcm2835_isp_node_buffer_queue+0xa4/0x130 [bcm2835_isp]
[ 35.407123] __enqueue_in_driver+0x78/0x160 [videobuf2_common]
[ 35.407160] vb2_start_streaming+0x3c/0x190 [videobuf2_common]
[ 35.407191] vb2_core_streamon+0xe0/0x1d0 [videobuf2_common]
[ 35.407221] vb2_ioctl_streamon+0x60/0xb0 [videobuf2_v4l2]
[ 35.407249] v4l_streamon+0x2c/0x40 [videodev]
[ 35.407340] __video_do_ioctl+0x410/0x4a0 [videodev]
[ 35.407426] video_usercopy+0x2f4/0x890 [videodev]
[ 35.407511] video_ioctl2+0x20/0x50 [videodev]
[ 35.407596] v4l2_ioctl+0x48/0x78 [videodev]
[ 35.407680] __arm64_sys_ioctl+0xbc/0x100
[ 35.407695] invoke_syscall+0x50/0x120
[ 35.407707] el0_svc_common.constprop.0+0xc8/0xf0
[ 35.407719] do_el0_svc+0x24/0x38
[ 35.407729] el0_svc+0x48/0x110
[ 35.407743] el0t_64_sync_handler+0x10c/0x140
[ 35.407757] el0t_64_sync+0x198/0x1a0
Additional context
The following seems to work so far:
diff --git a/drivers/staging/vc04_services/vc-sm-cma/vc_sm.c b/drivers/staging/vc04_services/vc-sm-cma/vc_sm.c
index 4ebe909d7d97..8609d6ec7657 100644
--- a/drivers/staging/vc04_services/vc-sm-cma/vc_sm.c
+++ b/drivers/staging/vc04_services/vc-sm-cma/vc_sm.c
@@ -91,7 +91,7 @@ struct sm_state_t {
struct sm_instance *sm_handle; /* Handle for videocore service. */
- spinlock_t kernelid_map_lock; /* Spinlock protecting kernelid_map */
+ struct mutex kernelid_map_lock; /* protecting kernelid_map */
struct idr kernelid_map;
struct mutex map_lock; /* Global map lock. */
@@ -127,25 +127,20 @@ static int sm_inited;
static int get_kernel_id(struct vc_sm_buffer *buffer)
{
- int handle;
-
- spin_lock(&sm_state->kernelid_map_lock);
- handle = idr_alloc(&sm_state->kernelid_map, buffer, 0, 0, GFP_KERNEL);
- spin_unlock(&sm_state->kernelid_map_lock);
-
- return handle;
+ guard(mutex)(&sm_state->kernelid_map_lock);
+ return idr_alloc(&sm_state->kernelid_map, buffer, 0, 0, GFP_KERNEL);
}
static struct vc_sm_buffer *lookup_kernel_id(int handle)
{
+ guard(mutex)(&sm_state->kernelid_map_lock);
return idr_find(&sm_state->kernelid_map, handle);
}
static void free_kernel_id(int handle)
{
- spin_lock(&sm_state->kernelid_map_lock);
+ guard(mutex)(&sm_state->kernelid_map_lock);
idr_remove(&sm_state->kernelid_map, handle);
- spin_unlock(&sm_state->kernelid_map_lock);
}
static int vc_sm_cma_seq_file_show(struct seq_file *s, void *v)
@@ -1494,7 +1489,7 @@ static int bcm2835_vc_sm_cma_probe(struct vchiq_device *device)
sm_state->device = device;
mutex_init(&sm_state->map_lock);
- spin_lock_init(&sm_state->kernelid_map_lock);
+ mutex_init(&sm_state->kernelid_map_lock);
idr_init_base(&sm_state->kernelid_map, 1);
device->dev.dma_parms = devm_kzalloc(&device->dev,
Possibly related: https://github.com/raspberrypi/linux/pull/6703/commits/bd5f8b97b2caa1ec35d3cabb5d618ee5dd8c4e22 (part of https://github.com/raspberrypi/linux/issues/6701#issuecomment-2701809352)
Again sorry for not replying.
My concern with switching from spinlocks to a mutex was the context that we get called with for VPU callbacks through VCHI.
Looking at it, all callbacks handled within vc_sm_vpu_event are called within a thread context of `vc_sm_cma_vchi_videocore_io, and that is using mutexes.
So there is no issue with downgrading to a mutex, and that means we won't sleep in an invalid context.
