David Woodhouse wrote:
> I'm not sure why we changed from the existing state machine / timer setup to
> sleeping in the PCMCIA parse_events() routine,
I don't remember the exact reason, but Linus changed it for yenta...
maybe because CardBus Watcher always called the state machine from user
context...
> +static int pcmcia_event_thread(void * dummy)
> +{
> + DECLARE_WAITQUEUE(wait, current);
> +
> + current->session = 1;
> + current->pgrp = 1;
> + strcpy(current->comm, "kpcmciad");
> + current->tty = NULL;
> + spin_lock_irq(¤t->sigmask_lock);
> + sigfillset(¤t->blocked);
> + recalc_sigpending(current);
> + spin_unlock_irq(¤t->sigmask_lock);
> + exit_mm(current);
> + exit_files(current);
> + exit_sighand(current);
> + exit_fs(current);
Replace most if not all of this crap w/ daemonize()
> +
> + while(!event_thread_leaving) {
> + void *active;
> +
> + set_current_state(TASK_INTERRUPTIBLE);
> + add_wait_queue(&event_thread_wq, &wait);
> +
> + /* Don't really need locking. But the implied mb() */
> + spin_lock(&tqueue_lock);
> + active = tq_pcmcia;
> + spin_unlock(&tqueue_lock);
> +
> + if (!active)
> + schedule();
> +
> + set_current_state(TASK_RUNNING);
> + remove_wait_queue(&event_thread_wq, &wait);
> +
> + run_task_queue(&tq_pcmcia);
> + }
it looks like the loop can be simplified to
while (1) {
mb();
active = tq_pcmcia;
if (!active)
interruptible_sleep_on(&event_thread_wq);
if (signal_pending(current)
break;
run_task_queue(&tq_pcmcia);
}
> + /* Need up_and_exit() */
> + up(&event_thread_exit_sem);
Racy. Use waitpid() in the thread killer instead. You also need to
reap the process, just like in userland...
> + /* Start the thread for handling queued events for socket drivers */
> + kernel_thread (pcmcia_event_thread, NULL,
> + CLONE_FS | CLONE_FILES | CLONE_SIGHAND);
Why are you cloning _FS, _FILES, and _SIGHAND? I don't see why the
third arg should not be zero. man clone...
--
Jeff Garzik |
Building 1024 | The chief enemy of creativity is "good" sense
MandrakeSoft | -- Picasso
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/