Bug fix for module unloading.

Once a consumer has opened the /dev/dtrace/dtrace device file, providers can
no longer be safely unloaded, until the last consumer closed the device file.
This commit adds code to ensure this behaviour.  Failing to do so results in
almost certain OOPSes because Linux does not support kernel modules "refusing"
to be unloaded in response to an rmmod.

Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>

diff --git a/dtrace/dtrace-module.spec b/dtrace/dtrace-module.spec
index a518a0f39ae3d4b27ec01bcba1d4b1781efdb1dd..af02a89c49c654747c602effdd96ea91a18e2f15 100644 (file)
--- a/dtrace/dtrace-module.spec
+++ b/dtrace/dtrace-module.spec
@@ -100,6 +100,8 @@ rm -rf %{buildroot}
 /usr/include/linux/dtrace/types.h
 
 %changelog
+* Wed Aug  7 2013 Kris Van Hees <kris.van.hees@oracle.com> - 0.4.0-2
+- Bugfix for module unloading.
 * Thu Jul 25 2013 Kris Van Hees <kris.van.hees@oracle.com> - 0.4.0-1
 - Support meta-providers, USDT, and fasttrap (for USDT only).
 - Export DTrace kernel headers to userspace.

diff --git a/dtrace/dtrace_debug.h b/dtrace/dtrace_debug.h
index af2cef31cd67f9820c502f3e42a077ad5b3a0233..4f6d839900e48343fd1a3718df764159108ee840 100644 (file)
--- a/dtrace/dtrace_debug.h
+++ b/dtrace/dtrace_debug.h
@@ -8,6 +8,7 @@
 # undef DT_DBG_DOF
 # undef DT_DBG_ENABLE
 # undef DT_DBG_IOCTL
+# undef DT_DBG_PMOD
 # undef DT_DBG_PROBE
 
 #else /* CONFIG_DT_DEBUG */
@@ -17,6 +18,7 @@
 # undef DT_DBG_DOF
 # undef DT_DBG_ENABLE
 # undef DT_DBG_IOCTL
+# undef DT_DBG_PMOD
 # undef DT_DBG_PROBE
 
 #endif /* CONFIG_DT_DEBUG */
@@ -54,6 +56,12 @@
 # define dt_dbg_ioctl(fmt, ...)
 #endif
 
+#ifdef DT_DBG_PMOD
+# define dt_dbg_pmod(fmt, ...)         pr_info(fmt, ## __VA_ARGS__)
+#else
+# define dt_dbg_pmod(fmt, ...)
+#endif
+
 #ifdef DT_DBG_PROBE
 # define dt_dbg_probe(fmt, ...)                pr_info(fmt, ## __VA_ARGS__)
 #else

diff --git a/dtrace/dtrace_dev.c b/dtrace/dtrace_dev.c
index a00049f7dacf521e2cdd39c7c609f74caee62637..127f17520060958df1b4cc9a8cc72d7ec3a7cce0 100644 (file)
--- a/dtrace/dtrace_dev.c
+++ b/dtrace/dtrace_dev.c
@@ -153,6 +153,8 @@ static int dtrace_open(struct inode *inode, struct file *file)
        if (dtrace_opens == 1)
                dtrace_enable();
 
+       dtrace_pmod_add_consumer();
+
        mutex_unlock(&dtrace_lock);
 
        return 0;
@@ -994,6 +996,8 @@ static int dtrace_close(struct inode *inode, struct file *file)
                dtrace_disable();
 #endif
 
+       dtrace_pmod_del_consumer();
+
        mutex_unlock(&dtrace_lock);
        mutex_unlock(&cpu_lock);
 

diff --git a/dtrace/dtrace_ptofapi.c b/dtrace/dtrace_ptofapi.c
index ed3645494c7efa9f4154d808654e23b022ffd0aa..869cdfa244ae34faf7f3f4db634e24a3c24585aa 100644 (file)
--- a/dtrace/dtrace_ptofapi.c
+++ b/dtrace/dtrace_ptofapi.c
@@ -26,6 +26,7 @@
  */
 
 #include <linux/idr.h>
+#include <linux/list.h>
 #include <linux/module.h>
 #include <linux/slab.h>
 
@@ -39,6 +40,102 @@ DEFINE_MUTEX(dtrace_lock);
 DEFINE_MUTEX(dtrace_provider_lock);
 DEFINE_MUTEX(dtrace_meta_lock);
 
