Hi On Thu, Feb 11, 2016 at 7:08 PM, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> 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@xxxxxxxxx> wrote: >> >> From: Daniel Vetter <daniel.vetter@xxxxxxxx> > <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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
