How to unload a kernel module which has created kproc/kthread in FreeBSD

Question

I want to unload a module that has threads. I have referenced the codes in dev/random, and my code is like this:

$ cat tmp.c 

#include <sys/param.h>
#include <sys/module.h>
#include <sys/kernel.h>
#include <sys/systm.h>
#include <sys/proc.h>
#include <sys/kthread.h>

/* 
 * $ ps auxH | grep kproc 
 */ 

static int kproc_control = 1; 

#define output_id(p, td, fmt, args...)                                  \ 
        printf("%s[%d]:%s[%d]:[%s] %s\n", p->p_comm, p->p_pid,          \ 
            td->td_name, td->td_tid, __func__, msg) 

static void thread_routine(void *arg) 
{ 
        char *msg = arg; 
        struct thread *td = curthread; 
        struct proc *p = td->td_proc; 

        output_id(p, td, msg); 
        pause("-", hz * 100); 
        output_id(p, td, msg); 

        kthread_exit(); 
} 

static void proc_routine(void *arg) 
{ 
        char *msg = arg; 
        struct thread *td = curthread; 
        struct proc *p = td->td_proc; 
        struct thread *ntd; 
        int error; 

        output_id(p, td, msg); 

        error = kthread_add(thread_routine, "I'm kthread", p, &ntd, 
            0, 0, "kthread"); 
        if (error) 
                printf("error: %d\n", error); 

        while (kproc_control >= 0) { 
                pause("-", hz / 10); 
        } 

        wakeup(&kproc_control); 
        kproc_exit(0); 
} 

static int foobar_init(void) 
{ 
        int error; 
        struct proc *p; 

        error = kproc_create(proc_routine, "I'm kproc", &p, 0, 0, "kproc"); 
        uprintf("error: %d\n", error); 

        return error; 
} 

static void foobar_fini(void) 
{ 
        kproc_control = -1; 
        tsleep(&kproc_control, 0, "term", 0); 
        //pause("delay", 2 * hz); 
} 

static int 
foobar_modevent(module_t mod __unused, int event, void *arg __unused) 
{ 
        int error = 0; 

        switch (event) { 
        case MOD_LOAD: 
                error = foobar_init(); 
                break; 
        case MOD_UNLOAD: 
                foobar_fini(); 
                break; 
        default: 
                error = EOPNOTSUPP; 
                break; 
        } 

        return (error); 
} 

static moduledata_t foobar_mod = { 
        "foobar", 
        foobar_modevent, 
        NULL 
}; 

DECLARE_MODULE(foobar, foobar_mod, SI_SUB_DRIVERS, SI_ORDER_MIDDLE); 

When I unload it by kldunload, my kernel crashes and system reboots. What is correct way to solve this problem? Any comments will be appreciated! ;-)

PS. Could I sleep on &p->p_stype? I see the following codes in exit1():

    /*
     * Note that we are exiting and do another wakeup of anyone in
     * PIOCWAIT in case they aren't listening for S_EXIT stops or
     * decided to wait again after we told them we are exiting.
     */
    p->p_flag |= P_WEXIT;
    wakeup(&p->p_stype);

PS. Updated codes:

#include <sys/param.h>
#include <sys/module.h>
#include <sys/kernel.h>
#include <sys/systm.h>
#include <sys/proc.h>
#include <sys/kthread.h>
#include <sys/lock.h>
#include <sys/mutex.h>

/*
 * $ ps auxH | grep kproc
 */

static int kproc_control = 1;
static struct proc *foobar_proc;
static struct mtx mtx;

#define output_id(p, td, fmt, args...)                                  \
        printf("%s[%d]:%s[%d]:[%s] %s\n", p->p_comm, p->p_pid,          \
            td->td_name, td->td_tid, __func__, msg)

static void thread_routine(void *arg)
{
        char *msg = arg;
        struct thread *td = curthread;
        struct proc *p = td->td_proc;

        output_id(p, td, msg);
        pause("-", hz * 100);
        output_id(p, td, msg);

        kthread_exit();
}

static void proc_routine(void *arg)
{
        char *msg = arg;
        struct thread *td = curthread;
        struct proc *p = td->td_proc;
        struct thread *ntd;
        int error;

        output_id(p, td, msg);

        error = kthread_add(thread_routine, "I'm kthread", p, &ntd,
            0, 0, "kthread");
        if (error)
                printf("error: %d\n", error);

        mtx_lock(&mtx);
        while (kproc_control >= 0) {
                mtx_unlock(&mtx);
                pause("-", hz / 10);
                mtx_lock(&mtx);
        }
        mtx_unlock(&mtx);
        kproc_exit(0);
}
static int foobar_init(void)
{
        int error;

        mtx_init(&mtx, "foobar_mtx", NULL, MTX_DEF);
        error = kproc_create(proc_routine, "I'm kproc", &foobar_proc, 0, 0, "kproc");
        uprintf("error: %d\n", error);

        return error;
}

static void foobar_fini(void)
{
        mtx_lock(&mtx);
        kproc_control = -1;
        //mtx_sleep(foobar_proc, &mtx, 0, "waiting", 0);
        mtx_sleep(&foobar_proc->p_stype, &mtx, 0, "waiting", 0);
}

static int
foobar_modevent(module_t mod __unused, int event, void *arg __unused)
{
        int error = 0;

        switch (event) {
        case MOD_LOAD:
                error = foobar_init();
                break;
        case MOD_UNLOAD:
                foobar_fini();
                break;
        default:
                error = EOPNOTSUPP;
                break;
        }

        return (error);
}

static moduledata_t foobar_mod = {
        "foobar",
        foobar_modevent,
        NULL
};

DECLARE_MODULE(foobar, foobar_mod, SI_SUB_DRIVERS, SI_ORDER_MIDDLE);

Answer 1

In principle you wait on the thread or the proc handle to make sure it has exited.

You should use a lock of some kind to synchronise access to shared variables like your kproc_control flag. You can also then use mtx_sleep to atomically release the lock and wait on the proc handle to avoid race conditions in dealing with the termination events.

The model I use looks something like this:

void proc(whatver)
{
    mtx_lock(&sc->m_lock);

    while (!sc->time_to_die) {
        mtx_unlock(&sc->m_lock);
        /* Do whatever */
        mtx_lock(&sc->m_lock);
    }

    mtx_unlock(&sc->m_lock);

    kproc_exit(0);
}

void detach(whatever)
{
    mtx_lock(&sc->m_lock);
    sc->time_to_die = 1;
    mtx_sleep(sc->m_proc, &sc->m_lock, 0, "waiting", 0);
    /* proc cleaned up, safe to continue */
}