From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomasz Stanislawski Subject: Re: [PATCHv8 21/26] v4l: vb2-dma-contig: add reference counting for a device from allocator context Date: Thu, 27 Sep 2012 17:19:05 +0200 Message-ID: <50646E69.8040909@samsung.com> References: <1344958496-9373-1-git-send-email-t.stanislaws@samsung.com> <1344958496-9373-22-git-send-email-t.stanislaws@samsung.com> <10699580.p7WLNaRi24@avalon> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: In-reply-to: <10699580.p7WLNaRi24@avalon> Sender: linux-media-owner@vger.kernel.org To: Laurent Pinchart Cc: linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, airlied@redhat.com, m.szyprowski@samsung.com, kyungmin.park@samsung.com, sumit.semwal@ti.com, daeinki@gmail.com, daniel.vetter@ffwll.ch, robdclark@gmail.com, pawel@osciak.com, linaro-mm-sig@lists.linaro.org, hverkuil@xs4all.nl, remi@remlab.net, subashrp@gmail.com, mchehab@redhat.com, g.liakhovetski@gmx.de, dmitriyz@google.com, s.nawrocki@samsung.com, k.debski@samsung.com List-Id: dri-devel@lists.freedesktop.org Hi Laurent, On 08/15/2012 10:04 PM, Laurent Pinchart wrote: > Hi Tomasz, > > Thanks for the patch. > > On Tuesday 14 August 2012 17:34:51 Tomasz Stanislawski wrote: >> This patch adds taking reference to the device for MMAP buffers. >> >> Such buffers, may be exported using DMABUF mechanism. If the driver that >> created a queue is unloaded then the queue is released, the device might be >> released too. However, buffers cannot be released if they are referenced by >> DMABUF descriptor(s). The device pointer kept in a buffer must be valid for >> the whole buffer's lifetime. Therefore MMAP buffers should take a reference >> to the device to avoid risk of dangling pointers. > > That patch looks good, but that approach won't scale if we ever need a more > complex context that can't be copied. We can ignore that for now, but if we > decide to support "orphan" vb2 buffers, that's an issue we need to be aware of > as it might come back to bite us in the future. > Good news is that vb2-dma-contig knows the structure of the context. Therefore it will always be able to implement a proper 'copy constructor'. One could always add reference counting for contexts but I consider it little over-engineering. > I assume that this patch requires your dmabuf owner patch to fix the orphan > buffer case properly. > The 'dmabuf owner' patch is not required for this patchset. It will only protect 'videobuf2-dma-contig' module from being unloaded while its dmabuf is in use. Regards, Tomasz Stanislawski