| Message ID | 20230705121056.37017-2-benjamin.gaignard@collabora.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add DELETE_BUF ioctl | expand |
Le 21/08/2023 à 16:31, Hans Verkuil a écrit : > On 21/08/2023 16:24, Hans Verkuil wrote: >> Hi Benjamin, >> >> On 05/07/2023 14:10, Benjamin Gaignard wrote: >>> The first step before changing how vb2 buffers are stored into queue >>> is to avoid direct access to bufs arrays. >>> >>> This patch adds 2 helpers functions to add and remove vb2 buffers >>> from a queue. With these 2 and vb2_get_buffer(), bufs field of >>> struct vb2_queue becomes like a private member of the structure. >>> >>> After each call to vb2_get_buffer() we need to be sure that we get >>> a valid pointer so check the return value of all of them. >>> >>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> >>> --- >>> .../media/common/videobuf2/videobuf2-core.c | 215 ++++++++++++++---- >>> .../media/common/videobuf2/videobuf2-v4l2.c | 28 ++- >>> drivers/media/platform/amphion/vpu_dbg.c | 22 +- >>> .../platform/mediatek/jpeg/mtk_jpeg_core.c | 6 +- >>> .../vcodec/vdec/vdec_vp9_req_lat_if.c | 2 +- >>> drivers/media/platform/st/sti/hva/hva-v4l2.c | 4 + >>> drivers/media/test-drivers/visl/visl-dec.c | 28 ++- >>> .../staging/media/atomisp/pci/atomisp_ioctl.c | 2 +- >>> 8 files changed, 240 insertions(+), 67 deletions(-) >>> >>> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c >>> index cf6727d9c81f..42fd3984c2bc 100644 >>> --- a/drivers/media/common/videobuf2/videobuf2-core.c >>> +++ b/drivers/media/common/videobuf2/videobuf2-core.c >>> @@ -359,8 +359,16 @@ static void __setup_offsets(struct vb2_buffer *vb) >>> unsigned long off = 0; >>> >>> if (vb->index) { >>> - struct vb2_buffer *prev = q->bufs[vb->index - 1]; >>> - struct vb2_plane *p = &prev->planes[prev->num_planes - 1]; >>> + struct vb2_buffer *prev; >>> + struct vb2_plane *p; >>> + >>> + prev = vb2_get_buffer(q, vb->index - 1); >>> + if (!prev) { >>> + dprintk(q, 1, "can't find the requested buffer\n"); >>> + return; >>> + } >>> + >>> + p = &prev->planes[prev->num_planes - 1]; >>> >>> off = PAGE_ALIGN(p->m.offset + p->length); >> This does not work. Before this patch there should be a patch that changes >> the way the offset is determined since it affects this functionality. >> >> __setup_offsets() fills in the m.offset field for each plane in the buffer. >> In the current code it starts with 0 for the first buffer, adds the length >> for each plane to it, and for the next buffer it continues doing that. So >> the offset for a plane is the total length of all preceding buffers and planes. >> >> In addition, bit 30 of the offset field is used to signal if it is a capture >> or output buffer in the v4l2-mem2mem framework (see the DST_QUEUE_OFF_BASE >> define). >> >> In addition, the offset has to be page aligned, so the lower 12 bits are 0. >> That leaves only 32-1-12 = 19 bits for the offset. >> >> Since we allow 8 planes per buffer, that means that we can't have more than >> 65536 buffers given a 32 bit offset. >> >> I think the way m.offset is calculated should change drastically: it should >> instead be something like this: >> >> m.offset = ((vb->index << 3) | plane) << PAGE_SHIFT; >> >> and DST_QUEUE_OFF_BASE should use bit 31 instead of 30. >> >> Parsing the offset would be much simpler as well, since you can just decode >> the index and plane numbers directly rather than using the cumbersome >> __find_plane_by_offset() function. >> >> The currently mechanism will fail completely once you can delete buffers, >> and this proposal will simplify and improve the code, so I think it should >> be done first. > I could have saved myself some work if I read the other patches first. > > I see that patch 3/10 addresses this. But for the v5, just move that patch > to the beginning of the series. I will try to do that but these patches hit the same pieces of code so changing the order isn't that simple. > > I would also suggest changing DST_QUEUE_OFF_BASE to use bit 31 to allow for > 16 bits instead of 15 for the buffer index. DST_QUEUE_OFF_BASE is used with PAGE_SHIFT in v4l2-mem2mem and s5p_mfc in mmap/unmap functions. I'm bit afraid to change that. With my change each queue can store up to 0xffff (65535) buffers. Sound large enough for me ;-) Benjamin > > Regards, > > Hans > >> Regards, >> >> Hans >> >>> } >>> @@ -397,6 +405,37 @@ static void init_buffer_cache_hints(struct vb2_queue *q, struct vb2_buffer *vb) >>> vb->skip_cache_sync_on_finish = 1; >>> } >>> >>> +/** >>> + * vb2_queue_add_buffer() - add a buffer to a queue >>> + * @q: pointer to &struct vb2_queue with videobuf2 queue. >>> + * @vb: pointer to &struct vb2_buffer to be added to the queue. >>> + * @index: index where add vb2_buffer in the queue >>> + */ >>> +static bool vb2_queue_add_buffer(struct vb2_queue *q, struct vb2_buffer *vb, int index) >>> +{ >>> + if (index < VB2_MAX_FRAME && !q->bufs[index]) { >>> + q->bufs[index] = vb; >>> + vb->index = index; >>> + vb->vb2_queue = q; >>> + return true; >>> + } >>> + >>> + return false; >>> +} >>> + >>> +/** >>> + * vb2_queue_remove_buffer() - remove a buffer from a queue >>> + * @q: pointer to &struct vb2_queue with videobuf2 queue. >>> + * @vb: pointer to &struct vb2_buffer to be removed from the queue. >>> + */ >>> +static void vb2_queue_remove_buffer(struct vb2_queue *q, struct vb2_buffer *vb) >>> +{ >>> + if (vb->index < VB2_MAX_FRAME) { >>> + q->bufs[vb->index] = NULL; >>> + vb->vb2_queue = NULL; >>> + } >>> +} >>> + >>> /* >>> * __vb2_queue_alloc() - allocate vb2 buffer structures and (for MMAP type) >>> * video buffer memory for all buffers/planes on the queue and initializes the >>> @@ -425,9 +464,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory, >>> } >>> >>> vb->state = VB2_BUF_STATE_DEQUEUED; >>> - vb->vb2_queue = q; >>> vb->num_planes = num_planes; >>> - vb->index = q->num_buffers + buffer; >>> vb->type = q->type; >>> vb->memory = memory; >>> init_buffer_cache_hints(q, vb); >>> @@ -437,7 +474,11 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory, >>> } >>> call_void_bufop(q, init_buffer, vb); >>> >>> - q->bufs[vb->index] = vb; >>> + if (!vb2_queue_add_buffer(q, vb, q->num_buffers + buffer)) { >>> + dprintk(q, 1, "failed adding buffer %d to queue\n", buffer); >>> + kfree(vb); >>> + break; >>> + } >>> >>> /* Allocate video buffer memory for the MMAP type */ >>> if (memory == VB2_MEMORY_MMAP) { >>> @@ -445,7 +486,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory, >>> if (ret) { >>> dprintk(q, 1, "failed allocating memory for buffer %d\n", >>> buffer); >>> - q->bufs[vb->index] = NULL; >>> + vb2_queue_remove_buffer(q, vb); >>> kfree(vb); >>> break; >>> } >>> @@ -460,7 +501,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory, >>> dprintk(q, 1, "buffer %d %p initialization failed\n", >>> buffer, vb); >>> __vb2_buf_mem_free(vb); >>> - q->bufs[vb->index] = NULL; >>> + vb2_queue_remove_buffer(q, vb); >>> kfree(vb); >>> break; >>> } >>> @@ -483,7 +524,7 @@ static void __vb2_free_mem(struct vb2_queue *q, unsigned int buffers) >>> >>> for (buffer = q->num_buffers - buffers; buffer < q->num_buffers; >>> ++buffer) { >>> - vb = q->bufs[buffer]; >>> + vb = vb2_get_buffer(q, buffer); >>> if (!vb) >>> continue; >>> >>> @@ -511,7 +552,7 @@ static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) >>> /* Call driver-provided cleanup function for each buffer, if provided */ >>> for (buffer = q->num_buffers - buffers; buffer < q->num_buffers; >>> ++buffer) { >>> - struct vb2_buffer *vb = q->bufs[buffer]; >>> + struct vb2_buffer *vb = vb2_get_buffer(q, buffer); >>> >>> if (vb && vb->planes[0].mem_priv) >>> call_void_vb_qop(vb, buf_cleanup, vb); >>> @@ -551,15 +592,20 @@ static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) >>> q->cnt_unprepare_streaming = 0; >>> } >>> for (buffer = 0; buffer < q->num_buffers; ++buffer) { >>> - struct vb2_buffer *vb = q->bufs[buffer]; >>> - bool unbalanced = vb->cnt_mem_alloc != vb->cnt_mem_put || >>> - vb->cnt_mem_prepare != vb->cnt_mem_finish || >>> - vb->cnt_mem_get_userptr != vb->cnt_mem_put_userptr || >>> - vb->cnt_mem_attach_dmabuf != vb->cnt_mem_detach_dmabuf || >>> - vb->cnt_mem_map_dmabuf != vb->cnt_mem_unmap_dmabuf || >>> - vb->cnt_buf_queue != vb->cnt_buf_done || >>> - vb->cnt_buf_prepare != vb->cnt_buf_finish || >>> - vb->cnt_buf_init != vb->cnt_buf_cleanup; >>> + struct vb2_buffer *vb = vb2_get_buffer(q, buffer); >>> + bool unbalanced; >>> + >>> + if (!vb) >>> + continue; >>> + >>> + unbalanced = vb->cnt_mem_alloc != vb->cnt_mem_put || >>> + vb->cnt_mem_prepare != vb->cnt_mem_finish || >>> + vb->cnt_mem_get_userptr != vb->cnt_mem_put_userptr || >>> + vb->cnt_mem_attach_dmabuf != vb->cnt_mem_detach_dmabuf || >>> + vb->cnt_mem_map_dmabuf != vb->cnt_mem_unmap_dmabuf || >>> + vb->cnt_buf_queue != vb->cnt_buf_done || >>> + vb->cnt_buf_prepare != vb->cnt_buf_finish || >>> + vb->cnt_buf_init != vb->cnt_buf_cleanup; >>> >>> if (unbalanced || debug) { >>> pr_info(" counters for queue %p, buffer %d:%s\n", >>> @@ -591,8 +637,13 @@ static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) >>> /* Free vb2 buffers */ >>> for (buffer = q->num_buffers - buffers; buffer < q->num_buffers; >>> ++buffer) { >>> - kfree(q->bufs[buffer]); >>> - q->bufs[buffer] = NULL; >>> + struct vb2_buffer *vb = vb2_get_buffer(q, buffer); >>> + >>> + if (!vb) >>> + continue; >>> + >>> + vb2_queue_remove_buffer(q, vb); >>> + kfree(vb); >>> } >>> >>> q->num_buffers -= buffers; >>> @@ -628,7 +679,12 @@ static bool __buffers_in_use(struct vb2_queue *q) >>> { >>> unsigned int buffer; >>> for (buffer = 0; buffer < q->num_buffers; ++buffer) { >>> - if (vb2_buffer_in_use(q, q->bufs[buffer])) >>> + struct vb2_buffer *vb = vb2_get_buffer(q, buffer); >>> + >>> + if (!vb) >>> + continue; >>> + >>> + if (vb2_buffer_in_use(q, vb)) >>> return true; >>> } >>> return false; >>> @@ -636,7 +692,10 @@ static bool __buffers_in_use(struct vb2_queue *q) >>> >>> void vb2_core_querybuf(struct vb2_queue *q, unsigned int index, void *pb) >>> { >>> - call_void_bufop(q, fill_user_buffer, q->bufs[index], pb); >>> + struct vb2_buffer *vb = vb2_get_buffer(q, index); >>> + >>> + if (vb) >>> + call_void_bufop(q, fill_user_buffer, vb, pb); >>> } >>> EXPORT_SYMBOL_GPL(vb2_core_querybuf); >>> >>> @@ -1547,7 +1606,13 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb) >>> struct vb2_buffer *vb; >>> int ret; >>> >>> - vb = q->bufs[index]; >>> + vb = vb2_get_buffer(q, index); >>> + >>> + if (!vb) { >>> + dprintk(q, 1, "can't find the requested buffer\n"); >>> + return -EINVAL; >>> + } >>> + >>> if (vb->state != VB2_BUF_STATE_DEQUEUED) { >>> dprintk(q, 1, "invalid buffer state %s\n", >>> vb2_state_name(vb->state)); >>> @@ -1618,7 +1683,11 @@ static int vb2_start_streaming(struct vb2_queue *q) >>> * correctly return them to vb2. >>> */ >>> for (i = 0; i < q->num_buffers; ++i) { >>> - vb = q->bufs[i]; >>> + vb = vb2_get_buffer(q, i); >>> + >>> + if (!vb) >>> + continue; >>> + >>> if (vb->state == VB2_BUF_STATE_ACTIVE) >>> vb2_buffer_done(vb, VB2_BUF_STATE_QUEUED); >>> } >>> @@ -1646,7 +1715,12 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb, >>> return -EIO; >>> } >>> >>> - vb = q->bufs[index]; >>> + vb = vb2_get_buffer(q, index); >>> + >>> + if (!vb) { >>> + dprintk(q, 1, "can't find the requested buffer\n"); >>> + return -EINVAL; >>> + } >>> >>> if (!req && vb->state != VB2_BUF_STATE_IN_REQUEST && >>> q->requires_requests) { >>> @@ -2022,12 +2096,18 @@ static void __vb2_queue_cancel(struct vb2_queue *q) >>> * to vb2 in stop_streaming(). >>> */ >>> if (WARN_ON(atomic_read(&q->owned_by_drv_count))) { >>> - for (i = 0; i < q->num_buffers; ++i) >>> - if (q->bufs[i]->state == VB2_BUF_STATE_ACTIVE) { >>> + for (i = 0; i < q->num_buffers; ++i) { >>> + struct vb2_buffer *vb = vb2_get_buffer(q, i); >>> + >>> + if (!vb) >>> + continue; >>> + >>> + if (vb->state == VB2_BUF_STATE_ACTIVE) { >>> pr_warn("driver bug: stop_streaming operation is leaving buf %p in active state\n", >>> - q->bufs[i]); >>> - vb2_buffer_done(q->bufs[i], VB2_BUF_STATE_ERROR); >>> + vb); >>> + vb2_buffer_done(vb, VB2_BUF_STATE_ERROR); >>> } >>> + } >>> /* Must be zero now */ >>> WARN_ON(atomic_read(&q->owned_by_drv_count)); >>> } >>> @@ -2061,9 +2141,14 @@ static void __vb2_queue_cancel(struct vb2_queue *q) >>> * be changed, so we can't move the buf_finish() to __vb2_dqbuf(). >>> */ >>> for (i = 0; i < q->num_buffers; ++i) { >>> - struct vb2_buffer *vb = q->bufs[i]; >>> - struct media_request *req = vb->req_obj.req; >>> + struct vb2_buffer *vb; >>> + struct media_request *req; >>> >>> + vb = vb2_get_buffer(q, i); >>> + if (!vb) >>> + continue; >>> + >>> + req = vb->req_obj.req; >>> /* >>> * If a request is associated with this buffer, then >>> * call buf_request_cancel() to give the driver to complete() >>> @@ -2215,7 +2300,10 @@ static int __find_plane_by_offset(struct vb2_queue *q, unsigned long off, >>> * return its buffer and plane numbers. >>> */ >>> for (buffer = 0; buffer < q->num_buffers; ++buffer) { >>> - vb = q->bufs[buffer]; >>> + vb = vb2_get_buffer(q, buffer); >>> + >>> + if (!vb) >>> + continue; >>> >>> for (plane = 0; plane < vb->num_planes; ++plane) { >>> if (vb->planes[plane].m.offset == off) { >>> @@ -2262,7 +2350,12 @@ int vb2_core_expbuf(struct vb2_queue *q, int *fd, unsigned int type, >>> return -EINVAL; >>> } >>> >>> - vb = q->bufs[index]; >>> + vb = vb2_get_buffer(q, index); >>> + >>> + if (!vb) { >>> + dprintk(q, 1, "can't find the requested buffer\n"); >>> + return -EINVAL; >>> + } >>> >>> if (plane >= vb->num_planes) { >>> dprintk(q, 1, "buffer plane out of range\n"); >>> @@ -2339,7 +2432,13 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma) >>> if (ret) >>> goto unlock; >>> >>> - vb = q->bufs[buffer]; >>> + vb = vb2_get_buffer(q, buffer); >>> + >>> + if (!vb) { >>> + dprintk(q, 1, "can't find the requested buffer\n"); >>> + ret = -EINVAL; >>> + goto unlock; >>> + } >>> >>> /* >>> * MMAP requires page_aligned buffers. >>> @@ -2396,7 +2495,12 @@ unsigned long vb2_get_unmapped_area(struct vb2_queue *q, >>> if (ret) >>> goto unlock; >>> >>> - vb = q->bufs[buffer]; >>> + vb = vb2_get_buffer(q, buffer); >>> + if (!vb) { >>> + dprintk(q, 1, "can't find the requested buffer\n"); >>> + ret = -EINVAL; >>> + goto unlock; >>> + } >>> >>> vaddr = vb2_plane_vaddr(vb, plane); >>> mutex_unlock(&q->mmap_lock); >>> @@ -2625,6 +2729,7 @@ struct vb2_fileio_data { >>> static int __vb2_init_fileio(struct vb2_queue *q, int read) >>> { >>> struct vb2_fileio_data *fileio; >>> + struct vb2_buffer *vb; >>> int i, ret; >>> unsigned int count = 0; >>> >>> @@ -2679,7 +2784,13 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read) >>> * Check if plane_count is correct >>> * (multiplane buffers are not supported). >>> */ >>> - if (q->bufs[0]->num_planes != 1) { >>> + vb = vb2_get_buffer(q, 0); >>> + if (!vb) { >>> + ret = -EBUSY; >>> + goto err_reqbufs; >>> + } >>> + >>> + if (vb->num_planes != 1) { >>> ret = -EBUSY; >>> goto err_reqbufs; >>> } >>> @@ -2688,12 +2799,17 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read) >>> * Get kernel address of each buffer. >>> */ >>> for (i = 0; i < q->num_buffers; i++) { >>> - fileio->bufs[i].vaddr = vb2_plane_vaddr(q->bufs[i], 0); >>> + vb = vb2_get_buffer(q, i); >>> + >>> + if (!vb) >>> + continue; >>> + >>> + fileio->bufs[i].vaddr = vb2_plane_vaddr(vb, 0); >>> if (fileio->bufs[i].vaddr == NULL) { >>> ret = -EINVAL; >>> goto err_reqbufs; >>> } >>> - fileio->bufs[i].size = vb2_plane_size(q->bufs[i], 0); >>> + fileio->bufs[i].size = vb2_plane_size(vb, 0); >>> } >>> >>> /* >>> @@ -2821,15 +2937,18 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ >>> >>> fileio->cur_index = index; >>> buf = &fileio->bufs[index]; >>> - b = q->bufs[index]; >>> + b = vb2_get_buffer(q, index); >>> + >>> + if (!b) >>> + return -EINVAL; >>> >>> /* >>> * Get number of bytes filled by the driver >>> */ >>> buf->pos = 0; >>> buf->queued = 0; >>> - buf->size = read ? vb2_get_plane_payload(q->bufs[index], 0) >>> - : vb2_plane_size(q->bufs[index], 0); >>> + buf->size = read ? vb2_get_plane_payload(b, 0) >>> + : vb2_plane_size(b, 0); >>> /* Compensate for data_offset on read in the multiplanar case. */ >>> if (is_multiplanar && read && >>> b->planes[0].data_offset < buf->size) { >>> @@ -2872,8 +2991,12 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ >>> * Queue next buffer if required. >>> */ >>> if (buf->pos == buf->size || (!read && fileio->write_immediately)) { >>> - struct vb2_buffer *b = q->bufs[index]; >>> + struct vb2_buffer *b = vb2_get_buffer(q, index); >>> >>> + if (!b) { >>> + dprintk(q, 1, "can't find the requested buffer\n"); >>> + return -EINVAL; >>> + } >>> /* >>> * Check if this is the last buffer to read. >>> */ >>> @@ -2899,7 +3022,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ >>> */ >>> buf->pos = 0; >>> buf->queued = 1; >>> - buf->size = vb2_plane_size(q->bufs[index], 0); >>> + buf->size = vb2_plane_size(b, 0); >>> fileio->q_count += 1; >>> /* >>> * If we are queuing up buffers for the first time, then >>> @@ -2970,7 +3093,9 @@ static int vb2_thread(void *data) >>> * Call vb2_dqbuf to get buffer back. >>> */ >>> if (prequeue) { >>> - vb = q->bufs[index++]; >>> + vb = vb2_get_buffer(q, index++); >>> + if (!vb) >>> + continue; >>> prequeue--; >>> } else { >>> call_void_qop(q, wait_finish, q); >>> @@ -2979,7 +3104,7 @@ static int vb2_thread(void *data) >>> call_void_qop(q, wait_prepare, q); >>> dprintk(q, 5, "file io: vb2_dqbuf result: %d\n", ret); >>> if (!ret) >>> - vb = q->bufs[index]; >>> + vb = vb2_get_buffer(q, index); >>> } >>> if (ret || threadio->stop) >>> break; >>> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c >>> index c7a54d82a55e..724135d41f7f 100644 >>> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c >>> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c >>> @@ -383,8 +383,7 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md >>> return -EINVAL; >>> } >>> >>> - if (q->bufs[b->index] == NULL) { >>> - /* Should never happen */ >>> + if (!vb2_get_buffer(q, b->index)) { >>> dprintk(q, 1, "%s: buffer is NULL\n", opname); >>> return -EINVAL; >>> } >>> @@ -394,7 +393,7 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md >>> return -EINVAL; >>> } >>> >>> - vb = q->bufs[b->index]; >>> + vb = vb2_get_buffer(q, b->index); >>> vbuf = to_vb2_v4l2_buffer(vb); >>> ret = __verify_planes_array(vb, b); >>> if (ret) >>> @@ -628,11 +627,18 @@ static const struct vb2_buf_ops v4l2_buf_ops = { >>> struct vb2_buffer *vb2_find_buffer(struct vb2_queue *q, u64 timestamp) >>> { >>> unsigned int i; >>> + struct vb2_buffer *vb2; >>> >>> - for (i = 0; i < q->num_buffers; i++) >>> - if (q->bufs[i]->copied_timestamp && >>> - q->bufs[i]->timestamp == timestamp) >>> - return vb2_get_buffer(q, i); >>> + for (i = 0; i < q->num_buffers; i++) { >>> + vb2 = vb2_get_buffer(q, i); >>> + >>> + if (!vb2) >>> + continue; >>> + >>> + if (vb2->copied_timestamp && >>> + vb2->timestamp == timestamp) >>> + return vb2; >>> + } >>> return NULL; >>> } >>> EXPORT_SYMBOL_GPL(vb2_find_buffer); >>> @@ -664,7 +670,13 @@ int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b) >>> dprintk(q, 1, "buffer index out of range\n"); >>> return -EINVAL; >>> } >>> - vb = q->bufs[b->index]; >>> + vb = vb2_get_buffer(q, b->index); >>> + >>> + if (!vb) { >>> + dprintk(q, 1, "can't find the requested buffer\n"); >>> + return -EINVAL; >>> + } >>> + >>> ret = __verify_planes_array(vb, b); >>> if (!ret) >>> vb2_core_querybuf(q, b->index, b); >>> diff --git a/drivers/media/platform/amphion/vpu_dbg.c b/drivers/media/platform/amphion/vpu_dbg.c >>> index 44b830ae01d8..e2c371c2fde5 100644 >>> --- a/drivers/media/platform/amphion/vpu_dbg.c >>> +++ b/drivers/media/platform/amphion/vpu_dbg.c >>> @@ -133,11 +133,18 @@ static int vpu_dbg_instance(struct seq_file *s, void *data) >>> >>> vq = v4l2_m2m_get_src_vq(inst->fh.m2m_ctx); >>> for (i = 0; i < vq->num_buffers; i++) { >>> - struct vb2_buffer *vb = vq->bufs[i]; >>> - struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb); >>> + struct vb2_buffer *vb; >>> + struct vb2_v4l2_buffer *vbuf; >>> + >>> + vb = vb2_get_buffer(vq, i); >>> + if (!vb) >>> + continue; >>> >>> if (vb->state == VB2_BUF_STATE_DEQUEUED) >>> continue; >>> + >>> + vbuf = to_vb2_v4l2_buffer(vb); >>> + >>> num = scnprintf(str, sizeof(str), >>> "output [%2d] state = %10s, %8s\n", >>> i, vb2_stat_name[vb->state], >>> @@ -148,11 +155,18 @@ static int vpu_dbg_instance(struct seq_file *s, void *data) >>> >>> vq = v4l2_m2m_get_dst_vq(inst->fh.m2m_ctx); >>> for (i = 0; i < vq->num_buffers; i++) { >>> - struct vb2_buffer *vb = vq->bufs[i]; >>> - struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb); >>> + struct vb2_buffer *vb; >>> + struct vb2_v4l2_buffer *vbuf; >>> + >>> + vb = vb2_get_buffer(vq, i); >>> + if (!vb) >>> + continue; >>> >>> if (vb->state == VB2_BUF_STATE_DEQUEUED) >>> continue; >>> + >>> + vbuf = to_vb2_v4l2_buffer(vb); >>> + >>> num = scnprintf(str, sizeof(str), >>> "capture[%2d] state = %10s, %8s\n", >>> i, vb2_stat_name[vb->state], >>> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c >>> index 0051f372a66c..ea37069ba355 100644 >>> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c >>> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c >>> @@ -603,7 +603,11 @@ static int mtk_jpeg_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf) >>> return -EINVAL; >>> } >>> >>> - vb = vq->bufs[buf->index]; >>> + vb = vb2_get_buffer(vq, buf->index); >>> + if (!vb) { >>> + dev_err(ctx->jpeg->dev, "buffer not found\n"); >>> + return -EINVAL; >>> + } >>> jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(vb); >>> jpeg_src_buf->bs_size = buf->m.planes[0].bytesused; >>> >>> diff --git a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c >>> index cf16cf2807f0..6532a69f1fa8 100644 >>> --- a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c >>> +++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c >>> @@ -1701,7 +1701,7 @@ static int vdec_vp9_slice_setup_core_buffer(struct vdec_vp9_slice_instance *inst >>> >>> /* update internal buffer's width/height */ >>> for (i = 0; i < vq->num_buffers; i++) { >>> - if (vb == vq->bufs[i]) { >>> + if (vb == vb2_get_buffer(vq, i)) { >>> instance->dpb[i].width = w; >>> instance->dpb[i].height = h; >>> break; >>> diff --git a/drivers/media/platform/st/sti/hva/hva-v4l2.c b/drivers/media/platform/st/sti/hva/hva-v4l2.c >>> index 3a848ca32a0e..326be09bdb55 100644 >>> --- a/drivers/media/platform/st/sti/hva/hva-v4l2.c >>> +++ b/drivers/media/platform/st/sti/hva/hva-v4l2.c >>> @@ -577,6 +577,10 @@ static int hva_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf) >>> } >>> >>> vb2_buf = vb2_get_buffer(vq, buf->index); >>> + if (!vb2_buf) { >>> + dev_dbg(dev, "%s buffer index %d not found\n", ctx->name, buf->index); >>> + return -EINVAL; >>> + } >>> stream = to_hva_stream(to_vb2_v4l2_buffer(vb2_buf)); >>> stream->bytesused = buf->bytesused; >>> } >>> diff --git a/drivers/media/test-drivers/visl/visl-dec.c b/drivers/media/test-drivers/visl/visl-dec.c >>> index 318d675e5668..ba20ea998d19 100644 >>> --- a/drivers/media/test-drivers/visl/visl-dec.c >>> +++ b/drivers/media/test-drivers/visl/visl-dec.c >>> @@ -290,13 +290,20 @@ static void visl_tpg_fill(struct visl_ctx *ctx, struct visl_run *run) >>> for (i = 0; i < out_q->num_buffers; i++) { >>> char entry[] = "index: %u, state: %s, request_fd: %d, "; >>> u32 old_len = len; >>> - char *q_status = visl_get_vb2_state(out_q->bufs[i]->state); >>> + struct vb2_buffer *vb2; >>> + char *q_status; >>> + >>> + vb2 = vb2_get_buffer(out_q, i); >>> + if (!vb2) >>> + continue; >>> + >>> + q_status = visl_get_vb2_state(vb2->state); >>> >>> len += scnprintf(&buf[len], TPG_STR_BUF_SZ - len, >>> entry, i, q_status, >>> - to_vb2_v4l2_buffer(out_q->bufs[i])->request_fd); >>> + to_vb2_v4l2_buffer(vb2)->request_fd); >>> >>> - len += visl_fill_bytesused(to_vb2_v4l2_buffer(out_q->bufs[i]), >>> + len += visl_fill_bytesused(to_vb2_v4l2_buffer(vb2), >>> &buf[len], >>> TPG_STR_BUF_SZ - len); >>> >>> @@ -342,13 +349,20 @@ static void visl_tpg_fill(struct visl_ctx *ctx, struct visl_run *run) >>> len = 0; >>> for (i = 0; i < cap_q->num_buffers; i++) { >>> u32 old_len = len; >>> - char *q_status = visl_get_vb2_state(cap_q->bufs[i]->state); >>> + struct vb2_buffer *vb2; >>> + char *q_status; >>> + >>> + vb2 = vb2_get_buffer(cap_q, i); >>> + if (!vb2) >>> + continue; >>> + >>> + q_status = visl_get_vb2_state(vb2->state); >>> >>> len += scnprintf(&buf[len], TPG_STR_BUF_SZ - len, >>> "index: %u, status: %s, timestamp: %llu, is_held: %d", >>> - cap_q->bufs[i]->index, q_status, >>> - cap_q->bufs[i]->timestamp, >>> - to_vb2_v4l2_buffer(cap_q->bufs[i])->is_held); >>> + vb2->index, q_status, >>> + vb2->timestamp, >>> + to_vb2_v4l2_buffer(vb2)->is_held); >>> >>> tpg_gen_text(&ctx->tpg, basep, line++ * line_height, 16, &buf[old_len]); >>> frame_dprintk(ctx->dev, run->dst->sequence, "%s", &buf[old_len]); >>> diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c >>> index 384f31fc66c5..8df301ab41ad 100644 >>> --- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c >>> +++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c >>> @@ -1077,7 +1077,7 @@ static int atomisp_dqbuf_wrapper(struct file *file, void *fh, struct v4l2_buffer >>> if (ret) >>> return ret; >>> >>> - vb = pipe->vb_queue.bufs[buf->index]; >>> + vb = vb2_get_buffer(&pipe->vb_queue, buf->index); >>> frame = vb_to_frame(vb); >>> >>> buf->reserved = asd->frame_status[buf->index]; >
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index cf6727d9c81f..42fd3984c2bc 100644 --- a/drivers/media/common/videobuf2/videobuf2-core.c +++ b/drivers/media/common/videobuf2/videobuf2-core.c @@ -359,8 +359,16 @@ static void __setup_offsets(struct vb2_buffer *vb) unsigned long off = 0; if (vb->index) { - struct vb2_buffer *prev = q->bufs[vb->index - 1]; - struct vb2_plane *p = &prev->planes[prev->num_planes - 1]; + struct vb2_buffer *prev; + struct vb2_plane *p; + + prev = vb2_get_buffer(q, vb->index - 1); + if (!prev) { + dprintk(q, 1, "can't find the requested buffer\n"); + return; + } + + p = &prev->planes[prev->num_planes - 1]; off = PAGE_ALIGN(p->m.offset + p->length); } @@ -397,6 +405,37 @@ static void init_buffer_cache_hints(struct vb2_queue *q, struct vb2_buffer *vb) vb->skip_cache_sync_on_finish = 1; } +/** + * vb2_queue_add_buffer() - add a buffer to a queue + * @q: pointer to &struct vb2_queue with videobuf2 queue. + * @vb: pointer to &struct vb2_buffer to be added to the queue. + * @index: index where add vb2_buffer in the queue + */ +static bool vb2_queue_add_buffer(struct vb2_queue *q, struct vb2_buffer *vb, int index) +{ + if (index < VB2_MAX_FRAME && !q->bufs[index]) { + q->bufs[index] = vb; + vb->index = index; + vb->vb2_queue = q; + return true; + } + + return false; +} + +/** + * vb2_queue_remove_buffer() - remove a buffer from a queue + * @q: pointer to &struct vb2_queue with videobuf2 queue. + * @vb: pointer to &struct vb2_buffer to be removed from the queue. + */ +static void vb2_queue_remove_buffer(struct vb2_queue *q, struct vb2_buffer *vb) +{ + if (vb->index < VB2_MAX_FRAME) { + q->bufs[vb->index] = NULL; + vb->vb2_queue = NULL; + } +} + /* * __vb2_queue_alloc() - allocate vb2 buffer structures and (for MMAP type) * video buffer memory for all buffers/planes on the queue and initializes the @@ -425,9 +464,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory, } vb->state = VB2_BUF_STATE_DEQUEUED; - vb->vb2_queue = q; vb->num_planes = num_planes; - vb->index = q->num_buffers + buffer; vb->type = q->type; vb->memory = memory; init_buffer_cache_hints(q, vb); @@ -437,7 +474,11 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory, } call_void_bufop(q, init_buffer, vb); - q->bufs[vb->index] = vb; + if (!vb2_queue_add_buffer(q, vb, q->num_buffers + buffer)) { + dprintk(q, 1, "failed adding buffer %d to queue\n", buffer); + kfree(vb); + break; + } /* Allocate video buffer memory for the MMAP type */ if (memory == VB2_MEMORY_MMAP) { @@ -445,7 +486,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory, if (ret) { dprintk(q, 1, "failed allocating memory for buffer %d\n", buffer); - q->bufs[vb->index] = NULL; + vb2_queue_remove_buffer(q, vb); kfree(vb); break; } @@ -460,7 +501,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory, dprintk(q, 1, "buffer %d %p initialization failed\n", buffer, vb); __vb2_buf_mem_free(vb); - q->bufs[vb->index] = NULL; + vb2_queue_remove_buffer(q, vb); kfree(vb); break; } @@ -483,7 +524,7 @@ static void __vb2_free_mem(struct vb2_queue *q, unsigned int buffers) for (buffer = q->num_buffers - buffers; buffer < q->num_buffers; ++buffer) { - vb = q->bufs[buffer]; + vb = vb2_get_buffer(q, buffer); if (!vb) continue; @@ -511,7 +552,7 @@ static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) /* Call driver-provided cleanup function for each buffer, if provided */ for (buffer = q->num_buffers - buffers; buffer < q->num_buffers; ++buffer) { - struct vb2_buffer *vb = q->bufs[buffer]; + struct vb2_buffer *vb = vb2_get_buffer(q, buffer); if (vb && vb->planes[0].mem_priv) call_void_vb_qop(vb, buf_cleanup, vb); @@ -551,15 +592,20 @@ static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) q->cnt_unprepare_streaming = 0; } for (buffer = 0; buffer < q->num_buffers; ++buffer) { - struct vb2_buffer *vb = q->bufs[buffer]; - bool unbalanced = vb->cnt_mem_alloc != vb->cnt_mem_put || - vb->cnt_mem_prepare != vb->cnt_mem_finish || - vb->cnt_mem_get_userptr != vb->cnt_mem_put_userptr || - vb->cnt_mem_attach_dmabuf != vb->cnt_mem_detach_dmabuf || - vb->cnt_mem_map_dmabuf != vb->cnt_mem_unmap_dmabuf || - vb->cnt_buf_queue != vb->cnt_buf_done || - vb->cnt_buf_prepare != vb->cnt_buf_finish || - vb->cnt_buf_init != vb->cnt_buf_cleanup; + struct vb2_buffer *vb = vb2_get_buffer(q, buffer); + bool unbalanced; + + if (!vb) + continue; + + unbalanced = vb->cnt_mem_alloc != vb->cnt_mem_put || + vb->cnt_mem_prepare != vb->cnt_mem_finish || + vb->cnt_mem_get_userptr != vb->cnt_mem_put_userptr || + vb->cnt_mem_attach_dmabuf != vb->cnt_mem_detach_dmabuf || + vb->cnt_mem_map_dmabuf != vb->cnt_mem_unmap_dmabuf || + vb->cnt_buf_queue != vb->cnt_buf_done || + vb->cnt_buf_prepare != vb->cnt_buf_finish || + vb->cnt_buf_init != vb->cnt_buf_cleanup; if (unbalanced || debug) { pr_info(" counters for queue %p, buffer %d:%s\n", @@ -591,8 +637,13 @@ static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) /* Free vb2 buffers */ for (buffer = q->num_buffers - buffers; buffer < q->num_buffers; ++buffer) { - kfree(q->bufs[buffer]); - q->bufs[buffer] = NULL; + struct vb2_buffer *vb = vb2_get_buffer(q, buffer); + + if (!vb) + continue; + + vb2_queue_remove_buffer(q, vb); + kfree(vb); } q->num_buffers -= buffers; @@ -628,7 +679,12 @@ static bool __buffers_in_use(struct vb2_queue *q) { unsigned int buffer; for (buffer = 0; buffer < q->num_buffers; ++buffer) { - if (vb2_buffer_in_use(q, q->bufs[buffer])) + struct vb2_buffer *vb = vb2_get_buffer(q, buffer); + + if (!vb) + continue; + + if (vb2_buffer_in_use(q, vb)) return true; } return false; @@ -636,7 +692,10 @@ static bool __buffers_in_use(struct vb2_queue *q) void vb2_core_querybuf(struct vb2_queue *q, unsigned int index, void *pb) { - call_void_bufop(q, fill_user_buffer, q->bufs[index], pb); + struct vb2_buffer *vb = vb2_get_buffer(q, index); + + if (vb) + call_void_bufop(q, fill_user_buffer, vb, pb); } EXPORT_SYMBOL_GPL(vb2_core_querybuf); @@ -1547,7 +1606,13 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb) struct vb2_buffer *vb; int ret; - vb = q->bufs[index]; + vb = vb2_get_buffer(q, index); + + if (!vb) { + dprintk(q, 1, "can't find the requested buffer\n"); + return -EINVAL; + } + if (vb->state != VB2_BUF_STATE_DEQUEUED) { dprintk(q, 1, "invalid buffer state %s\n", vb2_state_name(vb->state)); @@ -1618,7 +1683,11 @@ static int vb2_start_streaming(struct vb2_queue *q) * correctly return them to vb2. */ for (i = 0; i < q->num_buffers; ++i) { - vb = q->bufs[i]; + vb = vb2_get_buffer(q, i); + + if (!vb) + continue; + if (vb->state == VB2_BUF_STATE_ACTIVE) vb2_buffer_done(vb, VB2_BUF_STATE_QUEUED); } @@ -1646,7 +1715,12 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb, return -EIO; } - vb = q->bufs[index]; + vb = vb2_get_buffer(q, index); + + if (!vb) { + dprintk(q, 1, "can't find the requested buffer\n"); + return -EINVAL; + } if (!req && vb->state != VB2_BUF_STATE_IN_REQUEST && q->requires_requests) { @@ -2022,12 +2096,18 @@ static void __vb2_queue_cancel(struct vb2_queue *q) * to vb2 in stop_streaming(). */ if (WARN_ON(atomic_read(&q->owned_by_drv_count))) { - for (i = 0; i < q->num_buffers; ++i) - if (q->bufs[i]->state == VB2_BUF_STATE_ACTIVE) { + for (i = 0; i < q->num_buffers; ++i) { + struct vb2_buffer *vb = vb2_get_buffer(q, i); + + if (!vb) + continue; + + if (vb->state == VB2_BUF_STATE_ACTIVE) { pr_warn("driver bug: stop_streaming operation is leaving buf %p in active state\n", - q->bufs[i]); - vb2_buffer_done(q->bufs[i], VB2_BUF_STATE_ERROR); + vb); + vb2_buffer_done(vb, VB2_BUF_STATE_ERROR); } + } /* Must be zero now */ WARN_ON(atomic_read(&q->owned_by_drv_count)); } @@ -2061,9 +2141,14 @@ static void __vb2_queue_cancel(struct vb2_queue *q) * be changed, so we can't move the buf_finish() to __vb2_dqbuf(). */ for (i = 0; i < q->num_buffers; ++i) { - struct vb2_buffer *vb = q->bufs[i]; - struct media_request *req = vb->req_obj.req; + struct vb2_buffer *vb; + struct media_request *req; + vb = vb2_get_buffer(q, i); + if (!vb) + continue; + + req = vb->req_obj.req; /* * If a request is associated with this buffer, then * call buf_request_cancel() to give the driver to complete() @@ -2215,7 +2300,10 @@ static int __find_plane_by_offset(struct vb2_queue *q, unsigned long off, * return its buffer and plane numbers. */ for (buffer = 0; buffer < q->num_buffers; ++buffer) { - vb = q->bufs[buffer]; + vb = vb2_get_buffer(q, buffer); + + if (!vb) + continue; for (plane = 0; plane < vb->num_planes; ++plane) { if (vb->planes[plane].m.offset == off) { @@ -2262,7 +2350,12 @@ int vb2_core_expbuf(struct vb2_queue *q, int *fd, unsigned int type, return -EINVAL; } - vb = q->bufs[index]; + vb = vb2_get_buffer(q, index); + + if (!vb) { + dprintk(q, 1, "can't find the requested buffer\n"); + return -EINVAL; + } if (plane >= vb->num_planes) { dprintk(q, 1, "buffer plane out of range\n"); @@ -2339,7 +2432,13 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma) if (ret) goto unlock; - vb = q->bufs[buffer]; + vb = vb2_get_buffer(q, buffer); + + if (!vb) { + dprintk(q, 1, "can't find the requested buffer\n"); + ret = -EINVAL; + goto unlock; + } /* * MMAP requires page_aligned buffers. @@ -2396,7 +2495,12 @@ unsigned long vb2_get_unmapped_area(struct vb2_queue *q, if (ret) goto unlock; - vb = q->bufs[buffer]; + vb = vb2_get_buffer(q, buffer); + if (!vb) { + dprintk(q, 1, "can't find the requested buffer\n"); + ret = -EINVAL; + goto unlock; + } vaddr = vb2_plane_vaddr(vb, plane); mutex_unlock(&q->mmap_lock); @@ -2625,6 +2729,7 @@ struct vb2_fileio_data { static int __vb2_init_fileio(struct vb2_queue *q, int read) { struct vb2_fileio_data *fileio; + struct vb2_buffer *vb; int i, ret; unsigned int count = 0; @@ -2679,7 +2784,13 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read) * Check if plane_count is correct * (multiplane buffers are not supported). */ - if (q->bufs[0]->num_planes != 1) { + vb = vb2_get_buffer(q, 0); + if (!vb) { + ret = -EBUSY; + goto err_reqbufs; + } + + if (vb->num_planes != 1) { ret = -EBUSY; goto err_reqbufs; } @@ -2688,12 +2799,17 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read) * Get kernel address of each buffer. */ for (i = 0; i < q->num_buffers; i++) { - fileio->bufs[i].vaddr = vb2_plane_vaddr(q->bufs[i], 0); + vb = vb2_get_buffer(q, i); + + if (!vb) + continue; + + fileio->bufs[i].vaddr = vb2_plane_vaddr(vb, 0); if (fileio->bufs[i].vaddr == NULL) { ret = -EINVAL; goto err_reqbufs; } - fileio->bufs[i].size = vb2_plane_size(q->bufs[i], 0); + fileio->bufs[i].size = vb2_plane_size(vb, 0); } /* @@ -2821,15 +2937,18 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ fileio->cur_index = index; buf = &fileio->bufs[index]; - b = q->bufs[index]; + b = vb2_get_buffer(q, index); + + if (!b) + return -EINVAL; /* * Get number of bytes filled by the driver */ buf->pos = 0; buf->queued = 0; - buf->size = read ? vb2_get_plane_payload(q->bufs[index], 0) - : vb2_plane_size(q->bufs[index], 0); + buf->size = read ? vb2_get_plane_payload(b, 0) + : vb2_plane_size(b, 0); /* Compensate for data_offset on read in the multiplanar case. */ if (is_multiplanar && read && b->planes[0].data_offset < buf->size) { @@ -2872,8 +2991,12 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ * Queue next buffer if required. */ if (buf->pos == buf->size || (!read && fileio->write_immediately)) { - struct vb2_buffer *b = q->bufs[index]; + struct vb2_buffer *b = vb2_get_buffer(q, index); + if (!b) { + dprintk(q, 1, "can't find the requested buffer\n"); + return -EINVAL; + } /* * Check if this is the last buffer to read. */ @@ -2899,7 +3022,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ */ buf->pos = 0; buf->queued = 1; - buf->size = vb2_plane_size(q->bufs[index], 0); + buf->size = vb2_plane_size(b, 0); fileio->q_count += 1; /* * If we are queuing up buffers for the first time, then @@ -2970,7 +3093,9 @@ static int vb2_thread(void *data) * Call vb2_dqbuf to get buffer back. */ if (prequeue) { - vb = q->bufs[index++]; + vb = vb2_get_buffer(q, index++); + if (!vb) + continue; prequeue--; } else { call_void_qop(q, wait_finish, q); @@ -2979,7 +3104,7 @@ static int vb2_thread(void *data) call_void_qop(q, wait_prepare, q); dprintk(q, 5, "file io: vb2_dqbuf result: %d\n", ret); if (!ret) - vb = q->bufs[index]; + vb = vb2_get_buffer(q, index); } if (ret || threadio->stop) break; diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index c7a54d82a55e..724135d41f7f 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -383,8 +383,7 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md return -EINVAL; } - if (q->bufs[b->index] == NULL) { - /* Should never happen */ + if (!vb2_get_buffer(q, b->index)) { dprintk(q, 1, "%s: buffer is NULL\n", opname); return -EINVAL; } @@ -394,7 +393,7 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md return -EINVAL; } - vb = q->bufs[b->index]; + vb = vb2_get_buffer(q, b->index); vbuf = to_vb2_v4l2_buffer(vb); ret = __verify_planes_array(vb, b); if (ret) @@ -628,11 +627,18 @@ static const struct vb2_buf_ops v4l2_buf_ops = { struct vb2_buffer *vb2_find_buffer(struct vb2_queue *q, u64 timestamp) { unsigned int i; + struct vb2_buffer *vb2; - for (i = 0; i < q->num_buffers; i++) - if (q->bufs[i]->copied_timestamp && - q->bufs[i]->timestamp == timestamp) - return vb2_get_buffer(q, i); + for (i = 0; i < q->num_buffers; i++) { + vb2 = vb2_get_buffer(q, i); + + if (!vb2) + continue; + + if (vb2->copied_timestamp && + vb2->timestamp == timestamp) + return vb2; + } return NULL; } EXPORT_SYMBOL_GPL(vb2_find_buffer); @@ -664,7 +670,13 @@ int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b) dprintk(q, 1, "buffer index out of range\n"); return -EINVAL; } - vb = q->bufs[b->index]; + vb = vb2_get_buffer(q, b->index); + + if (!vb) { + dprintk(q, 1, "can't find the requested buffer\n"); + return -EINVAL; + } + ret = __verify_planes_array(vb, b); if (!ret) vb2_core_querybuf(q, b->index, b); diff --git a/drivers/media/platform/amphion/vpu_dbg.c b/drivers/media/platform/amphion/vpu_dbg.c index 44b830ae01d8..e2c371c2fde5 100644 --- a/drivers/media/platform/amphion/vpu_dbg.c +++ b/drivers/media/platform/amphion/vpu_dbg.c @@ -133,11 +133,18 @@ static int vpu_dbg_instance(struct seq_file *s, void *data) vq = v4l2_m2m_get_src_vq(inst->fh.m2m_ctx); for (i = 0; i < vq->num_buffers; i++) { - struct vb2_buffer *vb = vq->bufs[i]; - struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb); + struct vb2_buffer *vb; + struct vb2_v4l2_buffer *vbuf; + + vb = vb2_get_buffer(vq, i); + if (!vb) + continue; if (vb->state == VB2_BUF_STATE_DEQUEUED) continue; + + vbuf = to_vb2_v4l2_buffer(vb); + num = scnprintf(str, sizeof(str), "output [%2d] state = %10s, %8s\n", i, vb2_stat_name[vb->state], @@ -148,11 +155,18 @@ static int vpu_dbg_instance(struct seq_file *s, void *data) vq = v4l2_m2m_get_dst_vq(inst->fh.m2m_ctx); for (i = 0; i < vq->num_buffers; i++) { - struct vb2_buffer *vb = vq->bufs[i]; - struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb); + struct vb2_buffer *vb; + struct vb2_v4l2_buffer *vbuf; + + vb = vb2_get_buffer(vq, i); + if (!vb) + continue; if (vb->state == VB2_BUF_STATE_DEQUEUED) continue; + + vbuf = to_vb2_v4l2_buffer(vb); + num = scnprintf(str, sizeof(str), "capture[%2d] state = %10s, %8s\n", i, vb2_stat_name[vb->state], diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c index 0051f372a66c..ea37069ba355 100644 --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c @@ -603,7 +603,11 @@ static int mtk_jpeg_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf) return -EINVAL; } - vb = vq->bufs[buf->index]; + vb = vb2_get_buffer(vq, buf->index); + if (!vb) { + dev_err(ctx->jpeg->dev, "buffer not found\n"); + return -EINVAL; + } jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(vb); jpeg_src_buf->bs_size = buf->m.planes[0].bytesused; diff --git a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c index cf16cf2807f0..6532a69f1fa8 100644 --- a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c +++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c @@ -1701,7 +1701,7 @@ static int vdec_vp9_slice_setup_core_buffer(struct vdec_vp9_slice_instance *inst /* update internal buffer's width/height */ for (i = 0; i < vq->num_buffers; i++) { - if (vb == vq->bufs[i]) { + if (vb == vb2_get_buffer(vq, i)) { instance->dpb[i].width = w; instance->dpb[i].height = h; break; diff --git a/drivers/media/platform/st/sti/hva/hva-v4l2.c b/drivers/media/platform/st/sti/hva/hva-v4l2.c index 3a848ca32a0e..326be09bdb55 100644 --- a/drivers/media/platform/st/sti/hva/hva-v4l2.c +++ b/drivers/media/platform/st/sti/hva/hva-v4l2.c @@ -577,6 +577,10 @@ static int hva_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf) } vb2_buf = vb2_get_buffer(vq, buf->index); + if (!vb2_buf) { + dev_dbg(dev, "%s buffer index %d not found\n", ctx->name, buf->index); + return -EINVAL; + } stream = to_hva_stream(to_vb2_v4l2_buffer(vb2_buf)); stream->bytesused = buf->bytesused; } diff --git a/drivers/media/test-drivers/visl/visl-dec.c b/drivers/media/test-drivers/visl/visl-dec.c index 318d675e5668..ba20ea998d19 100644 --- a/drivers/media/test-drivers/visl/visl-dec.c +++ b/drivers/media/test-drivers/visl/visl-dec.c @@ -290,13 +290,20 @@ static void visl_tpg_fill(struct visl_ctx *ctx, struct visl_run *run) for (i = 0; i < out_q->num_buffers; i++) { char entry[] = "index: %u, state: %s, request_fd: %d, "; u32 old_len = len; - char *q_status = visl_get_vb2_state(out_q->bufs[i]->state); + struct vb2_buffer *vb2; + char *q_status; + + vb2 = vb2_get_buffer(out_q, i); + if (!vb2) + continue; + + q_status = visl_get_vb2_state(vb2->state); len += scnprintf(&buf[len], TPG_STR_BUF_SZ - len, entry, i, q_status, - to_vb2_v4l2_buffer(out_q->bufs[i])->request_fd); + to_vb2_v4l2_buffer(vb2)->request_fd); - len += visl_fill_bytesused(to_vb2_v4l2_buffer(out_q->bufs[i]), + len += visl_fill_bytesused(to_vb2_v4l2_buffer(vb2), &buf[len], TPG_STR_BUF_SZ - len); @@ -342,13 +349,20 @@ static void visl_tpg_fill(struct visl_ctx *ctx, struct visl_run *run) len = 0; for (i = 0; i < cap_q->num_buffers; i++) { u32 old_len = len; - char *q_status = visl_get_vb2_state(cap_q->bufs[i]->state); + struct vb2_buffer *vb2; + char *q_status; + + vb2 = vb2_get_buffer(cap_q, i); + if (!vb2) + continue; + + q_status = visl_get_vb2_state(vb2->state); len += scnprintf(&buf[len], TPG_STR_BUF_SZ - len, "index: %u, status: %s, timestamp: %llu, is_held: %d", - cap_q->bufs[i]->index, q_status, - cap_q->bufs[i]->timestamp, - to_vb2_v4l2_buffer(cap_q->bufs[i])->is_held); + vb2->index, q_status, + vb2->timestamp, + to_vb2_v4l2_buffer(vb2)->is_held); tpg_gen_text(&ctx->tpg, basep, line++ * line_height, 16, &buf[old_len]); frame_dprintk(ctx->dev, run->dst->sequence, "%s", &buf[old_len]); diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c index 384f31fc66c5..8df301ab41ad 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c +++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c @@ -1077,7 +1077,7 @@ static int atomisp_dqbuf_wrapper(struct file *file, void *fh, struct v4l2_buffer if (ret) return ret; - vb = pipe->vb_queue.bufs[buf->index]; + vb = vb2_get_buffer(&pipe->vb_queue, buf->index); frame = vb_to_frame(vb); buf->reserved = asd->frame_status[buf->index];
The first step before changing how vb2 buffers are stored into queue is to avoid direct access to bufs arrays. This patch adds 2 helpers functions to add and remove vb2 buffers from a queue. With these 2 and vb2_get_buffer(), bufs field of struct vb2_queue becomes like a private member of the structure. After each call to vb2_get_buffer() we need to be sure that we get a valid pointer so check the return value of all of them. Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> --- .../media/common/videobuf2/videobuf2-core.c | 215 ++++++++++++++---- .../media/common/videobuf2/videobuf2-v4l2.c | 28 ++- drivers/media/platform/amphion/vpu_dbg.c | 22 +- .../platform/mediatek/jpeg/mtk_jpeg_core.c | 6 +- .../vcodec/vdec/vdec_vp9_req_lat_if.c | 2 +- drivers/media/platform/st/sti/hva/hva-v4l2.c | 4 + drivers/media/test-drivers/visl/visl-dec.c | 28 ++- .../staging/media/atomisp/pci/atomisp_ioctl.c | 2 +- 8 files changed, 240 insertions(+), 67 deletions(-)