lkml.org 
[lkml]   [2000]   [Nov]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] pcmcia event thread. (fwd)
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(&current->sigmask_lock);
> + sigfillset(&current->blocked);
> + recalc_sigpending(current);
> + spin_unlock_irq(&current->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 majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 12:45    [W:0.117 / U:0.512 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site