http://www.gossamer-threads.com/lists/linux/kernel/758335?search_string=Remaining%20straight%20forward%20kthread%20API%20conversions...%20;#


The following patches are against 2.6.21.rc6-mm1.
Hopefully that is enough to catch most of the recent development
activity.

I am aiming to remove all kernel threads that handle signals
from user space, to remove all calls to daemonize and kernel_thread
from non-core kernel code.

kernel thrreads handling signals from user space is a problem because
it makes the kernel thread part of the user/kernel API which make
changing things difficult and it breaks as soon as you are inside of
a pid namespace because you won't be able to see your kernel thread.

Calling kernel_thread has problems because it returns a pid_t value
which once we get to the pid namespace is context depending so it
cannot be used to globally identify a process. kernel_thread is
also a problem because it traps user space state and requires us
to call daemonize to free that state.

daemonize is a maintenance problem because every time you play with
user space state and limiting things you need to remember to update
daemonize. Occasionally it has taken years like in the case of the
mount namespace before someone realizes they need to update it.
With the kthread api we no longer need daemonize.

In addition we don't want kernel threads visible in anything but
the initial pid namespace or they will hold a reference to a
child pid namespace. However calling kernel_thread from a non-kernel
parent in a child pid namespace will give the thread a pid in
the child pid namespace, and there is nothing daemonize can do about
it. So daemonize appears impossible to support going forward, and
I choose to remove all of it's callers rather than attempt to support
it.

Eric



Looks like you were missing at least the pcie hotplug driver. Another
one of the horrible thread abuses in drivers/pci/hotpug.

- full conversion to kthread infrastructure
- use wake_up_process to wake the thread up

Like most pci hotplug drivers it still uses very race non-atomic variable
assignment to communicated with the thread, but that's something the
maintainers should look into.


Signed-off-by: Christoph Hellwig <hch[at]lst.de>

Index: linux-2.6/drivers/pci/hotplug/pciehp_ctrl.c
===================================================================
--- linux-2.6.orig/drivers/pci/hotplug/pciehp_ctrl.c 2007-04-22 11:36:58.000000000 +0200
+++ linux-2.6/drivers/pci/hotplug/pciehp_ctrl.c 2007-04-22 11:42:56.000000000 +0200
@@ -32,14 +32,13 @@
#include <linux/types.h>
#include <linux/smp_lock.h>
#include <linux/pci.h>
+#include <linux/kthread.h>
#include "../pci.h"
#include "pciehp.h"

static void interrupt_event_handler(struct controller *ctrl);

-static struct semaphore event_semaphore; /* mutex for process loop (up if something to process) */
-static struct semaphore event_exit; /* guard ensure thread has exited before calling it quits */
-static int event_finished;
+static struct task_struct *pciehpd_event_thread;
static unsigned long pushbutton_pending; /* = 0 */
static unsigned long surprise_rm_pending; /* = 0 */

@@ -93,8 +92,9 @@ u8 pciehp_handle_attention_button(u8 hp_
info("Button ignore on Slot(%s)\n", slot_name(p_slot));
}

+ /* signal event thread that new event is posted */
if (rc)
- up(&event_semaphore); /* signal event thread that new event is posted */
+ wake_up_process(pciehpd_event_thread);

return 0;

@@ -135,8 +135,9 @@ u8 pciehp_handle_switch_change(u8 hp_slo
taskInfo->event_type = INT_SWITCH_CLOSE;
}

+ /* signal event thread that new event is posted */
if (rc)
- up(&event_semaphore); /* signal event thread that new event is posted */
+ wake_up_process(pciehpd_event_thread);

return rc;
}
@@ -178,8 +179,9 @@ u8 pciehp_handle_presence_change(u8 hp_s
taskInfo->event_type = INT_PRESENCE_OFF;
}

+ /* signal event thread that new event is posted */
if (rc)
- up(&event_semaphore); /* signal event thread that new event is posted */
+ wake_up_process(pciehpd_event_thread);

return rc;
}
@@ -217,8 +219,10 @@ u8 pciehp_handle_power_fault(u8 hp_slot,
taskInfo->event_type = INT_POWER_FAULT;
info("power fault bit %x set\n", hp_slot);
}
+
+ /* signal event thread that new event is posted */
if (rc)
- up(&event_semaphore); /* signal event thread that new event is posted */
+ wake_up_process(pciehpd_event_thread);

return rc;
}
@@ -362,7 +366,7 @@ static void pushbutton_helper_thread(uns
{
pushbutton_pending = data;

- up(&event_semaphore);
+ wake_up_process(pciehpd_event_thread);
}

/**
@@ -452,19 +456,14 @@ static void pciehp_surprise_rm_thread(un


/* this is the main worker thread */
-static int event_thread(void* data)
+static int event_thread(void *data)
{
struct controller *ctrl;
- lock_kernel();
- daemonize("pciehpd_event");
-
- unlock_kernel();

while (1) {
- dbg("!!!!event_thread sleeping\n");
- down_interruptible (&event_semaphore);
- dbg("event_thread woken finished = %d\n", event_finished);
- if (event_finished || signal_pending(current))
+ set_current_state(TASK_INTERRUPTIBLE);
+ schedule();
+ if (kthread_should_stop())
break;
/* Do stuff here */
if (pushbutton_pending)
@@ -476,24 +475,15 @@ static int event_thread(void* data)
interrupt_event_handler(ctrl);
}
dbg("event_thread signals exit\n");
- up(&event_exit);
return 0;
}

int pciehp_event_start_thread(void)
{
- int pid;
-
- /* initialize our semaphores */
- init_MUTEX_LOCKED(&event_exit);
- event_finished=0;
-
- init_MUTEX_LOCKED(&event_semaphore);
- pid = kernel_thread(event_thread, NULL, 0);
-
- if (pid < 0) {
+ pciehpd_event_thread = kthread_run(event_thread, NULL, "pciehpd_event");
+ if (IS_ERR(pciehpd_event_thread)) {
err ("Can't start up our event thread\n");
- return -1;
+ return PTR_ERR(pciehpd_event_thread);
}
return 0;
}
@@ -501,9 +491,7 @@ int pciehp_event_start_thread(void)

void pciehp_event_stop_thread(void)
{
- event_finished = 1;
- up(&event_semaphore);
- down(&event_exit);
+ kthread_stop(pciehpd_event_thread);
}


@@ -624,7 +612,7 @@ static void interrupt_event_handler(stru
dbg("Surprise Removal\n");
if (p_slot) {
surprise_rm_pending = (unsigned long) p_slot;
- up(&event_semaphore);
+ wake_up_process(pciehpd_event_thread);
update_slot_info(p_slot);
}
}


Reply via email to