+static LIST_HEAD(provider_modules);
+
+#ifdef DT_DBG_PMOD
+void dtrace_pmod_debug(void)
+{
+       dtrace_pmod_t   *pmod;
+       int             i = 0;
+
+       mutex_lock(&dtrace_provider_lock);
+       if (list_empty(&provider_modules))
+               pr_info("provider_modules[] is empty\n");
+       else {
+               list_for_each_entry(pmod, &provider_modules, list)
+                       pr_info("provider_modules[%3d] = %p (%s)\n",
+                               i++, pmod->mod, pmod->mod->name);
+       }
+       mutex_unlock(&dtrace_provider_lock);
+}
+#endif /* CONFIG_DT_DEBUG */
+
+/*
+ * Register a kernel module as a container for one or more providers.  This is
+ * called during module initialization.
+ */
+void dtrace_pmod_register(dtrace_pmod_t *pmod)
+{
+       if (pmod->mod == NULL) {
+               pr_warn("Provider module without a module?  Ignored!\n");
+               return;
+       }
+
+       mutex_lock(&dtrace_provider_lock);
+       list_add(&pmod->list, &provider_modules);
+       mutex_unlock(&dtrace_provider_lock);
+
+       /*
+        * If there already are consumers, we need to increase the reference
+        * count on the provider module with the number of consumers.  Failing
+        * to do so will result in very unhappy module handling when the
+        * reference count drops below zero.
+        */
+       mutex_lock(&dtrace_lock);
+       if (dtrace_opens) {
+               preempt_disable();
+               __this_cpu_add(pmod->mod->refptr->incs, dtrace_opens);
+               preempt_enable();
+       }
+       mutex_unlock(&dtrace_lock);
+#ifdef DT_DBG_PMOD
+       pr_info("After dtrace_pmod_register(%p):\n", pmod);
+       dtrace_pmod_debug();
+#endif
+}
+EXPORT_SYMBOL(dtrace_pmod_register);
+
+/*
+ * Account for a new consumer that opened /dev/dtrace/dtrace.  We can use
+ * __module_get() because a module in the provider_modules list is guaranteed
+ * to have been initialized, which means __module_get() cannot fail.
+ */
+void dtrace_pmod_add_consumer(void)
+{
+       dtrace_pmod_t   *pmod, *next;
+
+       list_for_each_entry_safe(pmod, next, &provider_modules, list)
+               __module_get(pmod->mod);
+}
+
+/*
+ * Account for a consumer that is done with its work, thereby closing the
+ * /dev/dtrace/dtrace device file.
+ */
+void dtrace_pmod_del_consumer(void)
+{
+       dtrace_pmod_t   *pmod, *next;
+
+       list_for_each_entry_safe(pmod, next, &provider_modules, list)
+               module_put(pmod->mod);
+}
+
+/*
+ * Deregister a kernel module as a container for providers.  This is called
+ * during module exit.
+ */
+void dtrace_pmod_unregister(dtrace_pmod_t *pmod)
+{
+       mutex_lock(&dtrace_provider_lock);
+       list_del(&pmod->list);
+       mutex_unlock(&dtrace_provider_lock);
+#ifdef DT_DBG_PMOD
+       pr_info("After dtrace_pmod_unregister(%p):\n", pmod);
+       dtrace_pmod_debug();
+#endif
+}
+EXPORT_SYMBOL(dtrace_pmod_unregister);
+
 /*
  * Register the calling provider with the DTrace core.  This should generally
  * be called by providers during module initialization.

diff --git a/dtrace/include/dtrace/provider.h b/dtrace/include/dtrace/provider.h
index a6eb7c1607009b94266c11b8353856ab06ceab30..d3653255dd9500a6d983b8c5d2878526725032f7 100644 (file)
--- a/dtrace/include/dtrace/provider.h
+++ b/dtrace/include/dtrace/provider.h
@@ -765,6 +765,18 @@ typedef struct dtrace_mprovider {
        dtrace_provider_id_t    dtmp_id;
 } dtrace_mprovider_t;
 
+typedef struct dtrace_pmod {
+       struct module           *mod;
+       struct list_head        list;
+} dtrace_pmod_t;
+
+#ifdef CONFIG_DT_DEBUG
+extern void dtrace_pmod_debug(void);
+#endif
+extern void dtrace_pmod_register(dtrace_pmod_t *);
+extern void dtrace_pmod_add_consumer(void);
+extern void dtrace_pmod_del_consumer(void);
+extern void dtrace_pmod_unregister(dtrace_pmod_t *);
 extern int dtrace_register(const char *, const dtrace_pattr_t *, uint32_t,
                           const cred_t *, const dtrace_pops_t *, void *,
                           dtrace_provider_id_t *);
@@ -820,6 +832,7 @@ extern void dtrace_probe(dtrace_id_t, uintptr_t, uintptr_t, uintptr_t,
 
 #define DT_PROVIDER_MODULE(name, priv)                                 \
   dtrace_provider_id_t name##_id;                                      \
+  static dtrace_pmod_t name##_pmod = { THIS_MODULE, };                 \
                                                                        \
   static int __init name##_init(void)                                  \
   {                                                                    \
@@ -829,6 +842,8 @@ extern void dtrace_probe(dtrace_id_t, uintptr_t, uintptr_t, uintptr_t,
        if (ret)                                                        \
                goto failed;                                            \
                                                                        \
+       dtrace_pmod_register(&name##_pmod);                             \
+                                                                       \
        ret = dtrace_register(__stringify(name), &name##_attr, priv,    \
                              NULL, &name##_pops, NULL, &name##_id);    \
        if (ret)                                                        \
@@ -843,6 +858,7 @@ extern void dtrace_probe(dtrace_id_t, uintptr_t, uintptr_t, uintptr_t,
   static void __exit name##_exit(void)                                 \
   {                                                                    \
        dtrace_unregister(name##_id);                                   \
+       dtrace_pmod_unregister(&name##_pmod);                           \
        name##_dev_exit();                                              \
   }                                                                    \
                                                                        \
@@ -851,6 +867,7 @@ extern void dtrace_probe(dtrace_id_t, uintptr_t, uintptr_t, uintptr_t,
 
 #define DT_META_PROVIDER_MODULE(name)                                  \
   dtrace_meta_provider_id_t    name##_id;                              \
+  static dtrace_pmod_t         name##_pmod = { THIS_MODULE, };         \
                                                                        \
   static int __init name##_init(void)                                  \
   {                                                                    \
@@ -860,6 +877,8 @@ extern void dtrace_probe(dtrace_id_t, uintptr_t, uintptr_t, uintptr_t,
        if (ret)                                                        \
                goto failed;                                            \
                                                                        \
+       dtrace_pmod_register(&name##_pmod);                             \
+                                                                       \
        ret = dtrace_meta_register(__stringify(name), &name##_mops,     \
                                   NULL, &name##_id);                   \
        if (ret)                                                        \
@@ -874,6 +893,7 @@ extern void dtrace_probe(dtrace_id_t, uintptr_t, uintptr_t, uintptr_t,
   static void __exit name##_exit(void)                                 \
   {                                                                    \
        dtrace_meta_unregister(name##_id);                              \
+       dtrace_pmod_unregister(&name##_pmod);                           \
        name##_dev_exit();                                              \
   }                                                                    \
                                                                        \
@@ -881,6 +901,8 @@ extern void dtrace_probe(dtrace_id_t, uintptr_t, uintptr_t, uintptr_t,
   module_exit(name##_exit);
 
 #define DT_MULTI_PROVIDER_MODULE(name, plist)                          \
+  static dtrace_pmod_t         name##_pmod = { THIS_MODULE, };         \
+                                                                       \
   static int __init name##_init(void)                                  \
   {                                                                    \
        int                     ret = 0;                                \
@@ -890,6 +912,8 @@ extern void dtrace_probe(dtrace_id_t, uintptr_t, uintptr_t, uintptr_t,
        if (ret)                                                        \
                goto failed;                                            \
                                                                        \
+       dtrace_pmod_register(&name##_pmod);                             \
+                                                                       \
        for (prov = plist; prov->dtmp_name != NULL; prov++) {           \
                if (dtrace_register(prov->dtmp_name, prov->dtmp_attr,   \
                                    prov->dtmp_priv, NULL,              \
@@ -914,12 +938,15 @@ extern void dtrace_probe(dtrace_id_t, uintptr_t, uintptr_t, uintptr_t,
                if (prov->dtmp_id != DTRACE_PROVNONE) {                 \
                        ret = dtrace_unregister(prov->dtmp_id);         \
                        if (ret != 0)                                   \
-                               return;                                 \
+                               pr_warning("Failed to unregister sdt "  \
+                                          "provider %s: %d",           \
+                                          prov->dtmp_name, ret);       \
                                                                        \
                        prov->dtmp_id = DTRACE_PROVNONE;                \
                }                                                       \
        }                                                               \
                                                                        \
+       dtrace_pmod_unregister(&name##_pmod);                           \
        name##_dev_exit();                                              \
   }                                                                    \
                                                                        \