Re: [PATCH v2 01/11] dma-buf/sync_file: de-stage sync_file

From: Gustavo Padovan
Date: Wed Jan 27 2016 - 15:26:24 EST


Hi Emil,

2016-01-27 Emil Velikov <emil.l.velikov@xxxxxxxxx>:

> Hi Gustavo,
>
> On 27 January 2016 at 17:03, Gustavo Padovan <gustavo@xxxxxxxxxxx> wrote:
> > Hi Maarten,
> >
> > 2016-01-27 Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>:
> >
> >> Hey,
> >>
> >> Op 27-01-16 om 14:30 schreef Gustavo Padovan:
> >> > From: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxxxx>
> >> >
> >> > sync_file is useful to connect one or more fences to the file. The file is
> >> > used by userspace to track fences.
> >> >
> >> > Signed-off-by: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxxxx>
> >> >
> >> Is there a value in keeping the abi unchanged?
> >> If not, then Documentation/ioctl/botching-up-ioctls.txt is worth a read.
> >
> > None from me. I'll look where we can improve the ABI.
> >
> Speaking of ABI... there are a couple of things that rang bells here:
> - In most/all of the kernel a len/size named member variable
> indicates the length of the extra data (zero sized array). While here
> it includes the size of the struct as well.

len in this case is the size of the buffer sent to the kernel,
the total length (including the whole struct) is returned in the ioctl.

> - struct sync_file_info_data::fence_info is of type __u8 yet it is "a
> fence_info struct for every fence in the sync_file". Thus shouldn't
> one use "struct fence_info" as the type ?

Agreed. But I'm currently thinking if we really should keep this ioctl.

Gustavo