From: Daniel Vetter <daniel@ffwll.ch>
To: Ben Widawsky <ben@bwidawsk.net>
Cc: Intel GFX <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 25/29] drm/i915: Convert execbuf code to use vmas
Date: Tue, 6 Aug 2013 22:45:26 +0200 [thread overview]
Message-ID: <20130806204526.GK22035@phenom.ffwll.local> (raw)
In-Reply-To: <20130806204308.GJ22035@phenom.ffwll.local>
On Tue, Aug 06, 2013 at 10:43:08PM +0200, Daniel Vetter wrote:
> On Wed, Jul 31, 2013 at 05:00:18PM -0700, Ben Widawsky wrote:
> > This attempts to convert all the execbuf code to speak in vmas. Since
> > the execbuf code is very self contained it was a nice isolated
> > conversion.
> >
> > The meat of the code is about turning eb_objects into eb_vma, and then
> > wiring up the rest of the code to use vmas instead of obj, vm pairs.
> >
> > Unfortunately, to do this, we must move the exec_list link from the obj
> > structure. This list is reused in the eviction code, so we must also
> > modify the eviction code to make this work.
> >
> > v2: Release table lock early, and two a 2 phase vma lookup to avoid
> > having to use a GFP_ATOMIC. (Chris)
> >
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
>
> I think the leaking of preallocated vmas if execbuf fails can blow up:
> 1. We call lookup_or_create and create new vmas, linked into the vma_link
> chain.
> 2. Later on execbuf fails somewhere (for an igt the simplest way is
> probaly to use more buffers than what would fit into the gtt) and we bail
> out.
> -> Note that at this point we leak vmas which are on the vma_link list but
> which have no gtt node allocation.
> 3. Userspace dies in flames (or just quits).
> 4. All buffers get their final unref and we call vma_unbind on each vma,
> even the ones that do not have an allocation.
> 5. hilarity ensues since vma_unbind doesn't bail out if
> drm_mm_node_allocated(vma->node) == false.
>
> We need broken userspace to actually exercise this bug since all normal
> ways for execbuf to bail out involve singals and ioclt restarting. If this
> is a real bug I think we need an igt to exercise it.
Forgotten to mention: Beside that error path the patch looks good. Same
applies to patches 21-24 before.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
next prev parent reply other threads:[~2013-08-06 20:45 UTC|newest]
Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-31 23:59 [PATCH 00/29] Completion of i915 VMAs v2 Ben Widawsky
2013-07-31 23:59 ` [PATCH 01/29] drm/i915: Create an init vm Ben Widawsky
2013-07-31 23:59 ` [PATCH 02/29] drm/i915: Rework drop caches for checkpatch Ben Widawsky
2013-08-03 11:32 ` Chris Wilson
2013-08-03 22:10 ` Ben Widawsky
2013-07-31 23:59 ` [PATCH 03/29] drm/i915: Make proper functions for VMs Ben Widawsky
2013-07-31 23:59 ` [PATCH 04/29] drm/i915: Use bound list for inactive shrink Ben Widawsky
2013-07-31 23:59 ` [PATCH 05/29] drm/i915: Add VM to pin Ben Widawsky
2013-07-31 23:59 ` [PATCH 06/29] drm/i915: Use ggtt_vm to save some typing Ben Widawsky
2013-08-01 0:00 ` [PATCH 07/29] drm/i915: Update describe_obj Ben Widawsky
2013-08-01 0:00 ` [PATCH 08/29] drm/i915: Rework __i915_gem_shrink Ben Widawsky
2013-08-05 8:59 ` Daniel Vetter
2013-08-01 0:00 ` [PATCH 09/29] drm/i915: thread address space through execbuf Ben Widawsky
2013-08-05 9:39 ` Daniel Vetter
2013-08-01 0:00 ` [PATCH 10/29] drm/i915: make caching operate on all address spaces Ben Widawsky
2013-08-05 9:41 ` Daniel Vetter
2013-08-01 0:00 ` [PATCH 11/29] drm/i915: BUG_ON put_pages later Ben Widawsky
2013-08-05 9:42 ` Daniel Vetter
2013-08-01 0:00 ` [PATCH 12/29] drm/i915: make reset&hangcheck code VM aware Ben Widawsky
2013-08-01 0:00 ` [PATCH 13/29] drm/i915: clear domains for all objects on reset Ben Widawsky
2013-08-03 10:59 ` Chris Wilson
2013-08-03 22:24 ` Ben Widawsky
2013-08-05 9:52 ` Daniel Vetter
2013-08-05 16:46 ` [PATCH 13/29] drm/i915: eliminate dead domain clearing " Ben Widawsky
2013-08-05 17:13 ` Daniel Vetter
2013-08-01 0:00 ` [PATCH 14/29] drm/i915: Restore PDEs on gtt restore Ben Widawsky
2013-08-06 18:14 ` Daniel Vetter
2013-08-01 0:00 ` [PATCH 15/29] drm/i915: Improve VMA comments Ben Widawsky
2013-08-01 0:00 ` [PATCH 16/29] drm/i915: Cleanup more of VMA in destroy Ben Widawsky
2013-08-01 0:00 ` [PATCH 17/29] drm/i915: plumb VM into bind/unbind code Ben Widawsky
2013-08-06 18:29 ` Daniel Vetter
2013-08-06 18:54 ` Daniel Vetter
2013-08-01 0:00 ` [PATCH 18/29] drm/i915: Use new bind/unbind in eviction code Ben Widawsky
2013-08-06 18:39 ` Daniel Vetter
2013-08-06 21:27 ` Ben Widawsky
2013-08-06 21:29 ` Daniel Vetter
2013-08-06 22:57 ` Ben Widawsky
2013-08-06 22:59 ` Daniel Vetter
2013-08-06 23:25 ` Ben Widawsky
2013-08-06 23:44 ` Daniel Vetter
2013-08-07 18:24 ` Ben Widawsky
2013-08-01 0:00 ` [PATCH 19/29] drm/i915: turn bound_ggtt checks to bound_any Ben Widawsky
2013-08-03 11:03 ` Chris Wilson
2013-08-03 22:26 ` Ben Widawsky
2013-08-06 18:43 ` Daniel Vetter
2013-08-06 21:29 ` Ben Widawsky
2013-08-01 0:00 ` [PATCH 20/29] drm/i915: Fix up map and fenceable for VMA Ben Widawsky
2013-08-06 19:11 ` Daniel Vetter
2013-08-07 18:37 ` Ben Widawsky
2013-08-07 20:32 ` Daniel Vetter
2013-08-01 0:00 ` [PATCH 21/29] drm/i915: mm_list is per VMA Ben Widawsky
2013-08-06 19:38 ` Daniel Vetter
2013-08-07 0:28 ` Ben Widawsky
2013-08-07 20:52 ` Daniel Vetter
2013-08-08 4:32 ` Ben Widawsky
2013-08-08 6:46 ` Daniel Vetter
2013-08-08 18:10 ` Ben Widawsky
2013-08-01 0:00 ` [PATCH 22/29] drm/i915: Update error capture for VMs Ben Widawsky
2013-08-01 0:00 ` [PATCH 23/29] drm/i915: Add vma to list at creation Ben Widawsky
2013-08-01 0:00 ` [PATCH 24/29] drm/i915: create vmas at execbuf Ben Widawsky
2013-08-07 20:52 ` Daniel Vetter
2013-08-01 0:00 ` [PATCH 25/29] drm/i915: Convert execbuf code to use vmas Ben Widawsky
2013-08-06 20:43 ` Daniel Vetter
2013-08-06 20:45 ` Daniel Vetter [this message]
2013-08-01 0:00 ` [PATCH 26/29] drm/i915: Convert active API to VMA Ben Widawsky
2013-08-06 20:47 ` Daniel Vetter
2013-08-01 0:00 ` [PATCH 27/29] drm/i915: Add bind/unbind object functions to VM Ben Widawsky
2013-08-01 0:00 ` [PATCH 28/29] drm/i915: Use the new vm [un]bind functions Ben Widawsky
2013-08-06 20:58 ` Daniel Vetter
2013-08-01 0:00 ` [PATCH 29/29] drm/i915: eliminate vm->insert_entries() 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=20130806204526.GK22035@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=ben@bwidawsk.net \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.