Re: [RFC][PATCH v2 20/31] timers: usb: Use del_timer_shutdown() before freeing timer
From: Steven Rostedt
Date: Fri Oct 28 2022 - 14:10:18 EST
On Fri, 28 Oct 2022 14:01:29 -0400
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> @@ -813,6 +839,14 @@ void destroy_timer_on_stack(struct timer_list *timer)
> }
> EXPORT_SYMBOL_GPL(destroy_timer_on_stack);
>
> +static struct timer_base *lock_timer_base(struct timer_list *timer,
> + unsigned long *flags);
> +
> +void __timer_reinit_debug_objects(struct timer_list *timer)
> +{
> + return;
> +}
> +
> #else
> static inline void debug_timer_init(struct timer_list *timer) { }
> static inline void debug_timer_activate(struct timer_list *timer) { }
Bah, the above chunk was leftover from some debugging.
Updated patch:
-- Steve
include/linux/timer.h | 38 +++++++++++++++++++++++++++++++++--
include/linux/workqueue.h | 4 ++--
kernel/time/timer.c | 42 +++++++++++++++++++++++++++++++++------
kernel/workqueue.c | 12 +++++++++++
4 files changed, 86 insertions(+), 10 deletions(-)
diff --git a/include/linux/timer.h b/include/linux/timer.h
index 45392b0ac2e1..27e3a8676ff8 100644
--- a/include/linux/timer.h
+++ b/include/linux/timer.h
@@ -8,6 +8,12 @@
#include <linux/debugobjects.h>
#include <linux/stringify.h>
+enum timer_debug_state {
+ TIMER_DEBUG_DISABLED,
+ TIMER_DEBUG_ENABLED,
+ TIMER_DEBUG_WORK,
+};
+
struct timer_list {
/*
* All fields that change during normal runtime grouped to the
@@ -18,6 +24,9 @@ struct timer_list {
void (*function)(struct timer_list *);
u32 flags;
+#ifdef CONFIG_DEBUG_OBJECTS_TIMERS
+ enum timer_debug_state enabled;
+#endif
#ifdef CONFIG_LOCKDEP
struct lockdep_map lockdep_map;
#endif
@@ -128,6 +137,31 @@ static inline void init_timer_on_stack_key(struct timer_list *timer,
init_timer_on_stack_key((_timer), (_fn), (_flags), NULL, NULL)
#endif
+#ifdef CONFIG_DEBUG_OBJECTS_TIMERS
+#define __init_timer_debug(_timer, _fn, _flags) \
+ do { \
+ (_timer)->enabled = TIMER_DEBUG_DISABLED; \
+ __init_timer((_timer), (_fn), (_flags)); \
+ } while (0)
+#define __init_timer_work(_timer, _fn, _flags) \
+ do { \
+ (_timer)->enabled = TIMER_DEBUG_WORK; \
+ __init_timer((_timer), (_fn), (_flags)); \
+ } while (0)
+#define __init_timer_work_on_stack(_timer, _fn, _flags) \
+ do { \
+ (_timer)->enabled = TIMER_DEBUG_WORK; \
+ __init_timer_on_stack((_timer), (_fn), (_flags)); \
+ } while (0)
+#else
+#define __init_timer_debug(_timer, _fn, _flags) \
+ __init_timer((_timer), (_fn), (_flags))
+#define __init_timer_work(_timer, _fn, _flags) \
+ __init_timer((_timer), (_fn), (_flags))
+#define __init_timer_work_on_stack(_timer, _fn, _flags) \
+ __init_timer_on_stack((_timer), (_fn), (_flags))
+#endif
+
/**
* timer_setup - prepare a timer for first use
* @timer: the timer in question
@@ -139,7 +173,7 @@ static inline void init_timer_on_stack_key(struct timer_list *timer,
* be used and must be balanced with a call to destroy_timer_on_stack().
*/
#define timer_setup(timer, callback, flags) \
- __init_timer((timer), (callback), (flags))
+ __init_timer_debug((timer), (callback), (flags))
#define timer_setup_on_stack(timer, callback, flags) \
__init_timer_on_stack((timer), (callback), (flags))
@@ -243,7 +277,7 @@ static inline int del_timer_shutdown(struct timer_list *timer)
return __del_timer_sync(timer, true);
}
-#define del_singleshot_timer_sync(t) del_timer_sync(t)
+#define del_singleshot_timer_sync(t) del_timer_shutdown(t)
extern void init_timers(void);
struct hrtimer;
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index a0143dd24430..290c96429ce1 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -250,7 +250,7 @@ static inline unsigned int work_static(struct work_struct *work) { return 0; }
#define __INIT_DELAYED_WORK(_work, _func, _tflags) \
do { \
INIT_WORK(&(_work)->work, (_func)); \
- __init_timer(&(_work)->timer, \
+ __init_timer_work(&(_work)->timer, \
delayed_work_timer_fn, \
(_tflags) | TIMER_IRQSAFE); \
} while (0)
@@ -258,7 +258,7 @@ static inline unsigned int work_static(struct work_struct *work) { return 0; }
#define __INIT_DELAYED_WORK_ONSTACK(_work, _func, _tflags) \
do { \
INIT_WORK_ONSTACK(&(_work)->work, (_func)); \
- __init_timer_on_stack(&(_work)->timer, \
+ __init_timer_work_on_stack(&(_work)->timer, \
delayed_work_timer_fn, \
(_tflags) | TIMER_IRQSAFE); \
} while (0)
diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 5179ac2335a0..ac2e8beb4235 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -691,7 +691,11 @@ static bool timer_fixup_init(void *addr, enum debug_obj_state state)
switch (state) {
case ODEBUG_STATE_ACTIVE:
- del_timer_sync(timer);
+ if (timer->enabled != TIMER_DEBUG_WORK)
+ timer->enabled = TIMER_DEBUG_ENABLED;
+ del_timer_shutdown(timer);
+ if (timer->enabled != TIMER_DEBUG_WORK)
+ timer->enabled = TIMER_DEBUG_DISABLED;
debug_object_init(timer, &timer_debug_descr);
return true;
default:
@@ -737,8 +741,10 @@ static bool timer_fixup_free(void *addr, enum debug_obj_state state)
switch (state) {
case ODEBUG_STATE_ACTIVE:
- del_timer_sync(timer);
+ del_timer_shutdown(timer);
debug_object_free(timer, &timer_debug_descr);
+ if (timer->enabled != TIMER_DEBUG_WORK)
+ timer->enabled = TIMER_DEBUG_DISABLED;
return true;
default:
return false;
@@ -774,16 +780,36 @@ static const struct debug_obj_descr timer_debug_descr = {
static inline void debug_timer_init(struct timer_list *timer)
{
+ if (timer->enabled == TIMER_DEBUG_ENABLED)
+ return;
+
debug_object_init(timer, &timer_debug_descr);
}
static inline void debug_timer_activate(struct timer_list *timer)
{
+ if (timer->enabled == TIMER_DEBUG_ENABLED)
+ return;
+
+ if (timer->enabled == TIMER_DEBUG_DISABLED)
+ timer->enabled = TIMER_DEBUG_ENABLED;
+
debug_object_activate(timer, &timer_debug_descr);
}
-static inline void debug_timer_deactivate(struct timer_list *timer)
+static inline void debug_timer_deactivate(struct timer_list *timer, bool free)
{
+ switch (timer->enabled) {
+ case TIMER_DEBUG_DISABLED:
+ return;
+ case TIMER_DEBUG_ENABLED:
+ if (!free)
+ return;
+ timer->enabled = TIMER_DEBUG_DISABLED;
+ break;
+ case TIMER_DEBUG_WORK:
+ break;
+ }
debug_object_deactivate(timer, &timer_debug_descr);
}
@@ -828,7 +854,7 @@ static inline void debug_init(struct timer_list *timer)
static inline void debug_deactivate(struct timer_list *timer)
{
- debug_timer_deactivate(timer);
+ debug_timer_deactivate(timer, false);
trace_timer_cancel(timer);
}
@@ -1251,8 +1277,10 @@ int __del_timer(struct timer_list *timer, bool free)
if (timer_pending(timer)) {
base = lock_timer_base(timer, &flags);
ret = detach_if_pending(timer, base, true);
- if (free && ret)
+ if (free) {
timer->function = NULL;
+ debug_timer_deactivate(timer, true);
+ }
raw_spin_unlock_irqrestore(&base->lock, flags);
}
@@ -1272,8 +1300,10 @@ static int __try_to_del_timer_sync(struct timer_list *timer, bool free)
if (base->running_timer != timer)
ret = detach_if_pending(timer, base, true);
- if (free)
+ if (free) {
timer->function = NULL;
+ debug_timer_deactivate(timer, true);
+ }
raw_spin_unlock_irqrestore(&base->lock, flags);
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 47a7124bbea4..9a48213fc4e4 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -1225,6 +1225,16 @@ static void pwq_dec_nr_in_flight(struct pool_workqueue *pwq, unsigned long work_
put_pwq(pwq);
}
+static void deactivate_timer(struct work_struct *work, bool is_dwork)
+{
+ struct delayed_work *dwork;
+
+ if (!is_dwork)
+ return;
+
+ dwork = to_delayed_work(work);
+}
+
/**
* try_to_grab_pending - steal work item from worklist and disable irq
* @work: work item to steal
@@ -3148,6 +3158,8 @@ static bool __cancel_work_timer(struct work_struct *work, bool is_dwork)
}
} while (unlikely(ret < 0));
+ deactivate_timer(work, is_dwork);
+
/* tell other tasks trying to grab @work to back off */
mark_work_canceling(work);
local_irq_restore(flags);
--
2.35.1