rt,workqueue: Fix blocking call when !TASK_RUNNING splat in rescuer_thread()

[    0.221225] ------------[ cut here ]------------
[    0.221241] WARNING: CPU: 1 PID: 69 at kernel/sched/core.c:7733 __might_sleep+0x7f/0x90()
[    0.221253] do not call blocking ops when !TASK_RUNNING; state=1 set at [<ffffffff8106edc4>] rescuer_thread+0x64/0x340
[    0.221263] Modules linked in:
[    0.221272] CPU: 1 PID: 69 Comm: khelper Not tainted 4.0.0-rt4-lockdep #71
[    0.221273] Hardware name: IBM System x3550 M3 -[7944K3G]-/69Y5698     , BIOS -[D6E150AUS-1.10]- 12/15/2010
[    0.221280]  ffffffff81a2269f ffff8801789a7cc8 ffffffff81605182 0000000000000002
[    0.221282]  ffff8801789a7d18 ffff8801789a7d08 ffffffff81053cda ffff8801789a7d48
[    0.221284]  ffffffff81a23b44 0000000000000394 0000000000000000 0000000000000000
[    0.221284] Call Trace:
[    0.221288]  [<ffffffff81605182>] dump_stack+0x4f/0x9d
[    0.221291]  [<ffffffff81053cda>] warn_slowpath_common+0x8a/0xe0
[    0.221293]  [<ffffffff81053d76>] warn_slowpath_fmt+0x46/0x50
[    0.221295]  [<ffffffff8106edc4>] ? rescuer_thread+0x64/0x340 
[    0.221296]  [<ffffffff8106edc4>] ? rescuer_thread+0x64/0x340
[    0.221298]  [<ffffffff810794ef>] __might_sleep+0x7f/0x90
[    0.221299]  [<ffffffff8106ed60>] ? worker_thread+0x4d0/0x4d0
[    0.221301]  [<ffffffff8160b4d4>] rt_spin_lock+0x24/0x70
[    0.221304]  [<ffffffff8107fddd>] ? migrate_disable+0x7d/0xe0
[    0.221305]  [<ffffffff8106ee09>] rescuer_thread+0xa9/0x340
[    0.221307]  [<ffffffff8106ed60>] ? worker_thread+0x4d0/0x4d0
[    0.221308]  [<ffffffff8106ed60>] ? worker_thread+0x4d0/0x4d0
[    0.221311]  [<ffffffff81073721>] kthread+0xe1/0x100
[    0.221312]  [<ffffffff8160b1e0>] ? _raw_spin_unlock_irq+0x30/0x70 
[    0.221315]  [<ffffffff81073640>] ? __kthread_parkme+0xa0/0xa0
[    0.221317]  [<ffffffff8160be88>] ret_from_fork+0x58/0x90
[    0.221319]  [<ffffffff81073640>] ? __kthread_parkme+0xa0/0xa0
[    0.221320] ---[ end trace 0000000000000001 ]---

---
 kernel/workqueue.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

