workqueue: Add proper debugobjects support for delayed_work on stack

PATCH In review Thomas Gleixner 3 participants 6 messages Mar 23, 2014 – Mar 25, 2014

Subsystems (3)

x86 Workqueue Hpet: x86

lore

Share & tools

lore

Thomas Gleixner

author

2014-03-23 14:20 UTC #0 lore raw

cover workqueue: Add proper debugobjects support for delayed_work on stack

While trying to decode Vince mystery crash I noticed, that we have a initializer for delayed work on stack but no destructor.

The only user in tree got it wrong, which leaks a tracking object for the work itself.

Provide destroy_delayed_work_on_stack() and make hpet use it.

Thanks,

tglx

Thomas Gleixner

author

2014-03-23 14:20 UTC #1 lore raw

PATCH 1/2 workqueue: Provide destroy_delayed_work_on_stack()

If a delayed or deferrable work is on stack we need to tell debug objects that we are destroying the timer and the work. Otherwise we leak the tracking object.

Signed-off-by Thomas Gleixner <[email protected]>

Cc Tejun Heo <[email protected]>

---
 include/linux/workqueue.h |    2 ++
 kernel/workqueue.c        |    7 +++++++
 2 files changed, 9 insertions(+)

include/linux/workqueue.h +2 -0

