Re: Proposed updates and guidelines for MPEG-2, H.264 and H.265 stateless support

[Hans Verkuil <hverkuil-cisco@xs4all.nl>]

On 6/7/19 8:11 AM, Tomasz Figa wrote:
> On Wed, May 22, 2019 at 7:56 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>>> I share the same experience. Bitstream buffers are usually so small that
>>> you can always find a physically contiguous memory region for them and a
>>> memcpy() will be faster than the overhead of getting an IOMMU involved.
>>> This obviously depends on the specific hardware, but there's always some
>>> threshold before which mapping through an IOMMU just doesn't make sense
>>> from a fragmentation and/or performance point of view.
>>>
>>> I wonder, though, if it's not possible to keep userptr buffers around
>>> and avoid the constant mapping/unmapping. If we only performed cache
>>> maintenance on them as necessary, perhaps that could provide a viable,
>>> maybe even good, zero-copy mechanism.
>>
>> The vb2 framework will keep the mapping for a userptr as long as userspace
>> uses the same userptr for every buffer.
>>
>> I.e. the first time a buffer with index I is queued the userptr is mapped.
>> If that buffer is later dequeued and then requeued again with the same
>> userptr the vb2 core will reuse the old mapping. Otherwise it will unmap
>> and map again with the new userptr.
> 
> That's a good point. I forgot that we've been seeing random memory
> corruptions (fortunately of the userptr memory only, not random system
> memory) because of this behavior and carrying a patch in all
> downstream branches to remove this caching.
> 
> I can see that we keep references on the pages that corresponded to
> the user VMA at the time the buffer was queued, but are we guaranteed
> that the list of pages backing that VMA hasn't changed over time?

Since you are seeing memory corruptions, the answer to this is perhaps 'no'?

I think the (quite possibly faulty) reasoning was that while memory is mapped,
userspace can't do a free()/malloc() pair and end up with the same address.

I suspect this might be a wrong assumption, and in that case we're better off
removing this check.

But I'd like to have some confirmation that it is really wrong.

USERPTR isn't used very often, so it wouldn't surprise me if it is buggy.

Regards,

	Hans

> 
>>
>> The same is done for dmabuf, BTW. So if userspace keeps changing dmabuf
>> fds for each buffer, then that is not optimal.
> 
> We could possibly try to search through the other buffers and reuse
> the mapping if there is a match?
> 
> Best regards,
> Tomasz
>