Re: [PATCH] drm/i915: Make vm eviction uninterruptible

Ben Widawsky
2014-04-09T04:09:21Z

On Tue, Apr 08, 2014 at 07:50:39AM +0100, Chris Wilson wrote:
> On Mon, Apr 07, 2014 at 02:58:34PM -0700, Ben Widawsky wrote:
> > Blocking important fixes for a test case is harmful to customers of our
> > software. I won't argue past that. If you won't take it as is, add it to the
> > JIRA task like you said. I'll carry this one around with my dynamic page table
> > allocations since you essentially can't do any real workloads with full PPGTT
> > without this (assuming you have signals). I'd venture to even say existing
> > tests can hit it with full PPGTT turned on.
> >
> > The following will hit it on the very first iteration:
> > #!/bin/bash
> >
> > while [ 1 ] ; do
> > (glxgears) & pid[0]=$!
> > (glxgears) & pid[1]=$!
> > (glxgears) & pid[2]=$!
> > sleep 3
> > kill ${pid[*]}
> > done
>
> You must agree that this is the fix for the above issue, but just a band
> aid to prevent it exploding upon the user?
> -Chris
>

I agree, IFF any use of mm.interruptible is duct-tape. I *really* agree
with your second, point wrt user explosion. We can argue until we're
blue in the face about paper, and duct tape - the user impact is
undeniable.

Reading optional for the rest:

This is no more or less flagrant than any other use. Evict CANNOT finish
if we get interrupted by a signal. If we can't properly evict everything
from the address space, I can't make any guarantee about anything being
clean when we teardown, full stop. If you want to limit calling evict to
when all vmas are idle, fine, but that's just as lame.

wait_seqno() makes sense as is. Changing that is IMO is not at all
preferable, and way more risky. As I said in the original patch, making
eviction, which generally shouldn't have any failure paths, atomic-esque
(it's just about freeing memory, and address space) makes complete sense
to me. This is something I've been trying really hard to keep in the
dynamic page table allocations as well. If a fini kind of function can
fail, or block, it's useless.

--
Ben Widawsky, Intel Open Source Technology Center