From patchwork Thu Aug 22 09:55:21 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 2848146 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 4E418BF546 for ; Thu, 22 Aug 2013 09:55:42 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id DF6FD205AD for ; Thu, 22 Aug 2013 09:55:40 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 0D7C220598 for ; Thu, 22 Aug 2013 09:55:39 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E5B94E7D2B for ; Thu, 22 Aug 2013 02:55:38 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-ea0-f173.google.com (mail-ea0-f173.google.com [209.85.215.173]) by gabe.freedesktop.org (Postfix) with ESMTP id D586CE60C7 for ; Thu, 22 Aug 2013 02:55:14 -0700 (PDT) Received: by mail-ea0-f173.google.com with SMTP id g10so791100eak.32 for ; Thu, 22 Aug 2013 02:55:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id; bh=kWbWASaNfHc1ULOo3vtzMWKd5mBipiGmv5H/1vBs/2Q=; b=Ty4XFlj7oh56J8LzjvlBP2L+0QtSevhMXu1r2NgQ06a61Vvy5zBJytL2fkdgUv0UqW 4BmjlNMU0jT28ohFH8Jk1xmmOmeTRw/yFFzDP7KM0oSAEUX/NiSQQtzy4QmwCdbUF/hr S9VKgOeUGrOjY6fkgjkOxusci4jGuTFxAjHxg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=kWbWASaNfHc1ULOo3vtzMWKd5mBipiGmv5H/1vBs/2Q=; b=Y7o3sij4ySGvUYanZJwDNFHLrhv3JbXC3osXQI21uDN5FTXHHl3OkKqQDzW5NxjSB/ jLrWLb9AwtjUQiFXQH9eSGPdbFqO1tLsRfpvvmdyhV33Aaw8CRQHusnGsVimQw05j7D8 OgzxwmHcYkN3rU7bDr26IWzoJ5j4F15bGhriBIT2n4EKIGlFX7dL8VWYTRhhcs2W4HdE 6GshxGnwpv/HE4kq5116Dh9Mb+++RoAzkLMmPxYIaSZ3+NNF895EjE392ixoCVoHXZoR oiTwXA6NNcziViERkx2S2JjYYSUnU74tSw499ypjVhz4KzzdwvRBjPn5qd1DlX4JpKIy bUjg== X-Gm-Message-State: ALoCoQmoZGAxlC1pXCRCwEzENmb22zwFaG2enFHu6zAW7s2w6ax4z0OWLu8p1bu60ePD/5jpudrT X-Received: by 10.15.45.8 with SMTP id a8mr17568663eew.1.1377165313889; Thu, 22 Aug 2013 02:55:13 -0700 (PDT) Received: from phenom.ffwll.local (178-83-130-250.dynamic.hispeed.ch. [178.83.130.250]) by mx.google.com with ESMTPSA id k7sm16300218eeg.13.1969.12.31.16.00.00 (version=TLSv1.2 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 22 Aug 2013 02:55:13 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Thu, 22 Aug 2013 11:55:21 +0200 Message-Id: <1377165321-29871-1-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 1.8.4.rc3 Cc: Daniel Vetter , Ben Widawsky Subject: [Intel-gfx] [PATCH] drm/i915: More vma fixups around unbind/destroy X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The important bugfix here is that we must not unlink the vma when we keep it around as a placeholder for the execbuf code. Since then we won't find it again when execbuf gets interrupt and restarted and create a 2nd vma. And since the code as-is isn't fit yet to deal with more than one vma, hilarity ensues. Specifically the dma map/unmap of the sg table isn't adjusted for multiple vmas yet and will blow up like this: Aug 15 20:12:37 x-ivb9 kernel: [ 8081.750976] BUG: unable to handle kernel NULL pointer dereference at 0000000000000008 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751021] IP: [] i915_gem_gtt_finish_object+0x73/0xc8 [i915] Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751073] PGD 56bb5067 PUD ad3dd067 PMD 0 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751096] Oops: 0000 [#1] SMP Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751114] Modules linked in: tcp_lp ppdev parport_pc lp parport ipv6 dm_mod dcdbas snd_hda_codec_hdmi pcspkr snd_hda_codec_realtek serio_raw i2c_i801 iTCO_wdt iTCO_vendor_support snd_hda_intel snd_hda_codec lpc_ich snd_hwdep mfd_core snd_pcm snd_page_alloc snd_timer snd soundcore acpi_cpufreq i915 video button drm_kms_helper drm mperf freq_table Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751291] CPU: 1 PID: 16650 Comm: fbo-maxsize Not tainted 3.11.0-rc4_nightlytop_d93f59_debug_20130814_+ #6957 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751334] Hardware name: Dell Inc. OptiPlex 9010/03JR84, BIOS A01 05/04/2012 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751365] task: ffff8800563b3f00 ti: ffff88004bdf4000 task.ti: ffff88004bdf4000 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751396] RIP: 0010:[] [] i915_gem_gtt_finish_object+0x73/0xc8 [i915] Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751446] RSP: 0018:ffff88004bdf5958 EFLAGS: 00010246 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751469] RAX: 0000000000000000 RBX: ffff8801135e0000 RCX: ffff8800ad3bf8e0 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751499] RDX: ffff8800ad3bf8e0 RSI: 0000000000000000 RDI: ffff8801007ee780 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751529] RBP: ffff88004bdf5978 R08: ffff8800ad3bf8e0 R09: 0000000000000000 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751559] R10: ffffffff86ca1810 R11: ffff880036a17101 R12: ffff8801007ee780 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751589] R13: 0000000000018001 R14: ffff880118c4e000 R15: ffff8801007ee780 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751619] FS: 00007f401a0ce740(0000) GS:ffff88011e280000(0000) knlGS:0000000000000000 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751653] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751677] CR2: 0000000000000008 CR3: 000000005635c000 CR4: 00000000001407e0 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751707] Stack: Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751717] ffff8801007ee780 ffff88005c253180 0000000000018000 ffff8801135e0000 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751752] ffff88004bdf59a8 ffffffffa0088e55 0000000000000011 ffff8801007eec00 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751788] 0000000000018000 ffff880036a17101 ffff88004bdf5a08 ffffffffa0089026 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751823] Call Trace: Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751843] [] i915_vma_unbind+0xdf/0x1ab [i915] Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751875] [] __i915_gem_shrink+0x105/0x177 [i915] Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751909] [] i915_gem_object_get_pages_gtt+0x108/0x309 [i915] Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751947] [] i915_gem_object_get_pages+0x61/0x90 [i915] Aug 15 20:12:37 x-ivb9 kernel: [ 8081.751983] [] ? gen6_ppgtt_insert_entries+0x103/0x125 [i915] Aug 15 20:12:37 x-ivb9 kernel: [ 8081.752019] [] i915_gem_object_pin+0x1fa/0x5df [i915] Aug 15 20:12:37 x-ivb9 kernel: [ 8081.752054] [] i915_gem_execbuffer_reserve_object.isra.6+0x8d/0x1bc [i915] Aug 15 20:12:37 x-ivb9 kernel: [ 8081.752094] [] i915_gem_execbuffer_reserve+0x229/0x367 [i915] Aug 15 20:12:37 x-ivb9 kernel: [ 8081.752131] [] i915_gem_do_execbuffer.isra.12+0x4dc/0xf3a [i915] Aug 15 20:12:37 x-ivb9 kernel: [ 8081.752166] [] ? might_fault+0x40/0x90 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.752195] [] i915_gem_execbuffer2+0x187/0x222 [i915] Aug 15 20:12:37 x-ivb9 kernel: [ 8081.752231] [] drm_ioctl+0x308/0x442 [drm] Aug 15 20:12:37 x-ivb9 kernel: [ 8081.752261] [] ? i915_gem_execbuffer+0x3ae/0x3ae [i915] Aug 15 20:12:37 x-ivb9 kernel: [ 8081.752292] [] ? __do_page_fault+0x3dd/0x481 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.752318] [] vfs_ioctl+0x26/0x39 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.752341] [] do_vfs_ioctl+0x40e/0x451 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.752366] [] ? sysret_check+0x1b/0x56 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.752391] [] SyS_ioctl+0x57/0x87 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.752415] [] ? trace_hardirqs_on_thunk+0x3a/0x3f Aug 15 20:12:37 x-ivb9 kernel: [ 8081.752443] [] system_call_fastpath+0x16/0x1b Aug 15 20:12:37 x-ivb9 kernel: [ 8081.752469] Code: 48 c7 c6 84 30 0e a0 31 c0 e8 d0 e9 f7 ff bf c6 a7 00 00 e8 07 af 2c e1 41 f6 84 24 03 01 00 00 10 75 44 49 8b 84 24 08 01 00 00 <8b> 50 08 48 8b 30 49 8b 86 b0 04 00 00 48 89 c7 48 81 c7 98 00 Aug 15 20:12:37 x-ivb9 kernel: [ 8081.752624] RIP [] i915_gem_gtt_finish_object+0x73/0xc8 [i915] Aug 15 20:12:37 x-ivb9 kernel: [ 8081.753750] RSP Aug 15 20:12:37 x-ivb9 kernel: [ 8081.754851] CR2: 0000000000000008 The second thing to correct is the "only one vma for now" check in vma_unbind - since vma_destroy isn't always called the obj->vma_list might not be empty. Instead check that the vma list is singular at the beginning of vma_unbind. This is also more symmetric with bind_to_vm. Our first attempting at fixing this was commit 1be81a2f2cfd8789a627401d470423358fba2d76 Author: Chris Wilson Date: Tue Aug 20 12:56:40 2013 +0100 drm/i915: Don't destroy the vma placeholder during execbuffer reservation Cc: Chris Wilson Cc: Ben Widawsky Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=68298 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=68171 Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ef92c69..3e855ff 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2616,6 +2616,9 @@ int i915_vma_unbind(struct i915_vma *vma) drm_i915_private_t *dev_priv = obj->base.dev->dev_private; int ret; + /* For now we only ever use 1 vma per object */ + WARN_ON(!list_is_singular(&obj->vma_list)); + if (list_empty(&vma->vma_link)) return 0; @@ -2661,12 +2664,12 @@ int i915_vma_unbind(struct i915_vma *vma) drm_mm_remove_node(&vma->node); destroy: - i915_gem_vma_destroy(vma); + /* Keep the vma as a placeholder in the execbuffer reservation lists */ + if (!list_empty(&vma->exec_list)) + i915_gem_vma_destroy(vma); /* Since the unbound list is global, only move to that list if - * no more VMAs exist. - * NB: Until we have real VMAs there will only ever be one */ - WARN_ON(!list_empty(&obj->vma_list)); + * no more VMAs exist. */ if (list_empty(&obj->vma_list)) list_move_tail(&obj->global_list, &dev_priv->mm.unbound_list); @@ -4171,10 +4174,6 @@ void i915_gem_vma_destroy(struct i915_vma *vma) WARN_ON(vma->node.allocated); list_del(&vma->vma_link); - /* Keep the vma as a placeholder in the execbuffer reservation lists */ - if (!list_empty(&vma->exec_list)) - return; - kfree(vma); }