On 19 March 2016 at 15:39, Daniel Vetter <daniel@xxxxxxxx> wrote:
> On Fri, Mar 18, 2016 at 08:02:39PM +0000, Chris Wilson wrote:
>> Drivers, especially i915.ko, can fail during the initial migration of a
>> dma-buf for CPU access. However, the error code from the driver was not
>> being propagated back to ioctl and so userspace was blissfully ignorant
>> of the failure. Rendering corruption ensues.
>>
>> Whilst fixing the ioctl to return the error code from
>> dma_buf_start_cpu_access(), also do the same for
>> dma_buf_end_cpu_access(). For most drivers, dma_buf_end_cpu_access()
>> cannot fail. i915.ko however, as most drivers would, wants to avoid being
>> uninterruptible (as would be required to guarrantee no failure when
>> flushing the buffer to the device). As userspace already has to handle
>> errors from the SYNC_IOCTL, take advantage of this to be able to restart
>> the syscall across signals.
>>
>> This fixes a coherency issue for i915.ko as well as reducing the
>> uninterruptible hold upon its BKL, the struct_mutex.
>>
>> Fixes commit c11e391da2a8fe973c3c2398452000bed505851e
>> Author: Daniel Vetter <daniel.vetter@xxxxxxxx>
>> Date: Thu Feb 11 20:04:51 2016 -0200
>>
>> dma-buf: Add ioctls to allow userspace to flush
>>
>> Testcase: igt/gem_concurrent_blit/*dmabuf*interruptible
>> Testcase: igt/prime_mmap_coherency/ioctl-errors
>> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
>> Cc: Tiago Vignatti <tiago.vignatti@xxxxxxxxx>
>> Cc: Stéphane Marchesin <marcheu@xxxxxxxxxxxx>
>> Cc: David Herrmann <dh.herrmann@xxxxxxxxx>
>> Cc: Sumit Semwal <sumit.semwal@xxxxxxxxxx>
>> Cc: Daniel Vetter <daniel.vetter@xxxxxxxxx>
>> CC: linux-media@xxxxxxxxxxxxxxx
>> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
>> Cc: linaro-mm-sig@xxxxxxxxxxxxxxxx
>> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>> Cc: devel@xxxxxxxxxxxxxxxxxxxx
>
> Applied to drm-misc, I'll send a pull to Dave the next few days if no one
> screams.
> -Daniel
Thanks for pulling it via drm-misc, Daniel.
Chris, I feel since this is an API change, it also needs an update to
the Documentation file.
With that, and a minor nit below, please feel free to add
Acked-by: Sumit Semwal <sumit.semwal@xxxxxxxxxx>
>
>> ---
>> drivers/dma-buf/dma-buf.c | 17 +++++++++++------
>> drivers/gpu/drm/i915/i915_gem_dmabuf.c | 15 +++++----------
>> drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 5 +++--
>> drivers/gpu/drm/udl/udl_fb.c | 4 ++--
>> drivers/staging/android/ion/ion.c | 6 ++++--
>> include/linux/dma-buf.h | 6 +++---
>> 6 files changed, 28 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index 9810d1df0691..774a60f4309a 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -259,6 +259,7 @@ static long dma_buf_ioctl(struct file *file,
>> struct dma_buf *dmabuf;
>> struct dma_buf_sync sync;
>> enum dma_data_direction direction;
>> + int ret;
>>
>> dmabuf = file->private_data;
>>
>> @@ -285,11 +286,11 @@ static long dma_buf_ioctl(struct file *file,
>> }
>>
>> if (sync.flags & DMA_BUF_SYNC_END)
>> - dma_buf_end_cpu_access(dmabuf, direction);
>> + ret = dma_buf_end_cpu_access(dmabuf, direction);
>> else
>> - dma_buf_begin_cpu_access(dmabuf, direction);
>> + ret = dma_buf_begin_cpu_access(dmabuf, direction);
>>
>> - return 0;
>> + return ret;
>> default:
>> return -ENOTTY;
>> }
>> @@ -613,13 +614,17 @@ EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
>> *
>> * This call must always succeed.
>> */
Perhaps update the above comment to reflect the change as well?
>> -void dma_buf_end_cpu_access(struct dma_buf *dmabuf,
>> - enum dma_data_direction direction)
>> +int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
>> + enum dma_data_direction direction)
>> {
>> + int ret = 0;
>> +
>> WARN_ON(!dmabuf);
>>
>> if (dmabuf->ops->end_cpu_access)
>> - dmabuf->ops->end_cpu_access(dmabuf, direction);
>> + ret = dmabuf->ops->end_cpu_access(dmabuf, direction);
>> +
>> + return ret;
>> }
<< snip>>
Best regards,
Sumit.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
