Hi Hans,
On Monday 01 Aug 2016 12:56:55 Hans Verkuil wrote:
> On 07/27/2016 02:57 PM, Laurent Pinchart wrote:
> > On Wednesday 27 Jul 2016 16:51:47 Kazunori Kobayashi wrote:
> >> Hi,
> >>
> >> I have a question about memory freeing by calling REQBUF(0) before all
> >> the dmabuf fds exported with VIDIOC_EXPBUF are closed.
> >>
> >> In calling REQBUF(0), videobuf2-core returns -EBUSY when the reference
> >> count of a vb2 buffer is more than 1. When dmabuf fds are not exported
> >> (usual V4L2_MEMORY_MMAP case), the check is no problem, but when dmabuf
> >> fds are exported and some of them are not closed (in other words the
> >> references to that memory are left), we cannot succeed in calling
> >> REQBUF(0) despite being able to free the memory after all the references
> >> are dropped.
> >>
> >> Actually REQBUF(0) does not force a vb2 buffer to be freed but decreases
> >> the refcount of it. Also all the vb2 memory allocators that support
> >> dmabuf exporting (dma-contig, dma-sg, vmalloc) implements memory freeing
> >> by release() of dma_buf_ops, so I think there is no need to return -EBUSY
> >> when exporting dmabuf fds.
> >>
> >> Could you please tell me what you think?
> >
> > I think you're right. vb2 allocates the vb2_buffer and the memops-specific
> > structure separately. videobuf2-core.c will free the vb2_buffer instance,
> > but won't touch the memops-specific structure or the buffer memory. Both
> > of these are reference-counted in the memops allocators. We could thus
> > allow REQBUFS(0) to proceed even when buffers have been exported (or at
> > least after fixing the small issues we'll run into, I have a feeling that
> > this is too easy to be true).
> >
> > Hans, Marek, any opinion on this ?
>
> What is the use-case for this? What you are doing here is to either free all
> existing buffers or reallocate buffers. We can decide to rely on
> refcounting, but then you would create a second set of buffers (when
> re-allocating) or leave a lot of unfreed memory behind. That's pretty hard
> on the memory usage.
Speaking of which, we have no way today to really limit memory usage. I wonder
whether we should try to integrate support for resource limits in V4L2.
> I think the EBUSY is there to protect the user against him/herself: i.e.
> don't call this unless you know all refs are closed.
>
> Given the typical large buffersizes we're talking about, I think that EBUSY
> makes sense.
>
> >> The code that I am talking about is in
> >>
> >> drivers/media/v4l2-core/videobuf2-core.c:
> >> if (*count == 0 || q->num_buffers != 0 || q->memory != memory) {
> >> /*
> >> * We already have buffers allocated, so first check if they
> >> * are not in use and can be freed.
> >> */
> >> mutex_lock(&q->mmap_lock);
> >> if (q->memory == VB2_MEMORY_MMAP && __buffers_in_use(q)) {
> >> mutex_unlock(&q->mmap_lock);
> >> dprintk(1, "memory in use, cannot free\n");
> >> return -EBUSY;
> >> }
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
