Le 20/09/2023 à 16:56, Hans Verkuil a écrit :

On 20/09/2023 16:30, Benjamin Gaignard wrote:
<snip>

quoted

quoted

quoted

       num_buffers = min_t(unsigned int, num_buffers,
                   q->max_allowed_buffers - vb2_get_num_buffers(q));
   -    first_index = vb2_get_num_buffers(q);
+    first_index = bitmap_find_next_zero_area(q->bufs_map, q->max_allowed_buffers,
+                         0, num_buffers, 0);
         if (first_index >= q->max_allowed_buffers)
           return 0;

@@ -675,7 +678,13 @@ static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
     struct vb2_buffer *vb2_get_buffer(struct vb2_queue *q, unsigned int index)
   {
-    if (index < q->num_buffers)
+    if (!q->bufs_map || !q->bufs)
+        return NULL;

I don't think this can ever happen.

I got kernel crash without them.
I will keep them.

What is the backtrace? How can this happen? It feels wrong that this can be
called with a vb2_queue that apparently is not properly initialized.

I will add backtrace when doing test on v8

quoted

quoted

quoted

+
+    return (bitmap_weight(q->bufs_map, q->max_allowed_buffers) > 0);

How about:

     return vb2_get_num_buffers(q) > 0;

vb2_get_num_buffers is defined in videobuf2-core.c, I'm not sure that
an inline function could depend of a module function.

Not a problem. E.g. v4l2-ctrls.h is full of such static inlines.

I will change vb2_get_num_buffers() to inline function that solve the problem too.

Regards,

	Hans

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel