linux-kernel - Re: [PATCH] [media] vb2: map dmabuf for planes on driver queue instead of vidioc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b0c71d30-4392-e404-c41b-923bcb5bcc2a@osg.samsung.com>
Date:	Wed, 20 Jul 2016 10:06:42 -0400
From:	Javier Martinez Canillas <javier@....samsung.com>
To:	Sakari Ailus <sakari.ailus@....fi>
Cc:	linux-kernel@...r.kernel.org,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	Mauro Carvalho Chehab <mchehab@...pensource.com>,
	Kyungmin Park <kyungmin.park@...sung.com>,
	Pawel Osciak <pawel@...iak.com>, linux-media@...r.kernel.org,
	Hans Verkuil <hverkuil@...all.nl>,
	Shuah Khan <shuahkh@....samsung.com>,
	Nicolas Dufresne <nicolas.dufresne@...labora.com>,
	Luis de Bethencourt <luisbg@....samsung.com>
Subject: Re: [PATCH] [media] vb2: map dmabuf for planes on driver queue
 instead of vidioc_qbuf

Hello Sakari,

On 07/20/2016 09:20 AM, Sakari Ailus wrote:
> Hi Javier,
> 
> On Fri, Jul 15, 2016 at 12:26:06PM -0400, Javier Martinez Canillas wrote:
>> The buffer planes' dma-buf are currently mapped when buffers are queued
>> from userspace but it's more appropriate to do the mapping when buffers
>> are queued in the driver since that's when the actual DMA operation are
>> going to happen.
>>
>> Suggested-by: Nicolas Dufresne <nicolas.dufresne@...labora.com>
>> Signed-off-by: Javier Martinez Canillas <javier@....samsung.com>
>>
>> ---
>>
>> Hello,
>>
>> A side effect of this change is that if the dmabuf map fails for some
>> reasons (i.e: a driver using the DMA contig memory allocator but CMA
>> not being enabled), the fail will no longer happen on VIDIOC_QBUF but
>> later (i.e: in VIDIOC_STREAMON).
>>
>> I don't know if that's an issue though but I think is worth mentioning.
> 
> I have the same question has Hans --- why?
>

Yes, sorry for missing this information. Nicolas already explained a little
bit but the context is that I want to add dma-buf fence support to videobuf2,
and currently the dma-buf is unmapped in VIDIOC_DQBUF.

But with dma-buf fence, the idea is to be able to dequeue a buffer even when
the driver has not yet finished processing the buffer. So the dma-buf needs to
be mapped until vb2_buffer_done() when the driver is done processing the vb2,
and is able to signal the pending fence.

Since the unmapping was going to be delayed to vb2_buffer_done(), I thought
it would make sense to also move the mapping closer to when is really going
to be used and that's why I moved it to __enqueue_in_driver() in this patch.

But I didn't know that user-space was using the dma-buf map as a way to know
if the dma-buf will be compatible and fallback to a different streaming I/O
method if that's not the case. So $SUBJECT is wrong if it prevents user-space
to recover gracefully from a dma-buf mapping failure.

In any case, only delaying the unmapping is needed to support fence and doing
the map early in VIDIOC_QBUF is not an issue.

> I rather think we should keep the buffers mapped all the time. That'd
> require a bit of extra from the DMA-BUF framework I suppose, to support
> streaming mappings.
> 

Interesting, I can take a look to this possibility after adding the dma-buf
fence support.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America