public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Ben Widawsky <ben@bwidawsk.net>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Daniel Vetter <daniel@ffwll.ch>,
	Ben Widawsky <benjamin.widawsky@intel.com>,
	Intel GFX <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/i915: Make vm eviction uninterruptible
Date: Tue, 8 Apr 2014 21:09:14 -0700	[thread overview]
Message-ID: <20140409040913.GA2271@bwidawsk.net> (raw)
In-Reply-To: <20140408065039.GO8475@nuc-i3427.alporthouse.com>

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

  reply	other threads:[~2014-04-09  4:09 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-05 20:08 [PATCH] drm/i915: Make vm eviction uninterruptible Ben Widawsky
2014-04-05 20:34 ` Chris Wilson
2014-04-06  2:45   ` Ben Widawsky
2014-04-06 18:35     ` Ben Widawsky
2014-04-07  9:42       ` Chris Wilson
2014-04-07 12:15         ` Daniel Vetter
2014-04-07 12:30           ` Chris Wilson
2014-04-07 18:58             ` Ben Widawsky
2014-04-07 21:50               ` Daniel Vetter
2014-04-07 21:58                 ` Ben Widawsky
2014-04-08  6:50                   ` Chris Wilson
2014-04-09  4:09                     ` Ben Widawsky [this message]
2014-04-09  6:16                       ` Chris Wilson
2014-04-08  6:53                   ` Daniel Vetter
2014-04-09  4:11                     ` Ben Widawsky
2014-04-09 12:54                       ` Daniel Vetter
2014-04-07 21:17         ` [PATCH] drm/i915: Make vm eviction uninterruptible in preclose Ben Widawsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140409040913.GA2271@bwidawsk.net \
    --to=ben@bwidawsk.net \
    --cc=benjamin.widawsky@intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=daniel@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox