about mmap dma-buf and sync

[Thomas Hellstrom]

Hi,

On 08/24/2015 07:56 PM, Michael Spang wrote:
> On Mon, Aug 24, 2015 at 1:42 PM, Thomas Hellstrom <thellstrom at vmware.com> wrote:
>> On 08/24/2015 07:12 PM, Daniel Stone wrote:
>>> Hi,
>>>
>>> On 24 August 2015 at 18:10, Thomas Hellstrom <thellstrom at vmware.com> wrote:
>>>> On 08/24/2015 07:04 PM, Daniel Stone wrote:
>>>>> On 24 August 2015 at 17:56, Thomas Hellstrom <thellstrom at vmware.com> wrote:
>>>>>> On 08/24/2015 05:52 PM, Daniel Stone wrote:
>>>>>>> I still don't think this ameliorates the need for batching: consider
>>>>>>> the case where you update two disjoint screen regions and want them
>>>>>>> both flushed. Either you issue two separate sync calls (which can be
>>>>>>> disadvantageous performance-wise on some hardware / setups), or you
>>>>>>> accumulate the regions and only flush later. So either two ioctls (one
>>>>>>> in the style of dirtyfb and one to perform the sync/flush; you can
>>>>>>> shortcut to assume the full buffer was damaged if the former is
>>>>>>> missing), or one like this:
>>>>>>>
>>>>>>> struct dma_buf_sync_2d {
>>>>>>>         enum dma_buf_sync_flags flags;
>>>>>>>
>>>>>>>         __u64 stride_bytes;
>>>>>>>         __u32 bytes_per_pixel;
>>>>>>>         __u32 num_regions;
>>>>>>>
>>>>>>>         struct {
>>>>>>>                 __u64 x;
>>>>>>>                 __u64 y;
>>>>>>>                 __u64 width;
>>>>>>>                 __u64 height;
>>>>>>>         } regions[];
>>>>>>> };
>>>>>> Fine with me, although perhaps bytes_per_pixel is a bit redundant?
>>>>> Redundant how? It's not implicit in stride.
>>>> For flushing purposes, isn't it possible to cover all cases by assuming
>>>> bytes_per_pixel=1? Not that it matters much.
>>> Sure, though in that case best to replace x with line_byte_offset or
>>> something, because otherwise I guarantee you everyone will get it
>>> wrong, and it'll be a pain to track down. Like how I managed to
>>> misread it now. :)
>> OK, yeah you have a point. IMO let's go for your proposal.
>>
>> Tiago, is this OK with you?
> Is there any obstacle to making the above API a new syscall?

I think that whether this API can be accepted as a syscall or not is
really up to Dave and in the end Linus.
What speaks against is that it's not applicable to all file types, only
dma-bufs and also that we might want to extend
it in the future.

>
> The reason we have issues with ioctls is because it's not possible to
> whitelist them properly with seccomp BPF - there's no single namespace
> for ioctls.
>
> If this API is merged as a ioctl, we may not be able to actually use
> it. Which is a bit unfortunate when it has been proposed with the
> chrome renderer use case in mind.

I don't think mmap() on dma-bufs can be accepted as a _generic_ API
without a mandatory sync mechanism, and I think there is a need to
specify a sync api regardless of who is the first user.

Thanks,
Thomas


>
> Michael