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 <benjamin.gaignard@collabora.com>
---
.../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
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
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/202303240148.lKRnUqW9-lkp@intel.com/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 <lkp@intel.com> | Reported-by: Dan Carpenter <error27@gmail.com> | Link: https://lore.kernel.org/r/202303240148.lKRnUqW9-lkp@intel.com/ 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/202303240148.lKRnUqW9-lkp@intel.com/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 <lkp@intel.com> > | Reported-by: Dan Carpenter <error27@gmail.com> > | Link: https://lore.kernel.org/r/202303240148.lKRnUqW9-lkp@intel.com/ > > 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/202303240148.lKRnUqW9-lkp@intel.com/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 <lkp@intel.com> >> | Reported-by: Dan Carpenter <error27@gmail.com> >> | Link: https://lore.kernel.org/r/202303240148.lKRnUqW9-lkp@intel.com/ >> >> 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/202303240148.lKRnUqW9-lkp@intel.com/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 <lkp@intel.com> > >> | Reported-by: Dan Carpenter <error27@gmail.com> > >> | Link: https://lore.kernel.org/r/202303240148.lKRnUqW9-lkp@intel.com/ > >> > >> 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/202303240148.lKRnUqW9-lkp@intel.com/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 <lkp@intel.com> >>>> | Reported-by: Dan Carpenter <error27@gmail.com> >>>> | Link: https://lore.kernel.org/r/202303240148.lKRnUqW9-lkp@intel.com/ >>>> >>>> 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/202303240148.lKRnUqW9-lkp@intel.com/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 <lkp@intel.com> >>>>> | Reported-by: Dan Carpenter <error27@gmail.com> >>>>> | Link: https://lore.kernel.org/r/202303240148.lKRnUqW9-lkp@intel.com/ >>>>> >>>>> 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 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/202303240148.lKRnUqW9-lkp@intel.com/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 <lkp@intel.com> > > > > > > | Reported-by: Dan Carpenter <error27@gmail.com> > > > > > > | Link: https://lore.kernel.org/r/202303240148.lKRnUqW9-lkp@intel.com/ > > > > > > > > > > > > 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
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 <benjamin.gaignard@collabora.com> > --- > .../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
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 <benjamin.gaignard@collabora.com> > > --- > > .../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
© 2016 - 2024 Red Hat, Inc.