Skip to content

Commit 18b862d

Browse files
ickledanvet
ickle
authored and
danvet
committed
dma-buf, drm, ion: Propagate error code from dma_buf_start_cpu_access()
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 c11e391 Author: Daniel Vetter <daniel.vetter@ffwll.ch> 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@chris-wilson.co.uk> Cc: Tiago Vignatti <tiago.vignatti@intel.com> Cc: Stéphane Marchesin <marcheu@chromium.org> Cc: David Herrmann <dh.herrmann@gmail.com> Cc: Sumit Semwal <sumit.semwal@linaro.org> Cc: Daniel Vetter <daniel.vetter@intel.com> CC: linux-media@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linaro-mm-sig@lists.linaro.org Cc: intel-gfx@lists.freedesktop.org Cc: devel@driverdev.osuosl.org Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/1458331359-2634-1-git-send-email-chris@chris-wilson.co.uk
1 parent b47ff7e commit 18b862d

File tree

6 files changed

+28
-25
lines changed

6 files changed

+28
-25
lines changed

‎drivers/dma-buf/dma-buf.c

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,7 @@ static long dma_buf_ioctl(struct file *file,
259259
struct dma_buf *dmabuf;
260260
struct dma_buf_sync sync;
261261
enum dma_data_direction direction;
262+
int ret;
262263

263264
dmabuf = file->private_data;
264265

@@ -285,11 +286,11 @@ static long dma_buf_ioctl(struct file *file,
285286
}
286287

287288
if (sync.flags & DMA_BUF_SYNC_END)
288-
dma_buf_end_cpu_access(dmabuf, direction);
289+
ret = dma_buf_end_cpu_access(dmabuf, direction);
289290
else
290-
dma_buf_begin_cpu_access(dmabuf, direction);
291+
ret = dma_buf_begin_cpu_access(dmabuf, direction);
291292

292-
return 0;
293+
return ret;
293294
default:
294295
return -ENOTTY;
295296
}
@@ -613,13 +614,17 @@ EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
613614
*
614615
* This call must always succeed.
615616
*/
616-
void dma_buf_end_cpu_access(struct dma_buf *dmabuf,
617-
enum dma_data_direction direction)
617+
int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
618+
enum dma_data_direction direction)
618619
{
620+
int ret = 0;
621+
619622
WARN_ON(!dmabuf);
620623

621624
if (dmabuf->ops->end_cpu_access)
622-
dmabuf->ops->end_cpu_access(dmabuf, direction);
625+
ret = dmabuf->ops->end_cpu_access(dmabuf, direction);
626+
627+
return ret;
623628
}
624629
EXPORT_SYMBOL_GPL(dma_buf_end_cpu_access);
625630

‎drivers/gpu/drm/i915/i915_gem_dmabuf.c

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -228,25 +228,20 @@ static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, enum dma_data_dire
228228
return ret;
229229
}
230230

231-
static void i915_gem_end_cpu_access(struct dma_buf *dma_buf, enum dma_data_direction direction)
231+
static int i915_gem_end_cpu_access(struct dma_buf *dma_buf, enum dma_data_direction direction)
232232
{
233233
struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
234234
struct drm_device *dev = obj->base.dev;
235-
struct drm_i915_private *dev_priv = to_i915(dev);
236-
bool was_interruptible;
237235
int ret;
238236

239-
mutex_lock(&dev->struct_mutex);
240-
was_interruptible = dev_priv->mm.interruptible;
241-
dev_priv->mm.interruptible = false;
237+
ret = i915_mutex_lock_interruptible(dev);
238+
if (ret)
239+
return ret;
242240

243241
ret = i915_gem_object_set_to_gtt_domain(obj, false);
244-
245-
dev_priv->mm.interruptible = was_interruptible;
246242
mutex_unlock(&dev->struct_mutex);
247243

248-
if (unlikely(ret))
249-
DRM_ERROR("unable to flush buffer following CPU access; rendering may be corrupt\n");
244+
return ret;
250245
}
251246

252247
static const struct dma_buf_ops i915_dmabuf_ops = {

‎drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,11 +93,12 @@ static int omap_gem_dmabuf_begin_cpu_access(struct dma_buf *buffer,
9393
return omap_gem_get_pages(obj, &pages, true);
9494
}
9595

96-
static void omap_gem_dmabuf_end_cpu_access(struct dma_buf *buffer,
97-
enum dma_data_direction dir)
96+
static int omap_gem_dmabuf_end_cpu_access(struct dma_buf *buffer,
97+
enum dma_data_direction dir)
9898
{
9999
struct drm_gem_object *obj = buffer->priv;
100100
omap_gem_put_pages(obj);
101+
return 0;
101102
}
102103

103104

‎drivers/gpu/drm/udl/udl_fb.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -423,8 +423,8 @@ static int udl_user_framebuffer_dirty(struct drm_framebuffer *fb,
423423
}
424424

425425
if (ufb->obj->base.import_attach) {
426-
dma_buf_end_cpu_access(ufb->obj->base.import_attach->dmabuf,
427-
DMA_FROM_DEVICE);
426+
ret = dma_buf_end_cpu_access(ufb->obj->base.import_attach->dmabuf,
427+
DMA_FROM_DEVICE);
428428
}
429429

430430
unlock:

‎drivers/staging/android/ion/ion.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1075,14 +1075,16 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
10751075
return PTR_ERR_OR_ZERO(vaddr);
10761076
}
10771077

1078-
static void ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf,
1079-
enum dma_data_direction direction)
1078+
static int ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf,
1079+
enum dma_data_direction direction)
10801080
{
10811081
struct ion_buffer *buffer = dmabuf->priv;
10821082

10831083
mutex_lock(&buffer->lock);
10841084
ion_buffer_kmap_put(buffer);
10851085
mutex_unlock(&buffer->lock);
1086+
1087+
return 0;
10861088
}
10871089

10881090
static struct dma_buf_ops dma_buf_ops = {

‎include/linux/dma-buf.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ struct dma_buf_ops {
9494
void (*release)(struct dma_buf *);
9595

9696
int (*begin_cpu_access)(struct dma_buf *, enum dma_data_direction);
97-
void (*end_cpu_access)(struct dma_buf *, enum dma_data_direction);
97+
int (*end_cpu_access)(struct dma_buf *, enum dma_data_direction);
9898
void *(*kmap_atomic)(struct dma_buf *, unsigned long);
9999
void (*kunmap_atomic)(struct dma_buf *, unsigned long, void *);
100100
void *(*kmap)(struct dma_buf *, unsigned long);
@@ -224,8 +224,8 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *, struct sg_table *,
224224
enum dma_data_direction);
225225
int dma_buf_begin_cpu_access(struct dma_buf *dma_buf,
226226
enum dma_data_direction dir);
227-
void dma_buf_end_cpu_access(struct dma_buf *dma_buf,
228-
enum dma_data_direction dir);
227+
int dma_buf_end_cpu_access(struct dma_buf *dma_buf,
228+
enum dma_data_direction dir);
229229
void *dma_buf_kmap_atomic(struct dma_buf *, unsigned long);
230230
void dma_buf_kunmap_atomic(struct dma_buf *, unsigned long, void *);
231231
void *dma_buf_kmap(struct dma_buf *, unsigned long);

0 commit comments

Comments
 (0)