]> git.proxmox.com Git - mirror_spl-debian.git/commitdiff
- Add more strict in_atomic() checking to the mutex entry
authorbehlendo <behlendo@7e1ea52c-4ff2-0310-8f11-9dd32ca42a1c>
Fri, 11 Apr 2008 17:03:57 +0000 (17:03 +0000)
committerbehlendo <behlendo@7e1ea52c-4ff2-0310-8f11-9dd32ca42a1c>
Fri, 11 Apr 2008 17:03:57 +0000 (17:03 +0000)
function just to be extra safety and paranoid.

- Rewrite the thread shim to take full advantage of the
new kernel kthread API.  This greatly simplifies things.

- Add a new regression test for thread_exit() to ensure
it properly terminates a thread immediately without
allowing futher execution of the thread.

git-svn-id: https://outreach.scidac.gov/svn/spl/trunk@69 7e1ea52c-4ff2-0310-8f11-9dd32ca42a1c

include/sys/mutex.h
modules/spl/spl-thread.c
modules/splat/splat-thread.c

index 2db4a7f96d8307f2c1969986ba11c37a02c66a1c..ca76d6ea9b942f522ccc8aaf14739b266c6f4238 100644 (file)
@@ -6,6 +6,7 @@ extern "C" {
 #endif
 
 #include <linux/module.h>
+#include <linux/hardirq.h>
 #include <sys/types.h>
 
 /* See the "Big Theory Statement" in solaris mutex.c.
@@ -65,6 +66,14 @@ static __inline__ void
 mutex_enter(kmutex_t *mp)
 {
        BUG_ON(mp->km_magic != KM_MAGIC);
+
+       if (unlikely(in_atomic() && !current->exit_state)) {
+               dump_stack();
+               printk("Scheduling while atomic: %s/0x%08x/%d\n",
+                      current->comm, preempt_count(), current->pid);
+               BUG();
+       }
+
        down(&mp->km_sem);  /* Will check in_atomic() for us */
        BUG_ON(mp->km_owner != NULL);
        mp->km_owner = current;
@@ -78,6 +87,14 @@ mutex_tryenter(kmutex_t *mp)
        int result;
 
        BUG_ON(mp->km_magic != KM_MAGIC);
+
+       if (unlikely(in_atomic() && !current->exit_state)) {
+               dump_stack();
+               printk("Scheduling while atomic: %s/0x%08x/%d\n",
+                      current->comm, preempt_count(), current->pid);
+               BUG();
+       }
+
        result = down_trylock(&mp->km_sem); /* returns 0 if acquired */
        if (result == 0) {
                BUG_ON(mp->km_owner != NULL);
index 1cfa369ba1d0650e58fc94c5bcdb4769c82a5d0d..41968d76a1fe89e30ae8fbee00eeec81947951b4 100644 (file)
@@ -1,4 +1,5 @@
 #include <sys/thread.h>
+#include <sys/kmem.h>
 
 /*
  * Thread interfaces
@@ -10,9 +11,6 @@ typedef struct thread_priv_s {
        size_t tp_len;                  /* Len to be passed to function */
        int tp_state;                   /* State to start thread at */
        pri_t tp_pri;                   /* Priority to start threat at */
-       volatile kthread_t *tp_task;    /* Task pointer for new thread */
-       spinlock_t tp_lock;             /* Syncronization lock */
-        wait_queue_head_t tp_waitq;    /* Syncronization wait queue */
 } thread_priv_t;
 
 static int
@@ -22,21 +20,13 @@ thread_generic_wrapper(void *arg)
        void (*func)(void *);
        void *args;
 
-        spin_lock(&tp->tp_lock);
        BUG_ON(tp->tp_magic != TP_MAGIC);
        func = tp->tp_func;
        args = tp->tp_args;
-       tp->tp_task = get_current();
        set_current_state(tp->tp_state);
-       set_user_nice((kthread_t *)tp->tp_task, PRIO_TO_NICE(tp->tp_pri));
+       set_user_nice((kthread_t *)get_current(), PRIO_TO_NICE(tp->tp_pri));
+       kmem_free(arg, sizeof(thread_priv_t));
 
-        spin_unlock(&tp->tp_lock);
-       wake_up(&tp->tp_waitq);
-
-       /* DO NOT USE 'ARG' AFTER THIS POINT, EVER, EVER, EVER!
-        * Local variables are used here because after the calling thread
-        * has been woken up it will exit and this memory will no longer
-        * be safe to access since it was declared on the callers stack. */
        if (func)
                func(args);
 
@@ -59,7 +49,7 @@ __thread_create(caddr_t stk, size_t  stksize, thread_func_t func,
                const char *name, void *args, size_t len, int *pp,
                int state, pri_t pri)
 {
-       thread_priv_t tp;
+       thread_priv_t *tp;
        DEFINE_WAIT(wait);
        struct task_struct *tsk;
 
@@ -68,45 +58,24 @@ __thread_create(caddr_t stk, size_t  stksize, thread_func_t func,
        BUG_ON(stk != NULL);
        BUG_ON(stk != 0);
 
-       /* Variable tp is located on the stack and not the heap because I want
-        * to minimize any chance of a failure, since the Solaris code is designed
-        * such that this function cannot fail.  This is a little dangerous since
-        * we're passing a stack address to a new thread but correct locking was
-        * added to ensure the callee can use the data safely until wake_up(). */
-       tp.tp_magic = TP_MAGIC;
-       tp.tp_func  = func;
-       tp.tp_args  = args;
-       tp.tp_len   = len;
-       tp.tp_state = state;
-       tp.tp_pri   = pri;
-       tp.tp_task  = NULL;
-       spin_lock_init(&tp.tp_lock);
-        init_waitqueue_head(&tp.tp_waitq);
+       tp = kmem_alloc(sizeof(thread_priv_t), KM_SLEEP);
+       if (tp == NULL)
+               return NULL;
 
-       spin_lock(&tp.tp_lock);
+       tp->tp_magic = TP_MAGIC;
+       tp->tp_func  = func;
+       tp->tp_args  = args;
+       tp->tp_len   = len;
+       tp->tp_state = state;
+       tp->tp_pri   = pri;
 
-       tsk = kthread_create(thread_generic_wrapper, (void *)&tp, "%s", name);
+       tsk = kthread_create(thread_generic_wrapper, (void *)tp, "%s", name);
        if (IS_ERR(tsk)) {
                printk("spl: Failed to create thread: %ld\n", PTR_ERR(tsk));
                return NULL;
        }
 
        wake_up_process(tsk);
-
-       /* All signals are ignored due to sleeping TASK_UNINTERRUPTIBLE */
-       for (;;) {
-               prepare_to_wait(&tp.tp_waitq, &wait, TASK_UNINTERRUPTIBLE);
-               if (tp.tp_task != NULL)
-                       break;
-
-               spin_unlock(&tp.tp_lock);
-               schedule();
-               spin_lock(&tp.tp_lock);
-       }
-
-       BUG_ON(tsk != tp.tp_task); /* Extra paranoia */
-       spin_unlock(&tp.tp_lock);
-
-       return (kthread_t *)tp.tp_task;
+       return (kthread_t *)tsk;
 }
 EXPORT_SYMBOL(__thread_create);
index dec251efe744d19968bb467ab0ae23f81efcf68b..3cea573ed5bda7d62a3a0a01eca4963d4f698330 100644 (file)
@@ -6,7 +6,11 @@
 
 #define SPLAT_THREAD_TEST1_ID          0x0601
 #define SPLAT_THREAD_TEST1_NAME                "create"
-#define SPLAT_THREAD_TEST1_DESC                "Validate thread creation and destruction"
+#define SPLAT_THREAD_TEST1_DESC                "Validate thread creation"
+
+#define SPLAT_THREAD_TEST2_ID          0x0602
+#define SPLAT_THREAD_TEST2_NAME                "exit"
+#define SPLAT_THREAD_TEST2_DESC                "Validate thread exit"
 
 #define SPLAT_THREAD_TEST_MAGIC                0x4488CC00UL
 
@@ -18,19 +22,29 @@ typedef struct thread_priv {
        int tp_rc;
 } thread_priv_t;
 
+static int
+splat_thread_rc(thread_priv_t *tp, int rc)
+{
+       int ret;
+
+       spin_lock(&tp->tp_lock);
+       ret = (tp->tp_rc == rc);
+       spin_unlock(&tp->tp_lock);
+
+       return ret;
+}
 
 static void
-splat_thread_work(void *priv)
+splat_thread_work1(void *priv)
 {
        thread_priv_t *tp = (thread_priv_t *)priv;
 
        spin_lock(&tp->tp_lock);
        ASSERT(tp->tp_magic == SPLAT_THREAD_TEST_MAGIC);
        tp->tp_rc = 1;
-
        spin_unlock(&tp->tp_lock);
-       wake_up(&tp->tp_waitq);
 
+       wake_up(&tp->tp_waitq);
        thread_exit();
 }
 
@@ -40,7 +54,6 @@ splat_thread_test1(struct file *file, void *arg)
        thread_priv_t tp;
         DEFINE_WAIT(wait);
        kthread_t *thr;
-       int rc = 0;
 
        tp.tp_magic = SPLAT_THREAD_TEST_MAGIC;
        tp.tp_file = file;
@@ -48,31 +61,79 @@ splat_thread_test1(struct file *file, void *arg)
        init_waitqueue_head(&tp.tp_waitq);
        tp.tp_rc = 0;
 
-       spin_lock(&tp.tp_lock);
-
-       thr = (kthread_t *)thread_create(NULL, 0, splat_thread_work, &tp, 0,
+       thr = (kthread_t *)thread_create(NULL, 0, splat_thread_work1, &tp, 0,
                                         &p0, TS_RUN, minclsyspri);
-       /* Must never fail under Solaris, but we check anyway so we can
-        * report an error when this impossible thing happens */
-       if (thr == NULL) {
-               rc = -ESRCH;
-               goto out;
-       }
-
-        for (;;) {
-                prepare_to_wait(&tp.tp_waitq, &wait, TASK_UNINTERRUPTIBLE);
-                if (tp.tp_rc)
-                        break;
+       /* Must never fail under Solaris, but we check anyway since this
+        * can happen in the linux SPL, we may want to change this behavior */
+       if (thr == NULL)
+               return  -ESRCH;
 
-                spin_unlock(&tp.tp_lock);
-                schedule();
-                spin_lock(&tp.tp_lock);
-        }
+       /* Sleep until the thread sets tp.tp_rc == 1 */
+       wait_event(tp.tp_waitq, splat_thread_rc(&tp, 1));
 
         splat_vprint(file, SPLAT_THREAD_TEST1_NAME, "%s",
-                  "Thread successfully started and exited cleanly\n");
-out:
-       spin_unlock(&tp.tp_lock);
+                  "Thread successfully started properly\n");
+       return 0;
+}
+
+static void
+splat_thread_work2(void *priv)
+{
+       thread_priv_t *tp = (thread_priv_t *)priv;
+
+       spin_lock(&tp->tp_lock);
+       ASSERT(tp->tp_magic == SPLAT_THREAD_TEST_MAGIC);
+       tp->tp_rc = 1;
+       spin_unlock(&tp->tp_lock);
+
+       wake_up(&tp->tp_waitq);
+       thread_exit();
+
+       /* The following code is unreachable when thread_exit() is
+        * working properly, which is exactly what we're testing */
+       spin_lock(&tp->tp_lock);
+       tp->tp_rc = 2;
+       spin_unlock(&tp->tp_lock);
+
+       wake_up(&tp->tp_waitq);
+}
+
+static int
+splat_thread_test2(struct file *file, void *arg)
+{
+       thread_priv_t tp;
+        DEFINE_WAIT(wait);
+       kthread_t *thr;
+       int rc = 0;
+
+       tp.tp_magic = SPLAT_THREAD_TEST_MAGIC;
+       tp.tp_file = file;
+        spin_lock_init(&tp.tp_lock);
+       init_waitqueue_head(&tp.tp_waitq);
+       tp.tp_rc = 0;
+
+       thr = (kthread_t *)thread_create(NULL, 0, splat_thread_work2, &tp, 0,
+                                        &p0, TS_RUN, minclsyspri);
+       /* Must never fail under Solaris, but we check anyway since this
+        * can happen in the linux SPL, we may want to change this behavior */
+       if (thr == NULL)
+               return  -ESRCH;
+
+       /* Sleep until the thread sets tp.tp_rc == 1 */
+       wait_event(tp.tp_waitq, splat_thread_rc(&tp, 1));
+
+       /* Sleep until the thread sets tp.tp_rc == 2, or until we hit
+        * the timeout.  If thread exit is working properly we should
+        * hit the timeout and never see to.tp_rc == 2. */
+       rc = wait_event_timeout(tp.tp_waitq, splat_thread_rc(&tp, 2), HZ / 10);
+       if (rc > 0) {
+               rc = -EINVAL;
+               splat_vprint(file, SPLAT_THREAD_TEST2_NAME, "%s",
+                          "Thread did not exit properly at thread_exit()\n");
+       } else {
+               splat_vprint(file, SPLAT_THREAD_TEST2_NAME, "%s",
+                          "Thread successfully exited at thread_exit()\n");
+       }
 
        return rc;
 }
@@ -96,6 +157,8 @@ splat_thread_init(void)
 
         SPLAT_TEST_INIT(sub, SPLAT_THREAD_TEST1_NAME, SPLAT_THREAD_TEST1_DESC,
                       SPLAT_THREAD_TEST1_ID, splat_thread_test1);
+        SPLAT_TEST_INIT(sub, SPLAT_THREAD_TEST2_NAME, SPLAT_THREAD_TEST2_DESC,
+                      SPLAT_THREAD_TEST2_ID, splat_thread_test2);
 
         return sub;
 }
@@ -104,6 +167,7 @@ void
 splat_thread_fini(splat_subsystem_t *sub)
 {
         ASSERT(sub);
+        SPLAT_TEST_FINI(sub, SPLAT_THREAD_TEST2_ID);
         SPLAT_TEST_FINI(sub, SPLAT_THREAD_TEST1_ID);
 
         kfree(sub);