[Intel-gfx] [PATCH] drm/i915: Promote invalid buffer alignment.

[Kenneth Graunke]

On Saturday 05 December 2009 00:56:57 Chris Wilson wrote:
> On Fri, 04 Dec 2009 20:34:15 -0800, Eric Anholt <eric at anholt.net> wrote:
> > On Wed,  2 Dec 2009 21:07:37 +0000, Chris Wilson <chris at chris-
wilson.co.uk> wrote:
> > > WARN if somebody tries to pin a buffer with a too lax alignment, and
> > > use the minimum required alignment instead.
> >
> > Does this impact any existing userland code?
> 
> The two callers who can pass an alignment parameter to the kernel are pin
> and execbuffer.
> 
> pin should only be used by root, and is being phased out in the KMS world.
> execbuffer accepts an alignment per relocation from the normal user,
> however libdrm only fills that value with 0.
> 
> Maybe it would be more appropriate to push the error checking to the
> userland boundary and make this a BUG_ON instead? I've seen two cases of
> an invalid alignment reported since enabling tiling for some X Pixmaps:
> 256k and 4k. The 256k was the erroneous hack that we accidentally
> commited, but the source of the 4k invalid alignment is still a mystery.
> -ickle

I'm really uncomfortable with this suggestion.  For one, BUG_ON causes a 
kernel panic, killing the whole system.  This seems rather extreme...even 
with completely dead graphics, it's useful to keep the system alive so one 
can at least ssh in and shut down cleanly.

Also, if I understand correctly, a bad userspace could trigger the BUG_ON. 
Surely the kernel<->userspace interface should be robust enough that, no 
matter what userspace supplies, the kernel will not crash/assert.  Especially 
over bugs that aren't even in the kernel.  What if someone has an old 
userspace that doesn't check, or one with a buggy check?

Warning in the kernel seems reasonable, as does asserting in userspace.

--Kenneth