Re: [PATCH 2/2] drm/i915: Disable page-faults around the fast pwrite/pread paths

[Chris Wilson <chris@chris-wilson.co.uk>]

On Sat, 09 Jul 2011 13:24:02 -0700, Keith Packard <keithp@keithp.com> wrote:
> On Sat,  9 Jul 2011 09:38:51 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> 
> > +		/* We have to disable faulting here in case the user address
> > +		 * is really a GTT mapping and so we can not enter
> > +		 * i915_gem_fault() whilst already holding struct_mutex.
> > +		 */
> 
> I would (far, far) rather disallow pread through the GTT
> mapping. There's no credible reason to allow it. Is there some
> reasonably fast way to detect that these addresses are within the GTT
> and just bail?

Something like:

vma = find_vma(current->mm, uaddr);
if (vma->vm_ops == dev->driver->gem_vm_ops)
	return -EINVAL;

I think would do, find_vma() is not necessary cheap though, and there are a
couple of optimisations that we haven't done for pwrite/pread yet to speed
up the transition to the slow path.

> Any performance penalty that serves solely to enable abuse of the
> interface is not reasonable.

The current code generates lockdep OOPSes and inconsistently applies
pagefault_disable along some paths, in particular for 32-bit kernels,
but not others. And the abuse is permitted through the OpenGL
specification, I believe. The offending app is just doing
glBufferData(glMapBuffer()), iiuc;
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre