Op 15-08-17 om 12:02 schreef Daniel Vetter:
> On Tue, Aug 15, 2017 at 11:57:06AM +0200, Maarten Lankhorst wrote:
>> The last part of drm_atomic_check_only is testing whether we need to
>> fail with -EINVAL when modeset is not allowed, but forgets to return
>> the value when atomic_check() fails first.
>>
>> This results in -EDEADLK being replaced by -EINVAL, and the sanity
>> check in drm_modeset_drop_locks kicks in:
>>
>> [ 308.531734] ------------[ cut here ]------------
>> [ 308.531791] WARNING: CPU: 0 PID: 1886 at drivers/gpu/drm/drm_modeset_lock.c:217 drm_modeset_drop_locks+0x33/0xc0 [drm]
>> [ 308.531828] Modules linked in:
>> [ 308.532050] CPU: 0 PID: 1886 Comm: kms_atomic Tainted: G U W 4.13.0-rc5-patser+ #5225
>> [ 308.532082] Hardware name: NUC5i7RYB, BIOS RYBDWi35.86A.0246.2015.0309.1355 03/09/2015
>> [ 308.532124] task: ffff8800cd9dae00 task.stack: ffff8800ca3b8000
>> [ 308.532168] RIP: 0010:drm_modeset_drop_locks+0x33/0xc0 [drm]
>> [ 308.532189] RSP: 0018:ffff8800ca3bf980 EFLAGS: 00010282
>> [ 308.532211] RAX: dffffc0000000000 RBX: ffff8800ca3bfaf8 RCX: 0000000013a171e6
>> [ 308.532235] RDX: 1ffff10019477f69 RSI: ffffffffa8ba4fa0 RDI: ffff8800ca3bfb48
>> [ 308.532258] RBP: ffff8800ca3bf998 R08: 0000000000000000 R09: 0000000000000003
>> [ 308.532281] R10: 0000000079dbe066 R11: 00000000f760b34b R12: 0000000000000001
>> [ 308.532304] R13: dffffc0000000000 R14: 00000000ffffffea R15: ffff880096889680
>> [ 308.532328] FS: 00007ff00959cec0(0000) GS:ffff8800d4e00000(0000) knlGS:0000000000000000
>> [ 308.532359] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [ 308.532380] CR2: 0000000000000008 CR3: 00000000ca2e3000 CR4: 00000000003406f0
>> [ 308.532402] Call Trace:
>> [ 308.532440] drm_mode_atomic_ioctl+0x19fa/0x1c00 [drm]
>> [ 308.532488] ? drm_atomic_set_property+0x1220/0x1220 [drm]
>> [ 308.532565] ? avc_has_extended_perms+0xc39/0xff0
>> [ 308.532593] ? lock_downgrade+0x610/0x610
>> [ 308.532640] ? drm_atomic_set_property+0x1220/0x1220 [drm]
>> [ 308.532680] drm_ioctl_kernel+0x154/0x1a0 [drm]
>> [ 308.532755] drm_ioctl+0x624/0x8f0 [drm]
>> [ 308.532858] ? drm_atomic_set_property+0x1220/0x1220 [drm]
>> [ 308.532976] ? drm_getunique+0x210/0x210 [drm]
>> [ 308.533061] do_vfs_ioctl+0xd92/0xe40
>> [ 308.533121] ? ioctl_preallocate+0x1b0/0x1b0
>> [ 308.533160] ? selinux_capable+0x20/0x20
>> [ 308.533191] ? do_fcntl+0x1b1/0xbf0
>> [ 308.533219] ? kasan_slab_free+0xa2/0xb0
>> [ 308.533249] ? f_getown+0x4b/0xa0
>> [ 308.533278] ? putname+0xcf/0xe0
>> [ 308.533309] ? security_file_ioctl+0x57/0x90
>> [ 308.533342] SyS_ioctl+0x4e/0x80
>> [ 308.533374] entry_SYSCALL_64_fastpath+0x18/0xad
>> [ 308.533405] RIP: 0033:0x7ff00779e4d7
>> [ 308.533431] RSP: 002b:00007fff66a043d8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
>> [ 308.533481] RAX: ffffffffffffffda RBX: 000000e7c7ca5910 RCX: 00007ff00779e4d7
>> [ 308.533560] RDX: 00007fff66a04430 RSI: 00000000c03864bc RDI: 0000000000000003
>> [ 308.533608] RBP: 00007ff007a5fb00 R08: 000000e7c7ca4620 R09: 000000e7c7ca5e60
>> [ 308.533647] R10: 0000000000000001 R11: 0000000000000246 R12: 0000000000000070
>> [ 308.533685] R13: 0000000000000000 R14: 0000000000000000 R15: 000000e7c7ca5930
>> [ 308.533770] Code: ff df 55 48 89 e5 41 55 41 54 53 48 89 fb 48 83 c7
>> 50 48 89 fa 48 c1 ea 03 80 3c 02 00 74 05 e8 94 d4 16 e7 48 83 7b 50 00
>> 74 02 <0f> ff 4c 8d 6b 58 48 b8 00 00 00 00 00 fc ff df 4c 89 ea 48 c1
>> [ 308.534086] ---[ end trace 77f11e53b1df44ad ]---
>>
>> Solve this by adding the missing return.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
>> Testcase: kms_atomic
> I think we want this in drm-misc-fixes with Cc: stable@xxxxxxxxxxxxxxx,
> because we could end up with a EDEADLCK, and getting past that would
> perhaps have changed the modeset to a fast-set or something like that.
>
> Maybe add a few lines to explain that to the commit message, to make it
> clear it's not just a debug check fix, but a real bugfix.
>
> With that:
>
> Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
>
>
>> ---
>> drivers/gpu/drm/drm_atomic.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> index 20fec923333a..471551d2d8f3 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -1631,6 +1631,9 @@ int drm_atomic_check_only(struct drm_atomic_state *state)
>> if (config->funcs->atomic_check)
>> ret = config->funcs->atomic_check(state->dev, state);
>>
>> + if (ret)
>> + return ret;
>> +
>> if (!state->allow_modeset) {
>> for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
>> if (drm_atomic_crtc_needs_modeset(crtc_state)) {
>> @@ -1641,7 +1644,7 @@ int drm_atomic_check_only(struct drm_atomic_state *state)
>> }
>> }
>>
>> - return ret;
>> + return 0;
>> }
>> EXPORT_SYMBOL(drm_atomic_check_only);
>>
>> --
>> 2.11.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Thanks, pushed. :)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
