Instead of a static array change bufs to a dynamically allocated array.
This will allow to store more video buffer if needed.
Protect the array with a spinlock.
Signed-off-by: Benjamin Gaignard <[email protected]>
---
.../media/common/videobuf2/videobuf2-core.c | 8 +++
include/media/videobuf2-core.h | 49 ++++++++++++++++---
2 files changed, 49 insertions(+), 8 deletions(-)
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 79e90e338846..ae9d72f4d181 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -2452,6 +2452,13 @@ int vb2_core_queue_init(struct vb2_queue *q)
mutex_init(&q->mmap_lock);
init_waitqueue_head(&q->done_wq);
+ q->max_num_bufs = 32;
+ q->bufs = kmalloc_array(q->max_num_bufs, sizeof(*q->bufs), GFP_KERNEL | __GFP_ZERO);
+ if (!q->bufs)
+ return -ENOMEM;
+
+ spin_lock_init(&q->bufs_lock);
+
q->memory = VB2_MEMORY_UNKNOWN;
if (q->buf_struct_size == 0)
@@ -2479,6 +2486,7 @@ void vb2_core_queue_release(struct vb2_queue *q)
mutex_lock(&q->mmap_lock);
__vb2_queue_free(q, q->num_buffers);
mutex_unlock(&q->mmap_lock);
+ kfree(q->bufs);
}
EXPORT_SYMBOL_GPL(vb2_core_queue_release);
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 5b1e3d801546..397dbf6e61e1 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -558,6 +558,8 @@ struct vb2_buf_ops {
* @dma_dir: DMA mapping direction.
* @bufs: videobuf2 buffer structures
* @num_buffers: number of allocated/used buffers
+ * @bufs_lock: lock to protect bufs access
+ * @max_num_bufs: max number of buffers storable in bufs
* @queued_list: list of buffers currently queued from userspace
* @queued_count: number of buffers queued and ready for streaming.
* @owned_by_drv_count: number of buffers owned by the driver
@@ -619,8 +621,10 @@ struct vb2_queue {
struct mutex mmap_lock;
unsigned int memory;
enum dma_data_direction dma_dir;
- struct vb2_buffer *bufs[VB2_MAX_FRAME];
+ struct vb2_buffer **bufs;
unsigned int num_buffers;
+ spinlock_t bufs_lock;
+ size_t max_num_bufs;
struct list_head queued_list;
unsigned int queued_count;
@@ -1239,9 +1243,16 @@ static inline void vb2_clear_last_buffer_dequeued(struct vb2_queue *q)
static inline struct vb2_buffer *vb2_get_buffer(struct vb2_queue *q,
unsigned int index)
{
- if (index < q->num_buffers)
- return q->bufs[index];
- return NULL;
+ struct vb2_buffer *vb = NULL;
+
+ spin_lock(&q->bufs_lock);
+
+ if (index < q->max_num_bufs)
+ vb = q->bufs[index];
+
+ spin_unlock(&q->bufs_lock);
+
+ return vb;
}
/**
@@ -1251,12 +1262,30 @@ static inline struct vb2_buffer *vb2_get_buffer(struct vb2_queue *q,
*/
static inline bool vb2_queue_add_buffer(struct vb2_queue *q, struct vb2_buffer *vb)
{
- if (vb->index < VB2_MAX_FRAME) {
+ bool ret = false;
+
+ spin_lock(&q->bufs_lock);
+
+ if (vb->index >= q->max_num_bufs) {
+ struct vb2_buffer **tmp;
+
+ tmp = krealloc_array(q->bufs, q->max_num_bufs * 2, sizeof(*q->bufs), GFP_KERNEL);
+ if (!tmp)
+ goto realloc_failed;
+
+ q->max_num_bufs *= 2;
+ q->bufs = tmp;
+ }
+
+ if (vb->index < q->max_num_bufs) {
q->bufs[vb->index] = vb;
- return true;
+ ret = true;
}
- return false;
+realloc_failed:
+ spin_unlock(&q->bufs_lock);
+
+ return ret;
}
/**
@@ -1266,8 +1295,12 @@ static inline bool vb2_queue_add_buffer(struct vb2_queue *q, struct vb2_buffer *
*/
static inline void vb2_queue_remove_buffer(struct vb2_queue *q, struct vb2_buffer *vb)
{
- if (vb->index < VB2_MAX_FRAME)
+ spin_lock(&q->bufs_lock);
+
+ if (vb->index < q->max_num_bufs)
q->bufs[vb->index] = NULL;
+
+ spin_unlock(&q->bufs_lock);
}
/*
--
2.34.1
Hi Benjamin,
Thank you for the patch.
On Tue, Mar 21, 2023 at 11:28:49AM +0100, Benjamin Gaignard wrote:
> Instead of a static array change bufs to a dynamically allocated array.
> This will allow to store more video buffer if needed.
> Protect the array with a spinlock.
I think I asked in the review of v1 why we couldn't use the kernel
IDA/IDR APIs to allocate buffer IDs and register buffers, but I don't
think I've received a reply. Wouldn't it be better than rolling out our
own mechanism ?
> Signed-off-by: Benjamin Gaignard <[email protected]>
> ---
> .../media/common/videobuf2/videobuf2-core.c | 8 +++
> include/media/videobuf2-core.h | 49 ++++++++++++++++---
> 2 files changed, 49 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> index 79e90e338846..ae9d72f4d181 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -2452,6 +2452,13 @@ int vb2_core_queue_init(struct vb2_queue *q)
> mutex_init(&q->mmap_lock);
> init_waitqueue_head(&q->done_wq);
>
> + q->max_num_bufs = 32;
> + q->bufs = kmalloc_array(q->max_num_bufs, sizeof(*q->bufs), GFP_KERNEL | __GFP_ZERO);
> + if (!q->bufs)
> + return -ENOMEM;
> +
> + spin_lock_init(&q->bufs_lock);
> +
> q->memory = VB2_MEMORY_UNKNOWN;
>
> if (q->buf_struct_size == 0)
> @@ -2479,6 +2486,7 @@ void vb2_core_queue_release(struct vb2_queue *q)
> mutex_lock(&q->mmap_lock);
> __vb2_queue_free(q, q->num_buffers);
> mutex_unlock(&q->mmap_lock);
> + kfree(q->bufs);
> }
> EXPORT_SYMBOL_GPL(vb2_core_queue_release);
>
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 5b1e3d801546..397dbf6e61e1 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -558,6 +558,8 @@ struct vb2_buf_ops {
> * @dma_dir: DMA mapping direction.
> * @bufs: videobuf2 buffer structures
> * @num_buffers: number of allocated/used buffers
> + * @bufs_lock: lock to protect bufs access
> + * @max_num_bufs: max number of buffers storable in bufs
> * @queued_list: list of buffers currently queued from userspace
> * @queued_count: number of buffers queued and ready for streaming.
> * @owned_by_drv_count: number of buffers owned by the driver
> @@ -619,8 +621,10 @@ struct vb2_queue {
> struct mutex mmap_lock;
> unsigned int memory;
> enum dma_data_direction dma_dir;
> - struct vb2_buffer *bufs[VB2_MAX_FRAME];
> + struct vb2_buffer **bufs;
> unsigned int num_buffers;
> + spinlock_t bufs_lock;
> + size_t max_num_bufs;
>
> struct list_head queued_list;
> unsigned int queued_count;
> @@ -1239,9 +1243,16 @@ static inline void vb2_clear_last_buffer_dequeued(struct vb2_queue *q)
> static inline struct vb2_buffer *vb2_get_buffer(struct vb2_queue *q,
> unsigned int index)
> {
> - if (index < q->num_buffers)
> - return q->bufs[index];
> - return NULL;
> + struct vb2_buffer *vb = NULL;
> +
> + spin_lock(&q->bufs_lock);
> +
> + if (index < q->max_num_bufs)
> + vb = q->bufs[index];
> +
> + spin_unlock(&q->bufs_lock);
> +
> + return vb;
> }
>
> /**
> @@ -1251,12 +1262,30 @@ static inline struct vb2_buffer *vb2_get_buffer(struct vb2_queue *q,
> */
> static inline bool vb2_queue_add_buffer(struct vb2_queue *q, struct vb2_buffer *vb)
> {
> - if (vb->index < VB2_MAX_FRAME) {
> + bool ret = false;
> +
> + spin_lock(&q->bufs_lock);
> +
> + if (vb->index >= q->max_num_bufs) {
> + struct vb2_buffer **tmp;
> +
> + tmp = krealloc_array(q->bufs, q->max_num_bufs * 2, sizeof(*q->bufs), GFP_KERNEL);
> + if (!tmp)
> + goto realloc_failed;
> +
> + q->max_num_bufs *= 2;
> + q->bufs = tmp;
> + }
> +
> + if (vb->index < q->max_num_bufs) {
> q->bufs[vb->index] = vb;
> - return true;
> + ret = true;
> }
>
> - return false;
> +realloc_failed:
> + spin_unlock(&q->bufs_lock);
> +
> + return ret;
> }
>
> /**
> @@ -1266,8 +1295,12 @@ static inline bool vb2_queue_add_buffer(struct vb2_queue *q, struct vb2_buffer *
> */
> static inline void vb2_queue_remove_buffer(struct vb2_queue *q, struct vb2_buffer *vb)
> {
> - if (vb->index < VB2_MAX_FRAME)
> + spin_lock(&q->bufs_lock);
> +
> + if (vb->index < q->max_num_bufs)
> q->bufs[vb->index] = NULL;
> +
> + spin_unlock(&q->bufs_lock);
> }
>
> /*
--
Regards,
Laurent Pinchart
Hi Benjamin,
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Benjamin-Gaignard/media-videobuf2-Access-vb2_queue-bufs-array-through-helper-functions/20230321-183154
base: git://linuxtv.org/media_tree.git master
patch link: https://lore.kernel.org/r/20230321102855.346732-3-benjamin.gaignard%40collabora.com
patch subject: [PATCH v2 2/8] media: videobuf2: Make bufs array dynamic allocated
config: arm64-randconfig-m041-20230319 (https://download.01.org/0day-ci/archive/20230324/[email protected]/config)
compiler: aarch64-linux-gcc (GCC) 12.1.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Link: https://lore.kernel.org/r/[email protected]/
smatch warnings:
include/media/videobuf2-core.h:1272 vb2_queue_add_buffer() warn: sleeping in atomic context
drivers/media/common/videobuf2/videobuf2-core.c:2456 vb2_core_queue_init() warn: Please consider using kcalloc instead of kmalloc_array
vim +1272 include/media/videobuf2-core.h
625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1263 static inline bool vb2_queue_add_buffer(struct vb2_queue *q, struct vb2_buffer *vb)
625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1264 {
487d3f14d12ecf Benjamin Gaignard 2023-03-21 1265 bool ret = false;
487d3f14d12ecf Benjamin Gaignard 2023-03-21 1266
487d3f14d12ecf Benjamin Gaignard 2023-03-21 1267 spin_lock(&q->bufs_lock);
^^^^^^^^^^^^^^^^^^^^^^^
Holding a spin lock.
487d3f14d12ecf Benjamin Gaignard 2023-03-21 1268
487d3f14d12ecf Benjamin Gaignard 2023-03-21 1269 if (vb->index >= q->max_num_bufs) {
487d3f14d12ecf Benjamin Gaignard 2023-03-21 1270 struct vb2_buffer **tmp;
487d3f14d12ecf Benjamin Gaignard 2023-03-21 1271
487d3f14d12ecf Benjamin Gaignard 2023-03-21 @1272 tmp = krealloc_array(q->bufs, q->max_num_bufs * 2, sizeof(*q->bufs), GFP_KERNEL);
^^^^^^^^^^
Sleeping allocation. GFP_ATOMIC? Or is there a way to move the
allocation outside the lock?
487d3f14d12ecf Benjamin Gaignard 2023-03-21 1273 if (!tmp)
487d3f14d12ecf Benjamin Gaignard 2023-03-21 1274 goto realloc_failed;
487d3f14d12ecf Benjamin Gaignard 2023-03-21 1275
487d3f14d12ecf Benjamin Gaignard 2023-03-21 1276 q->max_num_bufs *= 2;
487d3f14d12ecf Benjamin Gaignard 2023-03-21 1277 q->bufs = tmp;
487d3f14d12ecf Benjamin Gaignard 2023-03-21 1278 }
487d3f14d12ecf Benjamin Gaignard 2023-03-21 1279
487d3f14d12ecf Benjamin Gaignard 2023-03-21 1280 if (vb->index < q->max_num_bufs) {
625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1281 q->bufs[vb->index] = vb;
487d3f14d12ecf Benjamin Gaignard 2023-03-21 1282 ret = true;
625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1283 }
625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1284
487d3f14d12ecf Benjamin Gaignard 2023-03-21 1285 realloc_failed:
487d3f14d12ecf Benjamin Gaignard 2023-03-21 1286 spin_unlock(&q->bufs_lock);
487d3f14d12ecf Benjamin Gaignard 2023-03-21 1287
487d3f14d12ecf Benjamin Gaignard 2023-03-21 1288 return ret;
625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1289 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
Le 24/03/2023 à 06:01, Dan Carpenter a écrit :
> Hi Benjamin,
>
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Benjamin-Gaignard/media-videobuf2-Access-vb2_queue-bufs-array-through-helper-functions/20230321-183154
> base: git://linuxtv.org/media_tree.git master
> patch link: https://lore.kernel.org/r/20230321102855.346732-3-benjamin.gaignard%40collabora.com
> patch subject: [PATCH v2 2/8] media: videobuf2: Make bufs array dynamic allocated
> config: arm64-randconfig-m041-20230319 (https://download.01.org/0day-ci/archive/20230324/[email protected]/config)
> compiler: aarch64-linux-gcc (GCC) 12.1.0
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <[email protected]>
> | Reported-by: Dan Carpenter <[email protected]>
> | Link: https://lore.kernel.org/r/[email protected]/
>
> smatch warnings:
> include/media/videobuf2-core.h:1272 vb2_queue_add_buffer() warn: sleeping in atomic context
> drivers/media/common/videobuf2/videobuf2-core.c:2456 vb2_core_queue_init() warn: Please consider using kcalloc instead of kmalloc_array
>
> vim +1272 include/media/videobuf2-core.h
>
> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1263 static inline bool vb2_queue_add_buffer(struct vb2_queue *q, struct vb2_buffer *vb)
> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1264 {
> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1265 bool ret = false;
> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1266
> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1267 spin_lock(&q->bufs_lock);
> ^^^^^^^^^^^^^^^^^^^^^^^
> Holding a spin lock.
>
> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1268
> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1269 if (vb->index >= q->max_num_bufs) {
> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1270 struct vb2_buffer **tmp;
> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1271
> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 @1272 tmp = krealloc_array(q->bufs, q->max_num_bufs * 2, sizeof(*q->bufs), GFP_KERNEL);
> ^^^^^^^^^^
> Sleeping allocation. GFP_ATOMIC? Or is there a way to move the
> allocation outside the lock?
I will add GFP_ATOMIC flag in next version.
Thanks for your help,
Benjamin
>
> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1273 if (!tmp)
> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1274 goto realloc_failed;
> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1275
> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1276 q->max_num_bufs *= 2;
> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1277 q->bufs = tmp;
> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1278 }
> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1279
> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1280 if (vb->index < q->max_num_bufs) {
> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1281 q->bufs[vb->index] = vb;
> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1282 ret = true;
> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1283 }
> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1284
> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1285 realloc_failed:
> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1286 spin_unlock(&q->bufs_lock);
> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1287
> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1288 return ret;
> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1289 }
>
On 24/03/2023 09:11, Benjamin Gaignard wrote:
>
> Le 24/03/2023 à 06:01, Dan Carpenter a écrit :
>> Hi Benjamin,
>>
>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>
>> url: https://github.com/intel-lab-lkp/linux/commits/Benjamin-Gaignard/media-videobuf2-Access-vb2_queue-bufs-array-through-helper-functions/20230321-183154
>> base: git://linuxtv.org/media_tree.git master
>> patch link: https://lore.kernel.org/r/20230321102855.346732-3-benjamin.gaignard%40collabora.com
>> patch subject: [PATCH v2 2/8] media: videobuf2: Make bufs array dynamic allocated
>> config: arm64-randconfig-m041-20230319 (https://download.01.org/0day-ci/archive/20230324/[email protected]/config)
>> compiler: aarch64-linux-gcc (GCC) 12.1.0
>>
>> If you fix the issue, kindly add following tag where applicable
>> | Reported-by: kernel test robot <[email protected]>
>> | Reported-by: Dan Carpenter <[email protected]>
>> | Link: https://lore.kernel.org/r/[email protected]/
>>
>> smatch warnings:
>> include/media/videobuf2-core.h:1272 vb2_queue_add_buffer() warn: sleeping in atomic context
>> drivers/media/common/videobuf2/videobuf2-core.c:2456 vb2_core_queue_init() warn: Please consider using kcalloc instead of kmalloc_array
>>
>> vim +1272 include/media/videobuf2-core.h
>>
>> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1263 static inline bool vb2_queue_add_buffer(struct vb2_queue *q, struct vb2_buffer *vb)
>> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1264 {
>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1265 bool ret = false;
>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1266
>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1267 spin_lock(&q->bufs_lock);
>> ^^^^^^^^^^^^^^^^^^^^^^^
>> Holding a spin lock.
>>
>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1268
>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1269 if (vb->index >= q->max_num_bufs) {
>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1270 struct vb2_buffer **tmp;
>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1271
>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 @1272 tmp = krealloc_array(q->bufs, q->max_num_bufs * 2, sizeof(*q->bufs), GFP_KERNEL);
>> ^^^^^^^^^^
>> Sleeping allocation. GFP_ATOMIC? Or is there a way to move the
>> allocation outside the lock?
>
> I will add GFP_ATOMIC flag in next version.
No need. Instead, don't use realloc here, just allocate a new array, copy over all
the data from the old, and then switch q->bufs with the spinlock held. Then you
can free the old one.
It's only when you update q->bufs that you need the lock.
Regards,
Hans
>
> Thanks for your help,
> Benjamin
>
>>
>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1273 if (!tmp)
>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1274 goto realloc_failed;
>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1275
>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1276 q->max_num_bufs *= 2;
>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1277 q->bufs = tmp;
>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1278 }
>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1279
>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1280 if (vb->index < q->max_num_bufs) {
>> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1281 q->bufs[vb->index] = vb;
>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1282 ret = true;
>> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1283 }
>> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1284
>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1285 realloc_failed:
>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1286 spin_unlock(&q->bufs_lock);
>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1287
>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1288 return ret;
>> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1289 }
>>
On Fri, Mar 24, 2023 at 09:31:35AM +0100, Hans Verkuil wrote:
> On 24/03/2023 09:11, Benjamin Gaignard wrote:
> >
> > Le 24/03/2023 à 06:01, Dan Carpenter a écrit :
> >> Hi Benjamin,
> >>
> >> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> >>
> >> url: https://github.com/intel-lab-lkp/linux/commits/Benjamin-Gaignard/media-videobuf2-Access-vb2_queue-bufs-array-through-helper-functions/20230321-183154
> >> base: git://linuxtv.org/media_tree.git master
> >> patch link: https://lore.kernel.org/r/20230321102855.346732-3-benjamin.gaignard%40collabora.com
> >> patch subject: [PATCH v2 2/8] media: videobuf2: Make bufs array dynamic allocated
> >> config: arm64-randconfig-m041-20230319 (https://download.01.org/0day-ci/archive/20230324/[email protected]/config)
> >> compiler: aarch64-linux-gcc (GCC) 12.1.0
> >>
> >> If you fix the issue, kindly add following tag where applicable
> >> | Reported-by: kernel test robot <[email protected]>
> >> | Reported-by: Dan Carpenter <[email protected]>
> >> | Link: https://lore.kernel.org/r/[email protected]/
> >>
> >> smatch warnings:
> >> include/media/videobuf2-core.h:1272 vb2_queue_add_buffer() warn: sleeping in atomic context
> >> drivers/media/common/videobuf2/videobuf2-core.c:2456 vb2_core_queue_init() warn: Please consider using kcalloc instead of kmalloc_array
> >>
> >> vim +1272 include/media/videobuf2-core.h
> >>
> >> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1263 static inline bool vb2_queue_add_buffer(struct vb2_queue *q, struct vb2_buffer *vb)
> >> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1264 {
> >> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1265 bool ret = false;
> >> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1266
> >> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1267 spin_lock(&q->bufs_lock);
> >> ^^^^^^^^^^^^^^^^^^^^^^^
> >> Holding a spin lock.
> >>
> >> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1268
> >> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1269 if (vb->index >= q->max_num_bufs) {
> >> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1270 struct vb2_buffer **tmp;
> >> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1271
> >> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 @1272 tmp = krealloc_array(q->bufs, q->max_num_bufs * 2, sizeof(*q->bufs), GFP_KERNEL);
> >> ^^^^^^^^^^
> >> Sleeping allocation. GFP_ATOMIC? Or is there a way to move the
> >> allocation outside the lock?
> >
> > I will add GFP_ATOMIC flag in next version.
>
> No need. Instead, don't use realloc here, just allocate a new array, copy over all
> the data from the old, and then switch q->bufs with the spinlock held. Then you
> can free the old one.
>
> It's only when you update q->bufs that you need the lock.
The copy also needs to be protected by the lock.
> >> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1273 if (!tmp)
> >> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1274 goto realloc_failed;
> >> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1275
> >> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1276 q->max_num_bufs *= 2;
> >> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1277 q->bufs = tmp;
> >> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1278 }
> >> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1279
> >> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1280 if (vb->index < q->max_num_bufs) {
> >> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1281 q->bufs[vb->index] = vb;
> >> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1282 ret = true;
> >> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1283 }
> >> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1284
> >> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1285 realloc_failed:
> >> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1286 spin_unlock(&q->bufs_lock);
> >> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1287
> >> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1288 return ret;
> >> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1289 }
--
Regards,
Laurent Pinchart
On 24/03/2023 09:48, Laurent Pinchart wrote:
> On Fri, Mar 24, 2023 at 09:31:35AM +0100, Hans Verkuil wrote:
>> On 24/03/2023 09:11, Benjamin Gaignard wrote:
>>>
>>> Le 24/03/2023 à 06:01, Dan Carpenter a écrit :
>>>> Hi Benjamin,
>>>>
>>>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>>>
>>>> url: https://github.com/intel-lab-lkp/linux/commits/Benjamin-Gaignard/media-videobuf2-Access-vb2_queue-bufs-array-through-helper-functions/20230321-183154
>>>> base: git://linuxtv.org/media_tree.git master
>>>> patch link: https://lore.kernel.org/r/20230321102855.346732-3-benjamin.gaignard%40collabora.com
>>>> patch subject: [PATCH v2 2/8] media: videobuf2: Make bufs array dynamic allocated
>>>> config: arm64-randconfig-m041-20230319 (https://download.01.org/0day-ci/archive/20230324/[email protected]/config)
>>>> compiler: aarch64-linux-gcc (GCC) 12.1.0
>>>>
>>>> If you fix the issue, kindly add following tag where applicable
>>>> | Reported-by: kernel test robot <[email protected]>
>>>> | Reported-by: Dan Carpenter <[email protected]>
>>>> | Link: https://lore.kernel.org/r/[email protected]/
>>>>
>>>> smatch warnings:
>>>> include/media/videobuf2-core.h:1272 vb2_queue_add_buffer() warn: sleeping in atomic context
>>>> drivers/media/common/videobuf2/videobuf2-core.c:2456 vb2_core_queue_init() warn: Please consider using kcalloc instead of kmalloc_array
>>>>
>>>> vim +1272 include/media/videobuf2-core.h
>>>>
>>>> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1263 static inline bool vb2_queue_add_buffer(struct vb2_queue *q, struct vb2_buffer *vb)
>>>> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1264 {
>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1265 bool ret = false;
>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1266
>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1267 spin_lock(&q->bufs_lock);
>>>> ^^^^^^^^^^^^^^^^^^^^^^^
>>>> Holding a spin lock.
>>>>
>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1268
>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1269 if (vb->index >= q->max_num_bufs) {
>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1270 struct vb2_buffer **tmp;
>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1271
>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 @1272 tmp = krealloc_array(q->bufs, q->max_num_bufs * 2, sizeof(*q->bufs), GFP_KERNEL);
>>>> ^^^^^^^^^^
>>>> Sleeping allocation. GFP_ATOMIC? Or is there a way to move the
>>>> allocation outside the lock?
>>>
>>> I will add GFP_ATOMIC flag in next version.
>>
>> No need. Instead, don't use realloc here, just allocate a new array, copy over all
>> the data from the old, and then switch q->bufs with the spinlock held. Then you
>> can free the old one.
>>
>> It's only when you update q->bufs that you need the lock.
>
> The copy also needs to be protected by the lock.
I suspect that that is not needed, since you shouldn't be able to add buffers here
since a mutex should be held at this time.
That said, it's something that Benjamin needs to analyze.
Regards,
Hans
>
>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1273 if (!tmp)
>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1274 goto realloc_failed;
>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1275
>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1276 q->max_num_bufs *= 2;
>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1277 q->bufs = tmp;
>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1278 }
>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1279
>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1280 if (vb->index < q->max_num_bufs) {
>>>> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1281 q->bufs[vb->index] = vb;
>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1282 ret = true;
>>>> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1283 }
>>>> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1284
>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1285 realloc_failed:
>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1286 spin_unlock(&q->bufs_lock);
>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1287
>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1288 return ret;
>>>> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1289 }
>
Le 24/03/2023 à 09:52, Hans Verkuil a écrit :
> On 24/03/2023 09:48, Laurent Pinchart wrote:
>> On Fri, Mar 24, 2023 at 09:31:35AM +0100, Hans Verkuil wrote:
>>> On 24/03/2023 09:11, Benjamin Gaignard wrote:
>>>> Le 24/03/2023 à 06:01, Dan Carpenter a écrit :
>>>>> Hi Benjamin,
>>>>>
>>>>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>>>>
>>>>> url: https://github.com/intel-lab-lkp/linux/commits/Benjamin-Gaignard/media-videobuf2-Access-vb2_queue-bufs-array-through-helper-functions/20230321-183154
>>>>> base: git://linuxtv.org/media_tree.git master
>>>>> patch link: https://lore.kernel.org/r/20230321102855.346732-3-benjamin.gaignard%40collabora.com
>>>>> patch subject: [PATCH v2 2/8] media: videobuf2: Make bufs array dynamic allocated
>>>>> config: arm64-randconfig-m041-20230319 (https://download.01.org/0day-ci/archive/20230324/[email protected]/config)
>>>>> compiler: aarch64-linux-gcc (GCC) 12.1.0
>>>>>
>>>>> If you fix the issue, kindly add following tag where applicable
>>>>> | Reported-by: kernel test robot <[email protected]>
>>>>> | Reported-by: Dan Carpenter <[email protected]>
>>>>> | Link: https://lore.kernel.org/r/[email protected]/
>>>>>
>>>>> smatch warnings:
>>>>> include/media/videobuf2-core.h:1272 vb2_queue_add_buffer() warn: sleeping in atomic context
>>>>> drivers/media/common/videobuf2/videobuf2-core.c:2456 vb2_core_queue_init() warn: Please consider using kcalloc instead of kmalloc_array
>>>>>
>>>>> vim +1272 include/media/videobuf2-core.h
>>>>>
>>>>> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1263 static inline bool vb2_queue_add_buffer(struct vb2_queue *q, struct vb2_buffer *vb)
>>>>> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1264 {
>>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1265 bool ret = false;
>>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1266
>>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1267 spin_lock(&q->bufs_lock);
>>>>> ^^^^^^^^^^^^^^^^^^^^^^^
>>>>> Holding a spin lock.
>>>>>
>>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1268
>>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1269 if (vb->index >= q->max_num_bufs) {
>>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1270 struct vb2_buffer **tmp;
>>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1271
>>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 @1272 tmp = krealloc_array(q->bufs, q->max_num_bufs * 2, sizeof(*q->bufs), GFP_KERNEL);
>>>>> ^^^^^^^^^^
>>>>> Sleeping allocation. GFP_ATOMIC? Or is there a way to move the
>>>>> allocation outside the lock?
>>>> I will add GFP_ATOMIC flag in next version.
>>> No need. Instead, don't use realloc here, just allocate a new array, copy over all
>>> the data from the old, and then switch q->bufs with the spinlock held. Then you
>>> can free the old one.
>>>
>>> It's only when you update q->bufs that you need the lock.
>> The copy also needs to be protected by the lock.
> I suspect that that is not needed, since you shouldn't be able to add buffers here
> since a mutex should be held at this time.
>
> That said, it's something that Benjamin needs to analyze.
Does using GFP_ATOMIC is problematic ?
>
> Regards,
>
> Hans
>
>>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1273 if (!tmp)
>>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1274 goto realloc_failed;
>>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1275
>>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1276 q->max_num_bufs *= 2;
>>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1277 q->bufs = tmp;
>>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1278 }
>>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1279
>>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1280 if (vb->index < q->max_num_bufs) {
>>>>> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1281 q->bufs[vb->index] = vb;
>>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1282 ret = true;
>>>>> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1283 }
>>>>> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1284
>>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1285 realloc_failed:
>>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1286 spin_unlock(&q->bufs_lock);
>>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1287
>>>>> 487d3f14d12ecf Benjamin Gaignard 2023-03-21 1288 return ret;
>>>>> 625d46c1c1fe8e Benjamin Gaignard 2023-03-21 1289 }
>
On 3/21/23 13:28, Benjamin Gaignard wrote:
> + q->bufs = kmalloc_array(q->max_num_bufs, sizeof(*q->bufs), GFP_KERNEL | __GFP_ZERO);
> + if (!q->bufs)
> + return -ENOMEM;
> +
Use kcalloc()
--
Best regards,
Dmitry
On Fri, Mar 24, 2023 at 09:56:34AM +0100, Benjamin Gaignard wrote:
>
> Le 24/03/2023 ? 09:52, Hans Verkuil a ?crit?:
> > On 24/03/2023 09:48, Laurent Pinchart wrote:
> > > On Fri, Mar 24, 2023 at 09:31:35AM +0100, Hans Verkuil wrote:
> > > > On 24/03/2023 09:11, Benjamin Gaignard wrote:
> > > > > Le 24/03/2023 ? 06:01, Dan Carpenter a ?crit?:
> > > > > > Hi Benjamin,
> > > > > >
> > > > > > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> > > > > >
> > > > > > url:??? https://github.com/intel-lab-lkp/linux/commits/Benjamin-Gaignard/media-videobuf2-Access-vb2_queue-bufs-array-through-helper-functions/20230321-183154
> > > > > > base:?? git://linuxtv.org/media_tree.git master
> > > > > > patch link:??? https://lore.kernel.org/r/20230321102855.346732-3-benjamin.gaignard%40collabora.com
> > > > > > patch subject: [PATCH v2 2/8] media: videobuf2: Make bufs array dynamic allocated
> > > > > > config: arm64-randconfig-m041-20230319 (https://download.01.org/0day-ci/archive/20230324/[email protected]/config)
> > > > > > compiler: aarch64-linux-gcc (GCC) 12.1.0
> > > > > >
> > > > > > If you fix the issue, kindly add following tag where applicable
> > > > > > | Reported-by: kernel test robot <[email protected]>
> > > > > > | Reported-by: Dan Carpenter <[email protected]>
> > > > > > | Link: https://lore.kernel.org/r/[email protected]/
> > > > > >
> > > > > > smatch warnings:
> > > > > > include/media/videobuf2-core.h:1272 vb2_queue_add_buffer() warn: sleeping in atomic context
> > > > > > drivers/media/common/videobuf2/videobuf2-core.c:2456 vb2_core_queue_init() warn: Please consider using kcalloc instead of kmalloc_array
> > > > > >
> > > > > > vim +1272 include/media/videobuf2-core.h
> > > > > >
> > > > > > 625d46c1c1fe8e Benjamin Gaignard 2023-03-21? 1263? static inline bool vb2_queue_add_buffer(struct vb2_queue *q, struct vb2_buffer *vb)
> > > > > > 625d46c1c1fe8e Benjamin Gaignard 2023-03-21? 1264? {
> > > > > > 487d3f14d12ecf Benjamin Gaignard 2023-03-21? 1265????? bool ret = false;
> > > > > > 487d3f14d12ecf Benjamin Gaignard 2023-03-21? 1266
> > > > > > 487d3f14d12ecf Benjamin Gaignard 2023-03-21? 1267????? spin_lock(&q->bufs_lock);
> > > > > > ???????????????????????????????????????????????????????? ^^^^^^^^^^^^^^^^^^^^^^^
> > > > > > Holding a spin lock.
> > > > > >
> > > > > > 487d3f14d12ecf Benjamin Gaignard 2023-03-21? 1268
> > > > > > 487d3f14d12ecf Benjamin Gaignard 2023-03-21? 1269????? if (vb->index >= q->max_num_bufs) {
> > > > > > 487d3f14d12ecf Benjamin Gaignard 2023-03-21? 1270????????? struct vb2_buffer **tmp;
> > > > > > 487d3f14d12ecf Benjamin Gaignard 2023-03-21? 1271
> > > > > > 487d3f14d12ecf Benjamin Gaignard 2023-03-21 @1272????????? tmp = krealloc_array(q->bufs, q->max_num_bufs * 2, sizeof(*q->bufs), GFP_KERNEL);
> > > > > > ????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????? ^^^^^^^^^^
> > > > > > Sleeping allocation.? GFP_ATOMIC?? Or is there a way to move the
> > > > > > allocation outside the lock?
> > > > > I will add GFP_ATOMIC flag in next version.
> > > > No need. Instead, don't use realloc here, just allocate a new array, copy over all
> > > > the data from the old, and then switch q->bufs with the spinlock held. Then you
> > > > can free the old one.
> > > >
> > > > It's only when you update q->bufs that you need the lock.
> > > The copy also needs to be protected by the lock.
> > I suspect that that is not needed, since you shouldn't be able to add buffers here
> > since a mutex should be held at this time.
> >
> > That said, it's something that Benjamin needs to analyze.
I spent some time looking through the call sites of vb2_get_buffer() and
how those can be called and it turned out to be a massive list of code
paths, including a lot of calls originating from codec drivers calling
vb2_find_buffer() in random contexts (possibly even interrupt). So a
spinlock protecting the array pointer makes sense indeed.
>
> Does using GFP_ATOMIC is problematic ?
>
Yes, because the ability to reclaim memory is drastically limited and
the allocation is more likely to fail (as in: it's actually possible).
(And generally the time with interrupts disabled should be minimized to
keep system latency reasonable.)
Best regards,
Tomasz
On Tue, Mar 21, 2023 at 08:16:10PM +0200, Laurent Pinchart wrote:
> Hi Benjamin,
>
> Thank you for the patch.
>
> On Tue, Mar 21, 2023 at 11:28:49AM +0100, Benjamin Gaignard wrote:
> > Instead of a static array change bufs to a dynamically allocated array.
> > This will allow to store more video buffer if needed.
> > Protect the array with a spinlock.
>
> I think I asked in the review of v1 why we couldn't use the kernel
> IDA/IDR APIs to allocate buffer IDs and register buffers, but I don't
> think I've received a reply. Wouldn't it be better than rolling out our
> own mechanism ?
>
+1, with a note that [1] suggests that IDR is deprecated and XArray[2]
should be used instead.
[1] https://docs.kernel.org/core-api/idr.html
[2] https://docs.kernel.org/core-api/xarray.html
Also from my quick look, XArray may be solving the locking concerns for us
automatically.
Best regards,
Tomasz
> > Signed-off-by: Benjamin Gaignard <[email protected]>
> > ---
> > .../media/common/videobuf2/videobuf2-core.c | 8 +++
> > include/media/videobuf2-core.h | 49 ++++++++++++++++---
> > 2 files changed, 49 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> > index 79e90e338846..ae9d72f4d181 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-core.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> > @@ -2452,6 +2452,13 @@ int vb2_core_queue_init(struct vb2_queue *q)
> > mutex_init(&q->mmap_lock);
> > init_waitqueue_head(&q->done_wq);
> >
> > + q->max_num_bufs = 32;
> > + q->bufs = kmalloc_array(q->max_num_bufs, sizeof(*q->bufs), GFP_KERNEL | __GFP_ZERO);
> > + if (!q->bufs)
> > + return -ENOMEM;
> > +
> > + spin_lock_init(&q->bufs_lock);
> > +
> > q->memory = VB2_MEMORY_UNKNOWN;
> >
> > if (q->buf_struct_size == 0)
> > @@ -2479,6 +2486,7 @@ void vb2_core_queue_release(struct vb2_queue *q)
> > mutex_lock(&q->mmap_lock);
> > __vb2_queue_free(q, q->num_buffers);
> > mutex_unlock(&q->mmap_lock);
> > + kfree(q->bufs);
> > }
> > EXPORT_SYMBOL_GPL(vb2_core_queue_release);
> >
> > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> > index 5b1e3d801546..397dbf6e61e1 100644
> > --- a/include/media/videobuf2-core.h
> > +++ b/include/media/videobuf2-core.h
> > @@ -558,6 +558,8 @@ struct vb2_buf_ops {
> > * @dma_dir: DMA mapping direction.
> > * @bufs: videobuf2 buffer structures
> > * @num_buffers: number of allocated/used buffers
> > + * @bufs_lock: lock to protect bufs access
> > + * @max_num_bufs: max number of buffers storable in bufs
> > * @queued_list: list of buffers currently queued from userspace
> > * @queued_count: number of buffers queued and ready for streaming.
> > * @owned_by_drv_count: number of buffers owned by the driver
> > @@ -619,8 +621,10 @@ struct vb2_queue {
> > struct mutex mmap_lock;
> > unsigned int memory;
> > enum dma_data_direction dma_dir;
> > - struct vb2_buffer *bufs[VB2_MAX_FRAME];
> > + struct vb2_buffer **bufs;
> > unsigned int num_buffers;
> > + spinlock_t bufs_lock;
> > + size_t max_num_bufs;
> >
> > struct list_head queued_list;
> > unsigned int queued_count;
> > @@ -1239,9 +1243,16 @@ static inline void vb2_clear_last_buffer_dequeued(struct vb2_queue *q)
> > static inline struct vb2_buffer *vb2_get_buffer(struct vb2_queue *q,
> > unsigned int index)
> > {
> > - if (index < q->num_buffers)
> > - return q->bufs[index];
> > - return NULL;
> > + struct vb2_buffer *vb = NULL;
> > +
> > + spin_lock(&q->bufs_lock);
> > +
> > + if (index < q->max_num_bufs)
> > + vb = q->bufs[index];
> > +
> > + spin_unlock(&q->bufs_lock);
> > +
> > + return vb;
> > }
> >
> > /**
> > @@ -1251,12 +1262,30 @@ static inline struct vb2_buffer *vb2_get_buffer(struct vb2_queue *q,
> > */
> > static inline bool vb2_queue_add_buffer(struct vb2_queue *q, struct vb2_buffer *vb)
> > {
> > - if (vb->index < VB2_MAX_FRAME) {
> > + bool ret = false;
> > +
> > + spin_lock(&q->bufs_lock);
> > +
> > + if (vb->index >= q->max_num_bufs) {
> > + struct vb2_buffer **tmp;
> > +
> > + tmp = krealloc_array(q->bufs, q->max_num_bufs * 2, sizeof(*q->bufs), GFP_KERNEL);
> > + if (!tmp)
> > + goto realloc_failed;
> > +
> > + q->max_num_bufs *= 2;
> > + q->bufs = tmp;
> > + }
> > +
> > + if (vb->index < q->max_num_bufs) {
> > q->bufs[vb->index] = vb;
> > - return true;
> > + ret = true;
> > }
> >
> > - return false;
> > +realloc_failed:
> > + spin_unlock(&q->bufs_lock);
> > +
> > + return ret;
> > }
> >
> > /**
> > @@ -1266,8 +1295,12 @@ static inline bool vb2_queue_add_buffer(struct vb2_queue *q, struct vb2_buffer *
> > */
> > static inline void vb2_queue_remove_buffer(struct vb2_queue *q, struct vb2_buffer *vb)
> > {
> > - if (vb->index < VB2_MAX_FRAME)
> > + spin_lock(&q->bufs_lock);
> > +
> > + if (vb->index < q->max_num_bufs)
> > q->bufs[vb->index] = NULL;
> > +
> > + spin_unlock(&q->bufs_lock);
> > }
> >
> > /*
>
> --
> Regards,
>
> Laurent Pinchart