diff --git a/include/sys/mutex.h b/include/sys/mutex.h index 2db4a7f96..ca76d6ea9 100644 --- a/include/sys/mutex.h +++ b/include/sys/mutex.h @@ -6,6 +6,7 @@ extern "C" { #endif #include +#include #include /* 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); diff --git a/modules/spl/spl-thread.c b/modules/spl/spl-thread.c index 1cfa369ba..41968d76a 100644 --- a/modules/spl/spl-thread.c +++ b/modules/spl/spl-thread.c @@ -1,4 +1,5 @@ #include +#include /* * 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); diff --git a/modules/splat/splat-thread.c b/modules/splat/splat-thread.c index dec251efe..3cea573ed 100644 --- a/modules/splat/splat-thread.c +++ b/modules/splat/splat-thread.c @@ -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,24 +22,84 @@ 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(); } static int splat_thread_test1(struct file *file, void *arg) +{ + thread_priv_t tp; + DEFINE_WAIT(wait); + kthread_t *thr; + + 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_work1, &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)); + + splat_vprint(file, SPLAT_THREAD_TEST1_NAME, "%s", + "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); @@ -48,32 +112,29 @@ 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_work2, &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; + /* 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"); } - for (;;) { - prepare_to_wait(&tp.tp_waitq, &wait, TASK_UNINTERRUPTIBLE); - if (tp.tp_rc) - break; - - spin_unlock(&tp.tp_lock); - schedule(); - spin_lock(&tp.tp_lock); - } - - splat_vprint(file, SPLAT_THREAD_TEST1_NAME, "%s", - "Thread successfully started and exited cleanly\n"); -out: - spin_unlock(&tp.tp_lock); - 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);