All of lore.kernel.org
 help / color / mirror / Atom feed
* [BUG] dw_wdt watchdog on linux-rt 4.18.5-rt4 not triggering
@ 2018-09-18 13:21 Steffen Trumtrar
  2018-09-18 13:46 ` Guenter Roeck
  2018-09-18 18:14 ` Julia Cartwright
  0 siblings, 2 replies; 27+ messages in thread
From: Steffen Trumtrar @ 2018-09-18 13:21 UTC (permalink / raw)
  To: linux-watchdog
  Cc: Wim Van Sebroeck, Guenter Roeck, Christophe Leroy, linux-rt-users


Hi all!

I'm seeing an issue with the dw_wdt watchdog on the SoCFPGA ARM 
platform with the latest linux-rt v4.18.5-rt4. Actually I seem to 
have the same problem, that these patches try to fix:

  38a1222ae4f364d5bd5221fe305dbb0889f45d15
  Author:     Christophe Leroy <christophe.leroy@c-s.fr>
  AuthorDate: Fri Dec 8 11:18:35 2017 +0100
  Commit:     Wim Van Sebroeck <wim@iguana.be>
  CommitDate: Thu Dec 28 20:45:57 2017 +0100

  Follows:    v4.15-rc3 (345)
  Precedes:   v4.16-rc1 (13997)

  watchdog: core: make sure the watchdog worker always works

  When running a command like 'chrt -f 50 dd if=/dev/zero 
  of=/dev/null',
  the watchdog_worker fails to service the HW watchdog and the
  HW watchdog fires long before the watchdog soft timeout.

  At the moment, the watchdog_worker is invoked as a delayed work.
  Delayed works are handled by non realtime kernel threads. The
  WQ_HIGHPRI flag only increases the niceness of that threads.

  This patch replaces the delayed work logic by kthread delayed 
  work,
  and sets the associated kernel task to SCHED_FIFO with the 
  highest
  priority, in order to ensure that the watchdog worker will run 
  as
  soon as possible.


  1ff688209e2ed23f699269b9733993e2ce123fd2
  Author:     Christophe Leroy <christophe.leroy@c-s.fr>
  AuthorDate: Thu Jan 18 12:11:21 2018 +0100
  Commit:     Wim Van Sebroeck <wim@iguana.be>
  CommitDate: Sun Jan 21 12:44:59 2018 +0100

  Follows:    v4.15-rc3 (349)
  Precedes:   v4.16-rc1 (13993)

  watchdog: core: make sure the watchdog_worker is not deferred

  commit 4cd13c21b207e ("softirq: Let ksoftirqd do its job") has 
  the
  effect of deferring timer handling in case of high CPU load, 
  hence
  delaying the delayed work allthought the worker is running which
  high realtime priority.

  As hrtimers are not managed by softirqs, this patch replaces the
  delayed work by a plain work and uses an hrtimer to schedule 
  that work.


If I run the same test or 'chrt 50 hackbench 20 -l 150' or any 
task where I change the prio with chrt and that runs long enough, 
I get a system reset from the watchdog because it times out. This 
only happens if the watchdog is already enabled on boot and 
CONFIG_PREEMPT_RT_FULL is set.

Any idea if I'm missing something essential? If I understand it 
correctly, the two commits fix the framework and therefore the 
dw_wdt driver doesn't need any updates.


Best regards,
Steffen

--
Pengutronix e.K.                          | Steffen Trumtrar 
|
Industrial Linux Solutions                | 
http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany| Phone: 
+49-5121-206917-0   |
Amtsgericht Hildesheim, HRA 2686          | Fax: 
+49-5121-206917-5555|

