Re: [PATCH v9] dma-buf: Add ioctls to allow userspace to flush

From: Tiago Vignatti <tiago.vignatti@intel.com>
To: Daniel Vetter <daniel@ffwll.ch>,
	Chris Wilson <chris@chris-wilson.co.uk>,
	dri-devel@lists.freedesktop.org, daniel.thompson@linaro.org,
	marcheu@google.com, daniel.vetter@ffwll.ch,
	thellstrom@vmware.com, jglisse@redhat.com, reveman@google.com,
	Daniel Vetter <daniel.vetter@intel.com>
Subject: Re: [PATCH v9] dma-buf: Add ioctls to allow userspace to flush
Date: Mon, 14 Mar 2016 17:21:10 -0300	[thread overview]
Message-ID: <56E71D36.1060205@intel.com> (raw)
In-Reply-To: <20160305093446.GC18536@phenom.ffwll.local>

On 03/05/2016 06:34 AM, Daniel Vetter wrote:
> On Mon, Feb 29, 2016 at 03:02:09PM +0000, Chris Wilson wrote:
>> On Mon, Feb 29, 2016 at 03:54:19PM +0100, Daniel Vetter wrote:
>>> On Thu, Feb 25, 2016 at 06:01:22PM +0000, Chris Wilson wrote:
>>>> On Thu, Feb 11, 2016 at 08:04:51PM -0200, Tiago Vignatti wrote:
>>>>> +static long dma_buf_ioctl(struct file *file,
>>>>> +			  unsigned int cmd, unsigned long arg)
>>>>> +{
>>>>> +	struct dma_buf *dmabuf;
>>>>> +	struct dma_buf_sync sync;
>>>>> +	enum dma_data_direction direction;
>>>>> +
>>>>> +	dmabuf = file->private_data;
>>>>> +
>>>>> +	switch (cmd) {
>>>>> +	case DMA_BUF_IOCTL_SYNC:
>>>>> +		if (copy_from_user(&sync, (void __user *) arg, sizeof(sync)))
>>>>> +			return -EFAULT;
>>>>> +
>>>>> +		if (sync.flags & ~DMA_BUF_SYNC_VALID_FLAGS_MASK)
>>>>> +			return -EINVAL;
>>>>> +
>>>>> +		switch (sync.flags & DMA_BUF_SYNC_RW) {
>>>>> +		case DMA_BUF_SYNC_READ:
>>>>> +			direction = DMA_FROM_DEVICE;
>>>>> +			break;
>>>>> +		case DMA_BUF_SYNC_WRITE:
>>>>> +			direction = DMA_TO_DEVICE;
>>>>> +			break;
>>>>> +		case DMA_BUF_SYNC_RW:
>>>>> +			direction = DMA_BIDIRECTIONAL;
>>>>> +			break;
>>>>> +		default:
>>>>> +			return -EINVAL;
>>>>> +		}
>>>>> +
>>>>> +		if (sync.flags & DMA_BUF_SYNC_END)
>>>>> +			dma_buf_end_cpu_access(dmabuf, direction);
>>>>> +		else
>>>>> +			dma_buf_begin_cpu_access(dmabuf, direction);
>>>>
>>>> We forgot to report the error back to userspace. Might as well fixup the
>>>> callchain to propagate error from end-cpu-access as well. Found after
>>>> updating igt/gem_concurrent_blit to exercise dmabuf mmaps vs the GPU.
>>>
>>> EINTR? Do we need to make this ABI - I guess so? Tiago, do you have
>>> patches? See drmIoctl() in libdrm for what's needed on the userspace side
>>> if my guess is right.
>>
>> EINTR is the easiest, but conceivably we could also get EIO and
>> currently EAGAIN.
>>
>> I am also seeing some strange timing dependent (i.e. valgrind doesn't
>> show up anything client side and the tests then pass) failures (SIGSEGV,
>> SIGBUS) with !llc.
>
> Tiago, ping. Also probably a gap in igt coverage besides the kernel side.
> -Daniel

Hey guys! I'm back from vacation now. I'll take a look on it in the next 
days and then come back to you.

Tiago

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel