Re: [git pull] drm fixes

Linus Torvalds
2015-03-02T01:59:55Z

[-- Attachment #1: Type: text/plain, Size: 1497 bytes --]

On Sun, Mar 1, 2015 at 1:00 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Back to the drawing board.

Ok, many hours later, but I found it.

The bisection was a disaster, having to work around other bugs in this
area, but it ended up getting "close enough" that I figured out what
went wrong.

The "intel_plane_duplicate_state()" is horribly horribly buggy. It
looks at the state->fb pointer, but it may have been free'd already.

This workaround "works for me", but it's really still very
questionable, because while the "kref_get_unless_zero()" works
correctly when the last reference has been dropped, I'm not sure that
there is any guarantee that the whole allocation even exists any more,
so I think the *correct* thing to do would be to clear state->fb when
dropping the kref. But this was the smallest working patch I could
come up with. Somebody who actually knows the code should start
looking at the places that do drm_framebuffer_unreference(), and
actually clear that pointer instead.

Added Matt Roper and Ander Conselvan de Oliveira to the discussion,
since they are the ones git says are involved with the original broken
intel_plane_duplicate_state().

Anyway, attached is

(a) the patch with a big comment

(b) the warnings I get on that machine that show where this problem
triggers (and another warning earlier).

Comments? I'm sure this probably only triggers with *old* X servers
that don't do all the modern dri stuff.

Linus

[-- Attachment #2: 0001-Workaround-for-drm-bug.patch --]
[-- Type: text/x-patch, Size: 1424 bytes --]

From c182b15c3abee75cdc9d9564b6ab826403690f4e Mon Sep 17 00:00:00 2001
From: Linus Torvalds <torvalds@localhost.localdomain>
Date: Sat, 28 Feb 2015 21:44:48 -0800
Subject: [PATCH] Workaround for drm bug

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

---
drivers/gpu/drm/i915/intel_atomic_plane.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 9e6f727..72714d3 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -85,8 +85,23 @@ intel_plane_duplicate_state(struct drm_plane *plane)
return NULL;

state = &intel_state->base;
- if (state->fb)
- drm_framebuffer_reference(state->fb);
+
+ /*
+ * We cannot do drm_framebuffer_reference(), because the reference
+ * may already have been dropped.
+ *
+ * So we do what drm_framebuffer_lookup() does, namely do a
+ * kref_get_unless_zero(). Even that is somewhat questionable,
+ * in that maybe the 'fb' already got free'd. So warn loudly
+ * about this.
+ *
+ * Maybe the base.fb should be cleared by whatever drops the
+ * reference?
+ */
+ if (state->fb && !kref_get_unless_zero(&state->fb->refcount)) {
+ state->fb = NULL;
+ WARN_ONCE(1, "intel_plane_duplicate_state got plane with dead frame buffer");
+ }

return state;
}
--
2.3.1.167.g7f4ba4b

[-- Attachment #3: drm-bug-dmesg --]
[-- Type: application/octet-stream, Size: 6501 bytes --]

...
[ 0.898565] random: nonblocking pool is initialized
[ 0.932621] [drm] Setting output timings on SDVOB failed
[ 1.082274] ------------[ cut here ]------------
[ 1.082283] WARNING: CPU: 0 PID: 15 at drivers/gpu/drm/i915/i915_gem.c:4524 i915_gem_free_object+0x22d/0x250()
[ 1.082291] WARN_ON(obj->frontbuffer_bits)
[ 1.082291] CPU: 0 PID: 15 Comm: kworker/u4:1 Tainted: G W 4.0.0-rc1-00154-g7c7766e #39
[ 1.082293] Hardware name: Apple Computer, Inc. Macmini1,1/Mac-F4208EC8, BIOS MM11.88Z.0055.B03.0604071521 04/07/06
[ 1.082299] Workqueue: events_unbound async_run_entry_fn
[ 1.082304] 00000000 00000000 f68cfb64 c166d337 c17ee3c4 f68cfb94 c103e3bf c17ee764
[ 1.082309] f68cfbc0 0000000f c17ee3c4 000011ac c12fe52d c12fe52d f69e0d00 00000000
[ 1.082313] f69e0d30 f68cfbac c103e43e 00000009 f68cfba4 c17ee764 f68cfbc0 f68cfbd8
[ 1.082314] Call Trace:
[ 1.082321] [<c166d337>] dump_stack+0x41/0x52
[ 1.082328] [<c103e3bf>] warn_slowpath_common+0x7f/0xb0
[ 1.082331] [<c12fe52d>] ? i915_gem_free_object+0x22d/0x250
[ 1.082334] [<c12fe52d>] ? i915_gem_free_object+0x22d/0x250
[ 1.082337] [<c103e43e>] warn_slowpath_fmt+0x2e/0x30
[ 1.082340] [<c12fe52d>] i915_gem_free_object+0x22d/0x250
[ 1.082344] [<c166ed04>] ? mutex_lock+0x14/0x40
[ 1.082348] [<c12bafea>] drm_gem_object_free+0x1a/0x20
[ 1.082353] [<c1326d1d>] intel_user_framebuffer_destroy+0x5d/0x80
[ 1.082356] [<c166ed04>] ? mutex_lock+0x14/0x40
[ 1.082360] [<c12c3f33>] drm_framebuffer_free+0x43/0x60
[ 1.082365] [<c12c4588>] drm_framebuffer_unreference+0x28/0x60
[ 1.082369] [<c12c5c7e>] drm_mode_set_config_internal+0x8e/0xc0
[ 1.082371] [<c12b4e32>] restore_fbdev_mode+0xc2/0xf0
[ 1.082375] [<c12d3276>] ? __drm_modeset_lock_all+0xa6/0xe0
[ 1.082379] [<c12b7314>] drm_fb_helper_restore_fbdev_mode_unlocked+0x24/0x70
[ 1.082382] [<c12b737d>] drm_fb_helper_set_par+0x1d/0x40
[ 1.082386] [<c13452c8>] intel_fbdev_set_par+0x18/0x60
[ 1.082390] [<c1232b72>] fbcon_init+0x4e2/0x530
[ 1.082395] [<c115c230>] ? __kernfs_create_file+0x60/0x90
[ 1.082400] [<c129068e>] do_bind_con_driver+0x15e/0x330
[ 1.082403] [<c115cac7>] ? sysfs_create_file_ns+0x27/0x30
[ 1.082406] [<c136ecbf>] ? device_create_file+0x2f/0xa0
[ 1.082411] [<c1291a15>] do_take_over_console+0xd5/0x160
[ 1.082414] [<c1232c1d>] do_fbcon_takeover+0x5d/0xc0
[ 1.082418] [<c123757d>] fbcon_event_notify+0x5dd/0x760
[ 1.082421] [<c115c8fc>] ? sysfs_add_file_mode_ns+0x6c/0x1e0
[ 1.082424] [<c1054fe1>] notifier_call_chain+0x41/0x60
[ 1.082427] [<c10552b6>] __blocking_notifier_call_chain+0x46/0x60
[ 1.082430] [<c10552ea>] blocking_notifier_call_chain+0x1a/0x20
[ 1.082432] [<c123c501>] fb_notifier_call_chain+0x11/0x20
[ 1.082435] [<c123ebf2>] register_framebuffer+0x1f2/0x2f0
[ 1.082439] [<c12b713a>] drm_fb_helper_initial_config+0x2aa/0x370
[ 1.082443] [<c1345d14>] intel_fbdev_initial_config+0x14/0x20
[ 1.082445] [<c10565a1>] async_run_entry_fn+0x31/0xd0
[ 1.082452] [<c104f6c9>] ? pwq_dec_nr_in_flight+0x39/0x90
[ 1.082455] [<c104f80e>] process_one_work+0xee/0x2b0
[ 1.082458] [<c104fa39>] worker_thread+0x39/0x400
[ 1.082461] [<c105427c>] kthread+0xac/0xd0
[ 1.082464] [<c104fa00>] ? process_scheduled_works+0x30/0x30
[ 1.082467] [<c1670781>] ret_from_kernel_thread+0x21/0x30
[ 1.082470] [<c10541d0>] ? __kthread_parkme+0x60/0x60
[ 1.082472] ---[ end trace 6309eba3c7a2324a ]---
[ 1.094040] Console: switching to colour frame buffer device 210x65
...
[ 2.903705] setfont (1128) used greatest stack depth: 6448 bytes left
[ 2.915551] ------------[ cut here ]------------
[ 2.917985] WARNING: CPU: 1 PID: 1126 at drivers/gpu/drm/i915/intel_atomic_plane.c:103 intel_plane_duplicate_state+0xc9/0xe0()
[ 2.920518] intel_plane_duplicate_state got plane with dead frame buffer
[ 2.920580] CPU: 1 PID: 1126 Comm: plymouthd Tainted: G W 4.0.0-rc1-00154-g7c7766e #39
[ 2.925701] Hardware name: Apple Computer, Inc. Macmini1,1/Mac-F4208EC8, BIOS MM11.88Z.0055.B03.0604071521 04/07/06
[ 2.928344] 00000000 00000000 f628ba4c c166d337 c17f5fd0 f628ba7c c103e3bf c17f5ffc
[ 2.931048] f628baa8 00000466 c17f5fd0 00000067 c1348ae9 c1348ae9 f6b47f04 f6ba5480
[ 2.933771] f6b47380 f628ba94 c103e43e 00000009 f628ba8c c17f5ffc f628baa8 f628bab4
[ 2.936491] Call Trace:
[ 2.939141] [<c166d337>] dump_stack+0x41/0x52
[ 2.941776] [<c103e3bf>] warn_slowpath_common+0x7f/0xb0
[ 2.944445] [<c1348ae9>] ? intel_plane_duplicate_state+0xc9/0xe0
[ 2.947117] [<c1348ae9>] ? intel_plane_duplicate_state+0xc9/0xe0
[ 2.949800] [<c103e43e>] warn_slowpath_fmt+0x2e/0x30
[ 2.952480] [<c1348ae9>] intel_plane_duplicate_state+0xc9/0xe0
[ 2.955192] [<c12ae154>] drm_plane_helper_update+0x24/0xc0
[ 2.957927] [<c1330d5c>] __intel_set_mode+0x71c/0x9c0
[ 2.960635] [<c13366cb>] intel_set_mode+0x6b/0x90
[ 2.963342] [<c1336b12>] intel_get_load_detect_pipe+0x332/0x470
[ 2.966026] [<c11fffff>] ? sg_last+0x2f/0x40
[ 2.968698] [<c11fb85a>] ? snprintf+0x1a/0x20
[ 2.971334] [<c12caf24>] ? drm_mode_set_name+0x44/0x50
[ 2.973968] [<c1367b24>] intel_tv_detect+0x84/0x4e0
[ 2.976599] [<c105fed3>] ? update_cfs_rq_blocked_load+0x123/0x170
[ 2.979249] [<c12ad573>] drm_helper_probe_single_connector_modes_merge_bits+0x1a3/0x440
[ 2.981936] [<c166ed04>] ? mutex_lock+0x14/0x40
[ 2.993074] [<c12ad842>] drm_helper_probe_single_connector_modes+0x12/0x20
[ 2.995760] [<c12c7db7>] drm_mode_getconnector+0x2e7/0x320
[ 2.998431] [<c1038d72>] ? kmap_atomic_prot+0x42/0x110
[ 3.001086] [<c12bc6f5>] drm_ioctl+0x1e5/0x560
[ 3.003725] [<c12c7ad0>] ? get_properties+0xd0/0xd0
[ 3.006376] [<c10ea55b>] ? page_add_new_anon_rmap+0x5b/0x70
[ 3.009000] [<c11a3e3a>] ? avc_has_perm+0x2a/0x110
[ 3.011614] [<c10e25a1>] ? handle_mm_fault+0xcf1/0xea0
[ 3.014189] [<c12bc510>] ? drm_setversion+0x170/0x170
[ 3.016766] [<c110fd21>] do_vfs_ioctl+0x71/0x590
[ 3.019327] [<c11a8f3c>] ? inode_has_perm.clone.37+0x2c/0x40
[ 3.021877] [<c11a8fe1>] ? file_has_perm+0x91/0xa0
[ 3.024430] [<c11aa7e2>] ? selinux_file_ioctl+0x42/0xe0
[ 3.026978] [<c11102d2>] SyS_ioctl+0x92/0xa0
[ 3.029532] [<c1670912>] syscall_call+0x7/0x7
[ 3.032058] ---[ end trace 6309eba3c7a2324c ]---
[ 3.102207] usb 1-6.1: new full-speed USB device number 5 using ehci-pci
..