[PATCH v7 3/5] dma-buf: Add ioctls to allow userspace to flush

[David Herrmann]

Hi

On Thu, Feb 11, 2016 at 7:08 PM, Ville Syrjälä
<ville.syrjala at linux.intel.com> wrote:
> On Thu, Feb 11, 2016 at 03:54:38PM -0200, Tiago Vignatti wrote:
>>
>> Thanks for reviewing, David. Please take a look in my comments in-line.
>>
>>
>> On 02/09/2016 07:26 AM, David Herrmann wrote:
>> >
>> > On Tue, Dec 22, 2015 at 10:36 PM, Tiago Vignatti
>> > <tiago.vignatti at intel.com> wrote:
>> >> From: Daniel Vetter <daniel.vetter at ffwll.ch>
> <snip>
>> >> +
>> >> +#define DMA_BUF_SYNC_READ      (1 << 0)
>> >> +#define DMA_BUF_SYNC_WRITE     (2 << 0)
>> >> +#define DMA_BUF_SYNC_RW        (DMA_BUF_SYNC_READ | DMA_BUF_SYNC_WRITE)
>> >> +#define DMA_BUF_SYNC_START     (0 << 2)
>> >> +#define DMA_BUF_SYNC_END       (1 << 2)
>> >> +#define DMA_BUF_SYNC_VALID_FLAGS_MASK \
>> >> +       (DMA_BUF_SYNC_RW | DMA_BUF_SYNC_END)
>> >> +
>> >> +#define DMA_BUF_BASE           'b'
>> >> +#define DMA_BUF_IOCTL_SYNC     _IOW(DMA_BUF_BASE, 0, struct dma_buf_sync)
>> >
>> > Why _IOW? A read-only ioctl should be able to call DMA_BUF_SYNC_READ, right?
>>
>> yup. I've changed it to _IOWR now.
>
> AFAICS the ioctl only does copy_from_user() so _IOW seemed perfectly
> correct to me.

AFAIK, the _IOC_DIR() arguments in ioctls specify the mode of the
operation, not the way the arguments are used. So if read(2) was an
ioctl, it would be annotated as _IOC_READ (even though it _writes_
into the passed buffer). write(2) would be annotated as _IOC_WRITE
(even though it _reads_ the buffer). As such, they correspond to the
file-access mode, whether you opened it readable and/or writable.

Anyway, turns out VFS does not verify those. As such, you can specify
whatever you want. I just checked with different existing ioctls
throughout the kernel (`git grep _IOC_DIR`), and they seem to match
what I describe.

But I don't care much. I guess _IORW is fine either way.
David