From 66283521fac33ac5716b71a6d2662093aec409c1 Mon Sep 17 00:00:00 2001 From: David Smith Date: Wed, 12 Mar 2014 09:07:58 -0500 Subject: [PATCH] Improve task_finder/utrace shutdown. * runtime/stp_utrace.c (utrace_exit): Move the call to stp_task_work_exit() up above the calls to free the kmem caches. This make sure any running task work items don't have the memory freed out from under them. * runtime/linux/task_finder2.c (__stp_task_finder_cleanup): Move utrace_shutdown() call into stap_stop_task_finder(). (stap_stop_task_finder): Call utrace_shutdown() directly. Wait to make sure all tracepoint probes are finished. --- runtime/linux/task_finder2.c | 24 +++++++----------------- runtime/stp_utrace.c | 27 +++++++++++++++++++-------- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/runtime/linux/task_finder2.c b/runtime/linux/task_finder2.c index a3abc16..ef074c5 100644 --- a/runtime/linux/task_finder2.c +++ b/runtime/linux/task_finder2.c @@ -418,15 +418,6 @@ stap_utrace_detach_ops(struct utrace_engine_ops *ops) debug_task_finder_report(); } -static void -__stp_task_finder_cleanup(void) -{ - // The utrace_shutdown() function detaches and deletes - // everything for us - we don't have to go through each - // engine. - utrace_shutdown(); -} - static char * __stp_get_mm_path(struct mm_struct *mm, char *buf, int buflen) { @@ -1849,16 +1840,16 @@ stap_stop_task_finder(void) atomic_set(&__stp_task_finder_state, __STP_TF_STOPPING); debug_task_finder_report(); -#if 0 - /* We don't need this since __stp_task_finder_cleanup() - * removes everything by calling utrace_shutdown(). */ - stap_utrace_detach_ops(&__stp_utrace_task_finder_ops); -#endif - __stp_task_finder_cleanup(); + + // The utrace_shutdown() function detaches and cleans up + // everything for us - we don't have to go through each + // engine. This also means that the attach_count could end up + // > 0 (since we don't got through each engine individually). + utrace_shutdown(); + debug_task_finder_report(); atomic_set(&__stp_task_finder_state, __STP_TF_STOPPED); -#if 0 /* Now that all the engines are detached, make sure * all the callbacks are finished. If they aren't, we'll * crash the kernel when the module is removed. */ @@ -1873,7 +1864,6 @@ stap_stop_task_finder(void) printk(KERN_ERR "it took %d polling loops to quit.\n", i); #endif debug_task_finder_report(); -#endif /* Make sure all outstanding task work requests are canceled. */ __stp_tf_cancel_task_work(); diff --git a/runtime/stp_utrace.c b/runtime/stp_utrace.c index 89ea0e4..a6f363d 100644 --- a/runtime/stp_utrace.c +++ b/runtime/stp_utrace.c @@ -286,12 +286,18 @@ static int utrace_exit(void) { utrace_shutdown(); - if (utrace_cachep) - kmem_cache_destroy(utrace_cachep); - if (utrace_engine_cachep) - kmem_cache_destroy(utrace_engine_cachep); - stp_task_work_exit(); + + /* After utrace_shutdown() and stp_task_work_exit() (and the + * code in stap_stop_task_finder()), we're *sure* there are no + * tracepoint probes or task work items running or scheduled + * to be run. So, now would be a great time to actually free + * everything. */ + + if (utrace_cachep) + kmem_cache_destroy(utrace_cachep); + if (utrace_engine_cachep) + kmem_cache_destroy(utrace_engine_cachep); return 0; } @@ -373,9 +379,14 @@ static void utrace_shutdown(void) unregister_trace_sys_exit(utrace_report_syscall_exit, NULL); tracepoint_synchronize_unregister(); - /* After calling tracepoint_synchronize_unregister(), we're - * sure there are no outstanding tracepoint probes being - * called. So, now would be a great time to free everything. */ + /* After tracepoint_synchronize_unregister(), we're *sure* + * there will be no new tracepoint probes running. There could + * be currently running tracepoint probes or task work + * items. So, now would be a great time to cleanup. + * + * Currently running items should be OK, since + * utrace_cleanup() just puts the memory back into the utrace + * kmem caches. */ #ifdef STP_TF_DEBUG printk(KERN_ERR "%s:%d - freeing task-specific\n", __FUNCTION__, __LINE__); -- 1.8.3.1