@@ -2255,8 +2255,6 @@ static int rescuer_thread(void *__rescue
 	 */
 	rescuer->task->flags |= PF_WQ_WORKER;
 repeat:
-	set_current_state(TASK_INTERRUPTIBLE);

DRM folks identified the spots, so use them.

---
 drivers/gpu/drm/i915/i915_irq.c         |    2 ++
 drivers/gpu/drm/radeon/radeon_display.c |    2 ++
 2 files changed, 4 insertions(+)

@@ -812,6 +812,7 @@ static int i915_get_crtc_scanoutpos(stru
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
 	/* preempt_disable_rt() should go right here in PREEMPT_RT patchset. */
+	preempt_disable_rt();
 
 	/* Get optional system timestamp before query. */
 	if (stime)

@@ -863,6 +864,7 @@ static int i915_get_crtc_scanoutpos(stru
 		*etime = ktime_get();
 
 	/* preempt_enable_rt() should go right here in PREEMPT_RT patchset. */
+	preempt_enable_rt();
 
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 

@@ -1849,6 +1849,7 @@ int radeon_get_crtc_scanoutpos(struct dr
 	struct radeon_device *rdev = dev->dev_private;
 
 	/* preempt_disable_rt() should go right here in PREEMPT_RT patchset. */
+	preempt_disable_rt();
 
 	/* Get optional system timestamp before query. */
 	if (stime)

@@ -1941,6 +1942,7 @@ int radeon_get_crtc_scanoutpos(struct dr
 		*etime = ktime_get();
 
 	/* preempt_enable_rt() should go right here in PREEMPT_RT patchset. */
+	preempt_enable_rt();
 
 	/* Decode into vertical and horizontal scanout position. */
 	*vpos = position & 0x1fff;

Let's not do that when snagging an rtmutex.

---
 kernel/locking/lglock.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

@@ -86,7 +86,7 @@ void lg_double_lock(struct lglock *lg, i
 	if (cpu2 < cpu1)
 		swap(cpu1, cpu2);
 
-	preempt_disable();
+	preempt_disable_nort();
 	lock_acquire_shared(&lg->lock_dep_map, 0, 0, NULL, _RET_IP_);
 	lg_do_lock(per_cpu_ptr(lg->lock, cpu1));
 	lg_do_lock(per_cpu_ptr(lg->lock, cpu2));

@@ -97,7 +97,7 @@ void lg_double_unlock(struct lglock *lg,
 	lock_release(&lg->lock_dep_map, 1, _RET_IP_);
 	lg_do_unlock(per_cpu_ptr(lg->lock, cpu1));
 	lg_do_unlock(per_cpu_ptr(lg->lock, cpu2));
-	preempt_enable();
+	preempt_enable_nort();
 }
 
 void lg_global_lock(struct lglock *lg)

[    8.014039] BUG: sleeping function called from invalid context at kernel/locking/rtmutex.c:918
[    8.014041] in_atomic(): 0, irqs_disabled(): 1, pid: 78, name: kworker/u4:4
[    8.014045] CPU: 1 PID: 78 Comm: kworker/u4:4 Not tainted 4.1.7-rt7 #5
[    8.014055] Workqueue: events_unbound async_run_entry_fn
[    8.014059]  0000000000000000 ffff880037153748 ffffffff815f32c9 0000000000000002
[    8.014063]  ffff88013a50e380 ffff880037153768 ffffffff815ef075 ffff8800372c06c8
[    8.014066]  ffff8800372c06c8 ffff880037153778 ffffffff8107c0b3 ffff880037153798
[    8.014067] Call Trace:
[    8.014074]  [<ffffffff815f32c9>] dump_stack+0x4a/0x61
[    8.014078]  [<ffffffff815ef075>] ___might_sleep.part.93+0xe9/0xee
[    8.014082]  [<ffffffff8107c0b3>] ___might_sleep+0x53/0x80
[    8.014086]  [<ffffffff815f9064>] rt_spin_lock+0x24/0x50
[    8.014090]  [<ffffffff8109368b>] prepare_to_wait+0x2b/0xa0
[    8.014152]  [<ffffffffa016c04c>] intel_pipe_update_start+0x17c/0x300 [i915]
[    8.014156]  [<ffffffff81093b40>] ? prepare_to_wait_event+0x120/0x120
[    8.014201]  [<ffffffffa0158f36>] intel_begin_crtc_commit+0x166/0x1e0 [i915]
[    8.014215]  [<ffffffffa00c806d>] drm_atomic_helper_commit_planes+0x5d/0x1a0 [drm_kms_helper]
[    8.014260]  [<ffffffffa0171e9b>] intel_atomic_commit+0xab/0xf0 [i915]
[    8.014288]  [<ffffffffa00654c7>] drm_atomic_commit+0x37/0x60 [drm]
[    8.014298]  [<ffffffffa00c6fcd>] drm_atomic_helper_plane_set_property+0x8d/0xd0 [drm_kms_helper]
[    8.014301]  [<ffffffff815f77d9>] ? __ww_mutex_lock+0x39/0x40
[    8.014319]  [<ffffffffa0053b3d>] drm_mode_plane_set_obj_prop+0x2d/0x90 [drm]
[    8.014328]  [<ffffffffa00c8edb>] restore_fbdev_mode+0x6b/0xf0 [drm_kms_helper]
[    8.014337]  [<ffffffffa00cae49>] drm_fb_helper_restore_fbdev_mode_unlocked+0x29/0x80 [drm_kms_helper]
[    8.014346]  [<ffffffffa00caec2>] drm_fb_helper_set_par+0x22/0x50 [drm_kms_helper]
[    8.014390]  [<ffffffffa016dfba>] intel_fbdev_set_par+0x1a/0x60 [i915]
[    8.014394]  [<ffffffff81327dc4>] fbcon_init+0x4f4/0x580
[    8.014398]  [<ffffffff8139ef4c>] visual_init+0xbc/0x120
[    8.014401]  [<ffffffff813a1623>] do_bind_con_driver+0x163/0x330
[    8.014405]  [<ffffffff813a1b2c>] do_take_over_console+0x11c/0x1c0
[    8.014408]  [<ffffffff813236e3>] do_fbcon_takeover+0x63/0xd0
[    8.014410]  [<ffffffff81328965>] fbcon_event_notify+0x785/0x8d0
[    8.014413]  [<ffffffff8107c12d>] ? __might_sleep+0x4d/0x90
[    8.014416]  [<ffffffff810775fe>] notifier_call_chain+0x4e/0x80
[    8.014419]  [<ffffffff810779cd>] __blocking_notifier_call_chain+0x4d/0x70
[    8.014422]  [<ffffffff81077a06>] blocking_notifier_call_chain+0x16/0x20
[    8.014425]  [<ffffffff8132b48b>] fb_notifier_call_chain+0x1b/0x20
[    8.014428]  [<ffffffff8132d8fa>] register_framebuffer+0x21a/0x350
[    8.014439]  [<ffffffffa00cb164>] drm_fb_helper_initial_config+0x274/0x3e0 [drm_kms_helper]
[    8.014483]  [<ffffffffa016f1cb>] intel_fbdev_initial_config+0x1b/0x20 [i915]
[    8.014486]  [<ffffffff8107912c>] async_run_entry_fn+0x4c/0x160
[    8.014490]  [<ffffffff81070ffa>] process_one_work+0x14a/0x470
[    8.014493]  [<ffffffff81071489>] worker_thread+0x169/0x4c0
[    8.014496]  [<ffffffff81071320>] ? process_one_work+0x470/0x470
[    8.014499]  [<ffffffff81076606>] kthread+0xc6/0xe0
[    8.014502]  [<ffffffff81070000>] ? queue_work_on+0x80/0x110
[    8.014506]  [<ffffffff81076540>] ? kthread_worker_fn+0x1c0/0x1c0

---
 drivers/gpu/drm/i915/intel_sprite.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

@@ -38,6 +38,7 @@
 #include "intel_drv.h"
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
+#include <linux/locallock.h>
 
 static bool
 format_is_yuv(uint32_t format)

@@ -64,6 +65,8 @@ static int usecs_to_scanlines(const stru
 			    1000 * adjusted_mode->crtc_htotal);
 }
 
+static DEFINE_LOCAL_IRQ_LOCK(pipe_update_lock);
+
 /**
  * intel_pipe_update_start() - start update of a set of display registers
  * @crtc: the crtc of which the registers are going to be updated

@@ -96,7 +99,7 @@ void intel_pipe_update_start(struct inte
 	min = vblank_start - usecs_to_scanlines(adjusted_mode, 100);
 	max = vblank_start - 1;
 
-	local_irq_disable();
+	local_lock_irq(pipe_update_lock);
 
 	if (min <= 0 || max <= 0)
 		return;

@@ -126,11 +129,11 @@ void intel_pipe_update_start(struct inte
 			break;
 		}
 
-		local_irq_enable();
+		local_unlock_irq(pipe_update_lock);
 
 		timeout = schedule_timeout(timeout);
 
-		local_irq_disable();
+		local_lock_irq(pipe_update_lock);
 	}
 
 	finish_wait(wq, &wait);

@@ -164,7 +167,7 @@ void intel_pipe_update_end(struct intel_
 
 	trace_i915_pipe_update_end(crtc, end_vbl_count, scanline_end);
 
-	local_irq_enable();
+	local_unlock_irq(pipe_update_lock);
 
 	if (crtc->debug.start_vbl_count &&
 	    crtc->debug.start_vbl_count != end_vbl_count) {

Hmm. That looks wrong. Spinlocks are state preserving on RT. So I need to figure out why that thingy triggers here.

Thanks,

tglx

How do you end up here?

rt_spin_lock()

    might_sleep_no_state_check()
      ___might_sleep()

Can you spot the subtle difference?

Thanks,

tglx

Can to verify that against some less random rt version?

Thanks,

tglx

Yeah, I'll remove the patch from 4.4-rt and see if it still happens.
Patch has been moldering for quite a while.

-Mike

Hohum, elves and gremlins at play I suppose. I think it's safe to say oh, never mind to that one.

-Mike