^ permalink raw reply	[flat|nested] 27+ messages in thread
* [PATCH 1/2] kthread: convert worker lock to raw spinlock
@ 2019-02-12 16:25 Sebastian Andrzej Siewior
  2019-02-13 12:13 ` Petr Mladek
  0 siblings, 1 reply; 27+ messages in thread
From: Sebastian Andrzej Siewior @ 2019-02-12 16:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, tglx, Julia Cartwright, Sebastian Andrzej Siewior,
	Guenter Roeck, Steffen Trumtrar, Tim Sander

From: Julia Cartwright <julia@ni.com>

In order to enable the queuing of kthread work items from hardirq
context even when PREEMPT_RT_FULL is enabled, convert the worker
spin_lock to a raw_spin_lock.

This is only acceptable to do because the work performed under the lock
is well-bounded and minimal.

Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Guenter Roeck <linux@roeck-us.net>
Reported-and-tested-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reported-by: Tim Sander <tim@krieglstein.org>
Signed-off-by: Julia Cartwright <julia@ni.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/kthread.h |  4 ++--
 kernel/kthread.c        | 42 ++++++++++++++++++++---------------------
 2 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index c1961761311db..6b8c064f0cbcd 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -85,7 +85,7 @@ enum {
 
 struct kthread_worker {
 	unsigned int		flags;
-	spinlock_t		lock;
+	raw_spinlock_t		lock;
 	struct list_head	work_list;
 	struct list_head	delayed_work_list;
 	struct task_struct	*task;
@@ -106,7 +106,7 @@ struct kthread_delayed_work {
 };
 
 #define KTHREAD_WORKER_INIT(worker)	{				\
-	.lock = __SPIN_LOCK_UNLOCKED((worker).lock),			\
+	.lock = __RAW_SPIN_LOCK_UNLOCKED((worker).lock),		\
 	.work_list = LIST_HEAD_INIT((worker).work_list),		\
 	.delayed_work_list = LIST_HEAD_INIT((worker).delayed_work_list),\
 	}
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 087d18d771b53..5641b55783a6f 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -599,7 +599,7 @@ void __kthread_init_worker(struct kthread_worker *worker,
 				struct lock_class_key *key)
 {
 	memset(worker, 0, sizeof(struct kthread_worker));
-	spin_lock_init(&worker->lock);
+	raw_spin_lock_init(&worker->lock);
 	lockdep_set_class_and_name(&worker->lock, key, name);
 	INIT_LIST_HEAD(&worker->work_list);
 	INIT_LIST_HEAD(&worker->delayed_work_list);
@@ -641,21 +641,21 @@ int kthread_worker_fn(void *worker_ptr)
 
 	if (kthread_should_stop()) {
 		__set_current_state(TASK_RUNNING);
-		spin_lock_irq(&worker->lock);
+		raw_spin_lock_irq(&worker->lock);
 		worker->task = NULL;
-		spin_unlock_irq(&worker->lock);
+		raw_spin_unlock_irq(&worker->lock);
 		return 0;
 	}
 
 	work = NULL;
-	spin_lock_irq(&worker->lock);
+	raw_spin_lock_irq(&worker->lock);
 	if (!list_empty(&worker->work_list)) {
 		work = list_first_entry(&worker->work_list,
 					struct kthread_work, node);
 		list_del_init(&work->node);
 	}
 	worker->current_work = work;
-	spin_unlock_irq(&worker->lock);
+	raw_spin_unlock_irq(&worker->lock);
 
 	if (work) {
 		__set_current_state(TASK_RUNNING);
@@ -812,12 +812,12 @@ bool kthread_queue_work(struct kthread_worker *worker,
 	bool ret = false;
 	unsigned long flags;
 
-	spin_lock_irqsave(&worker->lock, flags);
+	raw_spin_lock_irqsave(&worker->lock, flags);
 	if (!queuing_blocked(worker, work)) {
 		kthread_insert_work(worker, work, &worker->work_list);
 		ret = true;
 	}
-	spin_unlock_irqrestore(&worker->lock, flags);
+	raw_spin_unlock_irqrestore(&worker->lock, flags);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(kthread_queue_work);
@@ -843,7 +843,7 @@ void kthread_delayed_work_timer_fn(struct timer_list *t)
 	if (WARN_ON_ONCE(!worker))
 		return;
 
-	spin_lock(&worker->lock);
+	raw_spin_lock(&worker->lock);
 	/* Work must not be used with >1 worker, see kthread_queue_work(). */
 	WARN_ON_ONCE(work->worker != worker);
 
@@ -852,7 +852,7 @@ void kthread_delayed_work_timer_fn(struct timer_list *t)
 	list_del_init(&work->node);
 	kthread_insert_work(worker, work, &worker->work_list);
 
-	spin_unlock(&worker->lock);
+	raw_spin_unlock(&worker->lock);
 }
 EXPORT_SYMBOL(kthread_delayed_work_timer_fn);
 
@@ -908,14 +908,14 @@ bool kthread_queue_delayed_work(struct kthread_worker *worker,
 	unsigned long flags;
 	bool ret = false;
 
-	spin_lock_irqsave(&worker->lock, flags);
+	raw_spin_lock_irqsave(&worker->lock, flags);
 
 	if (!queuing_blocked(worker, work)) {
 		__kthread_queue_delayed_work(worker, dwork, delay);
 		ret = true;
 	}
 
-	spin_unlock_irqrestore(&worker->lock, flags);
+	raw_spin_unlock_irqrestore(&worker->lock, flags);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(kthread_queue_delayed_work);
@@ -951,7 +951,7 @@ void kthread_flush_work(struct kthread_work *work)
 	if (!worker)
 		return;
 
-	spin_lock_irq(&worker->lock);
+	raw_spin_lock_irq(&worker->lock);
 	/* Work must not be used with >1 worker, see kthread_queue_work(). */
 	WARN_ON_ONCE(work->worker != worker);
 
@@ -963,7 +963,7 @@ void kthread_flush_work(struct kthread_work *work)
 	else
 		noop = true;
 
-	spin_unlock_irq(&worker->lock);
+	raw_spin_unlock_irq(&worker->lock);
 
 	if (!noop)
 		wait_for_completion(&fwork.done);
@@ -996,9 +996,9 @@ static bool __kthread_cancel_work(struct kthread_work *work, bool is_dwork,
 		 * any queuing is blocked by setting the canceling counter.
 		 */
 		work->canceling++;
-		spin_unlock_irqrestore(&worker->lock, *flags);
+		raw_spin_unlock_irqrestore(&worker->lock, *flags);
 		del_timer_sync(&dwork->timer);
-		spin_lock_irqsave(&worker->lock, *flags);
+		raw_spin_lock_irqsave(&worker->lock, *flags);
 		work->canceling--;
 	}
 
@@ -1045,7 +1045,7 @@ bool kthread_mod_delayed_work(struct kthread_worker *worker,
 	unsigned long flags;
 	int ret = false;
 
-	spin_lock_irqsave(&worker->lock, flags);
+	raw_spin_lock_irqsave(&worker->lock, flags);
 
 	/* Do not bother with canceling when never queued. */
 	if (!work->worker)
@@ -1062,7 +1062,7 @@ bool kthread_mod_delayed_work(struct kthread_worker *worker,
 fast_queue:
 	__kthread_queue_delayed_work(worker, dwork, delay);
 out:
-	spin_unlock_irqrestore(&worker->lock, flags);
+	raw_spin_unlock_irqrestore(&worker->lock, flags);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(kthread_mod_delayed_work);
@@ -1076,7 +1076,7 @@ static bool __kthread_cancel_work_sync(struct kthread_work *work, bool is_dwork)
 	if (!worker)
 		goto out;
 
-	spin_lock_irqsave(&worker->lock, flags);
+	raw_spin_lock_irqsave(&worker->lock, flags);
 	/* Work must not be used with >1 worker, see kthread_queue_work(). */
 	WARN_ON_ONCE(work->worker != worker);
 
@@ -1090,13 +1090,13 @@ static bool __kthread_cancel_work_sync(struct kthread_work *work, bool is_dwork)
 	 * In the meantime, block any queuing by setting the canceling counter.
 	 */
 	work->canceling++;
-	spin_unlock_irqrestore(&worker->lock, flags);
+	raw_spin_unlock_irqrestore(&worker->lock, flags);
 	kthread_flush_work(work);
-	spin_lock_irqsave(&worker->lock, flags);
+	raw_spin_lock_irqsave(&worker->lock, flags);
 	work->canceling--;
 
 out_fast:
-	spin_unlock_irqrestore(&worker->lock, flags);
+	raw_spin_unlock_irqrestore(&worker->lock, flags);
 out:
 	return ret;
 }
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread
end of thread, other threads:[~2019-02-13 12:13 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-18 13:21 [BUG] dw_wdt watchdog on linux-rt 4.18.5-rt4 not triggering Steffen Trumtrar
2018-09-18 13:46 ` Guenter Roeck
2018-09-19  6:46   ` Steffen Trumtrar
2018-09-19 19:43     ` Guenter Roeck
2018-09-20 20:48       ` Julia Cartwright
2018-09-21 13:34         ` Guenter Roeck
2018-09-21 16:42           ` Julia Cartwright
2018-09-21 20:21             ` Guenter Roeck
2018-09-24  7:24         ` Steffen Trumtrar
2018-09-24  7:24           ` Steffen Trumtrar
2018-09-28 21:03           ` [PATCH 0/2] Fix watchdogd wakeup deferral on RT Julia Cartwright
2018-09-28 21:03             ` [PATCH RT 2/2] watchdog, rt: prevent deferral of watchdogd wakeup Julia Cartwright
2018-09-28 22:38               ` kbuild test robot
2018-09-29  6:38                 ` Thomas Gleixner
2018-09-29 22:13                   ` Sebastian Andrzej Siewior
2018-09-30  1:41                     ` [kbuild-all] " Li, Philip
2018-09-28 23:20               ` kbuild test robot
2018-09-30 14:00               ` Guenter Roeck
2018-10-05 16:52               ` Sebastian Andrzej Siewior
2018-09-28 21:03             ` [PATCH 1/2] kthread: convert worker lock to raw spinlock Julia Cartwright
2018-10-05 16:46               ` Sebastian Andrzej Siewior
2018-10-05 18:10               ` Andrea Parri
2018-10-09 10:56                 ` Sebastian Andrzej Siewior
2018-09-20  8:18   ` [BUG] dw_wdt watchdog on linux-rt 4.18.5-rt4 not triggering Tim Sander
2018-09-18 18:14 ` Julia Cartwright
  -- strict thread matches above, loose matches on Subject: below --
2019-02-12 16:25 [PATCH 1/2] kthread: convert worker lock to raw spinlock Sebastian Andrzej Siewior
2019-02-13 12:13 ` Petr Mladek

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.