Index: tip/include/linux/workqueue.h
===================================================================
@@ -191,6 +191,7 @@ struct execute_work {
 #ifdef CONFIG_DEBUG_OBJECTS_WORK
 extern void __init_work(struct work_struct *work, int onstack);
 extern void destroy_work_on_stack(struct work_struct *work);
+extern void destroy_delayed_work_on_stack(struct delayed_work *work);
 static inline unsigned int work_static(struct work_struct *work)
 {
 	return *work_data_bits(work) & WORK_STRUCT_STATIC;

@@ -198,6 +199,7 @@ static inline unsigned int work_static(s
 #else
 static inline void __init_work(struct work_struct *work, int onstack) { }
 static inline void destroy_work_on_stack(struct work_struct *work) { }
+static inline void destroy_delayed_work_on_stack(struct delayed_work *work) { }
 static inline unsigned int work_static(struct work_struct *work) { return 0; }
 #endif

kernel/workqueue.c +7 -0

Index: tip/kernel/workqueue.c
===================================================================
@@ -516,6 +516,13 @@ void destroy_work_on_stack(struct work_s
 }
 EXPORT_SYMBOL_GPL(destroy_work_on_stack);
 
+void destroy_delayed_work_on_stack(struct delayed_work *work)
+{
+	destroy_timer_on_stack(&work->timer);
+	debug_object_free(&work->work, &work_debug_descr);
+}
+EXPORT_SYMBOL_GPL(destroy_delayed_work_on_stack);
+
 #else
 static inline void debug_work_activate(struct work_struct *work) { }
 static inline void debug_work_deactivate(struct work_struct *work) { }

Tejun Heo

maintainer Acked-by on v1 1/2

2014-03-24 12:51 UTC #2 lore raw

Re: [patch 1/2] workqueue: Provide destroy_delayed_work_on_stack()

Quoted 6 lines · Thomas Gleixner

If a delayed or deferrable work is on stack we need to tell debug objects that we are destroying the timer and the work. Otherwise we leak the tracking object.
Signed-off-by Thomas Gleixner <[email protected]>
Cc Tejun Heo <[email protected]>

Acked-by Tejun Heo <[email protected]>

How do you want to route this? If it's better to route it with the next patch, please feel free to do so.

Thanks.

Signature · 1 line

tejun

tip-bot for Thomas Gleixner

merge bot

2014-03-25 16:37 UTC #3 lore raw

[tip:timers/core] workqueue: Provide destroy_delayed_work_on_stack()

In reply to Thomas Gleixner

workqueue: Provide destroy_delayed_work_on_stack()

Commit-ID:  ea2e64f280d2a34a8ed9ae3d783cd770d14b70ec
Gitweb:     http://git.kernel.org/tip/ea2e64f280d2a34a8ed9ae3d783cd770d14b70ec
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Sun, 23 Mar 2014 14:20:44 +0000
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Tue, 25 Mar 2014 17:33:42 +0100

workqueue: Provide destroy_delayed_work_on_stack()

If a delayed or deferrable work is on stack we need to tell debug objects that we are destroying the timer and the work. Otherwise we leak the tracking object.

Signed-off-by Thomas Gleixner <[email protected]>

Cc Vince Weaver <[email protected]>

Acked-by Tejun Heo <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Thomas Gleixner <[email protected]>

---
 include/linux/workqueue.h | 2 ++
 kernel/workqueue.c        | 7 +++++++
 2 files changed, 9 insertions(+)

include/linux/workqueue.h +2 -0

index 594521b..abdbe5a 100644
@@ -191,6 +191,7 @@ struct execute_work {
 #ifdef CONFIG_DEBUG_OBJECTS_WORK
 extern void __init_work(struct work_struct *work, int onstack);
 extern void destroy_work_on_stack(struct work_struct *work);
+extern void destroy_delayed_work_on_stack(struct delayed_work *work);
 static inline unsigned int work_static(struct work_struct *work)
 {
 	return *work_data_bits(work) & WORK_STRUCT_STATIC;

@@ -198,6 +199,7 @@ static inline unsigned int work_static(struct work_struct *work)
 #else
 static inline void __init_work(struct work_struct *work, int onstack) { }
 static inline void destroy_work_on_stack(struct work_struct *work) { }
+static inline void destroy_delayed_work_on_stack(struct delayed_work *work) { }
 static inline unsigned int work_static(struct work_struct *work) { return 0; }
 #endif

kernel/workqueue.c +7 -0

index 82ef9f3..5b690b5 100644
@@ -516,6 +516,13 @@ void destroy_work_on_stack(struct work_struct *work)
 }
 EXPORT_SYMBOL_GPL(destroy_work_on_stack);
 
+void destroy_delayed_work_on_stack(struct delayed_work *work)
+{
+	destroy_timer_on_stack(&work->timer);
+	debug_object_free(&work->work, &work_debug_descr);
+}
+EXPORT_SYMBOL_GPL(destroy_delayed_work_on_stack);
+
 #else
 static inline void debug_work_activate(struct work_struct *work) { }
 static inline void debug_work_deactivate(struct work_struct *work) { }

Thomas Gleixner

author

2014-03-23 14:20 UTC #4 lore raw

PATCH 2/2 x86: hpet: Use proper destructor for delayed work

In reply to Thomas Gleixner

destroy_timer_on_stack() is hardly the right thing for a delayed work. We leak a tracking object for the work itself when DEBUG_OBJECTS is enabled.

Signed-off-by Thomas Gleixner <[email protected]>

Cc [email protected]

---
 arch/x86/kernel/hpet.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/x86/kernel/hpet.c +1 -1

Index: tip/arch/x86/kernel/hpet.c
===================================================================
@@ -699,7 +699,7 @@ static int hpet_cpuhp_notify(struct noti
 		/* FIXME: add schedule_work_on() */
 		schedule_delayed_work_on(cpu, &work.work, 0);
 		wait_for_completion(&work.complete);
-		destroy_timer_on_stack(&work.work.timer);
+		destroy_delayed_work_on_stack(&work.work);
 		break;
 	case CPU_DEAD:
 		if (hdev) {

tip-bot for Thomas Gleixner

merge bot

2014-03-25 16:38 UTC #5 lore raw

[tip:timers/core] x86: hpet: Use proper destructor for delayed work

x86: hpet: Use proper destructor for delayed work

Commit-ID:  b712c8dae05931a76b6c17a4254f403798e6caef
Gitweb:     http://git.kernel.org/tip/b712c8dae05931a76b6c17a4254f403798e6caef
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Sun, 23 Mar 2014 14:20:45 +0000
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Tue, 25 Mar 2014 17:34:01 +0100

x86: hpet: Use proper destructor for delayed work

destroy_timer_on_stack() is hardly the right thing for a delayed work. We leak a tracking object for the work itself when DEBUG_OBJECTS is enabled.

Signed-off-by Thomas Gleixner <[email protected]>

Cc Vince Weaver <[email protected]>

Cc [email protected]

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Thomas Gleixner <[email protected]>

---
 arch/x86/kernel/hpet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/x86/kernel/hpet.c +1 -1

index da85a8e..b91abfd 100644
@@ -699,7 +699,7 @@ static int hpet_cpuhp_notify(struct notifier_block *n,
 		/* FIXME: add schedule_work_on() */
 		schedule_delayed_work_on(cpu, &work.work, 0);
 		wait_for_completion(&work.complete);
-		destroy_timer_on_stack(&work.work.timer);
+		destroy_delayed_work_on_stack(&work.work);
 		break;
 	case CPU_DEAD:
 		if (hdev) {