lkml.org 
[lkml]   [2006]   [Feb]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] rxrpc: use kthread_ API
On Tue, Feb 14, 2006 at 06:20:12PM +0000, David Howells wrote:
> Christoph Hellwig <hch@lst.de> wrote:
> > Use the kthread_ API instead of opencoding lots of hairy code for kernel
> > thread creation and teardown.
>
> I take it daemonize() does not now need to be called because the new process
> derives from keventd.

Yes;. And does the others things daemonize did by hand.

> You've also broken things... The waitqueue is there for a specific reason:
> namely so that I can have multiple threads.

Currenyly calling rxprc_krx*_init can't be done twice as there's various
file-scope things that would need changing. Once you actually need
multiple threads and need to change that changing the wake_up_process
back to waitqueues can be done easily - until then it's a nice
optimization.

> > +static struct task_struct *krxiod_thread;
>
> That should be rxrpc_krxiod_thread.

Ok, fixed all the naming bits.

> > + krxiod_thread = kthread_run(rxrpc_krxiod, NULL, "krxiod");
> > + if (IS_ERR(krxiod_thread))
> > + return PTR_ERR(krxiod_thread);
> > + return 0;
>
> Don't assign an error to (rxrpc_)krxiod_thread.

Well, kthread_run can returns errors. If you want to avoid that for
some reasons we'd need a local varibale, which would be rather silly.
Note that there's nothing it could pollute, once one of these fails
rxrpc_initialise goes to the error path, unwinds and returns a failure,
so the rxrpc module never goes live.


Index: linux-2.6/net/rxrpc/krxiod.c
===================================================================
--- linux-2.6.orig/net/rxrpc/krxiod.c 2006-02-14 16:23:31.000000000 +0100
+++ linux-2.6/net/rxrpc/krxiod.c 2006-02-14 20:46:21.000000000 +0100
@@ -13,14 +13,15 @@
#include <linux/completion.h>
#include <linux/spinlock.h>
#include <linux/init.h>
+#include <linux/kthread.h>
#include <rxrpc/krxiod.h>
#include <rxrpc/transport.h>
#include <rxrpc/peer.h>
#include <rxrpc/call.h>
#include "internal.h"

-static DECLARE_WAIT_QUEUE_HEAD(rxrpc_krxiod_sleepq);
-static DECLARE_COMPLETION(rxrpc_krxiod_dead);
+
+static struct task_struct *rxrpc_krxiod_thread;

static atomic_t rxrpc_krxiod_qcount = ATOMIC_INIT(0);

@@ -30,20 +31,14 @@
static LIST_HEAD(rxrpc_krxiod_callq);
static DEFINE_SPINLOCK(rxrpc_krxiod_callq_lock);

-static volatile int rxrpc_krxiod_die;
-
/*****************************************************************************/
/*
* Rx I/O daemon
*/
static int rxrpc_krxiod(void *arg)
{
- DECLARE_WAITQUEUE(krxiod,current);
-
printk("Started krxiod %d\n",current->pid);

- daemonize("krxiod");
-
/* loop around waiting for work to do */
do {
/* wait for work or to be told to exit */
@@ -51,19 +46,16 @@
if (!atomic_read(&rxrpc_krxiod_qcount)) {
set_current_state(TASK_INTERRUPTIBLE);

- add_wait_queue(&rxrpc_krxiod_sleepq, &krxiod);
-
for (;;) {
set_current_state(TASK_INTERRUPTIBLE);
if (atomic_read(&rxrpc_krxiod_qcount) ||
- rxrpc_krxiod_die ||
+ kthread_should_stop() ||
signal_pending(current))
break;

schedule();
}

- remove_wait_queue(&rxrpc_krxiod_sleepq, &krxiod);
set_current_state(TASK_RUNNING);
}
_debug("### End Wait");
@@ -143,11 +135,9 @@
/* discard pending signals */
rxrpc_discard_my_signals();

- } while (!rxrpc_krxiod_die);
-
- /* and that's all */
- complete_and_exit(&rxrpc_krxiod_dead, 0);
+ } while (!kthread_should_stop());

+ return 0;
} /* end rxrpc_krxiod() */

/*****************************************************************************/
@@ -156,7 +146,10 @@
*/
int __init rxrpc_krxiod_init(void)
{
- return kernel_thread(rxrpc_krxiod, NULL, 0);
+ rxrpc_krxiod_thread = kthread_run(rxrpc_krxiod, NULL, "krxiod");
+ if (IS_ERR(rxrpc_krxiod_thread))
+ return PTR_ERR(rxrpc_krxiod_thread);
+ return 0;

} /* end rxrpc_krxiod_init() */

@@ -166,10 +159,7 @@
*/
void rxrpc_krxiod_kill(void)
{
- rxrpc_krxiod_die = 1;
- wake_up_all(&rxrpc_krxiod_sleepq);
- wait_for_completion(&rxrpc_krxiod_dead);
-
+ kthread_stop(rxrpc_krxiod_thread);
} /* end rxrpc_krxiod_kill() */

/*****************************************************************************/
@@ -194,7 +184,7 @@
}

spin_unlock_irqrestore(&rxrpc_krxiod_transportq_lock, flags);
- wake_up_all(&rxrpc_krxiod_sleepq);
+ wake_up_process(rxrpc_krxiod_thread);
}

_leave("");
@@ -239,7 +229,7 @@
}
spin_unlock_irqrestore(&rxrpc_krxiod_callq_lock, flags);
}
- wake_up_all(&rxrpc_krxiod_sleepq);
+ wake_up_process(rxrpc_krxiod_thread);

} /* end rxrpc_krxiod_queue_call() */

Index: linux-2.6/net/rxrpc/krxsecd.c
===================================================================
--- linux-2.6.orig/net/rxrpc/krxsecd.c 2006-02-14 16:23:31.000000000 +0100
+++ linux-2.6/net/rxrpc/krxsecd.c 2006-02-14 20:46:45.000000000 +0100
@@ -19,6 +19,7 @@
#include <linux/completion.h>
#include <linux/spinlock.h>
#include <linux/init.h>
+#include <linux/kthread.h>
#include <rxrpc/krxsecd.h>
#include <rxrpc/transport.h>
#include <rxrpc/connection.h>
@@ -30,8 +31,8 @@
#include <net/sock.h>
#include "internal.h"

-static DECLARE_WAIT_QUEUE_HEAD(rxrpc_krxsecd_sleepq);
-static DECLARE_COMPLETION(rxrpc_krxsecd_dead);
+
+static struct task_struct *rxrpc_krxsecd_thread;
static volatile int rxrpc_krxsecd_die;

static atomic_t rxrpc_krxsecd_qcount;
@@ -49,14 +50,10 @@
*/
static int rxrpc_krxsecd(void *arg)
{
- DECLARE_WAITQUEUE(krxsecd, current);
-
int die;

printk("Started krxsecd %d\n", current->pid);

- daemonize("krxsecd");
-
/* loop around waiting for work to do */
do {
/* wait for work or to be told to exit */
@@ -64,22 +61,19 @@
if (!atomic_read(&rxrpc_krxsecd_qcount)) {
set_current_state(TASK_INTERRUPTIBLE);

- add_wait_queue(&rxrpc_krxsecd_sleepq, &krxsecd);
-
for (;;) {
set_current_state(TASK_INTERRUPTIBLE);
if (atomic_read(&rxrpc_krxsecd_qcount) ||
- rxrpc_krxsecd_die ||
+ kthread_should_stop() ||
signal_pending(current))
break;

schedule();
}

- remove_wait_queue(&rxrpc_krxsecd_sleepq, &krxsecd);
set_current_state(TASK_RUNNING);
}
- die = rxrpc_krxsecd_die;
+ die = kthread_should_stop();
_debug("### End Wait");

/* see if there're incoming calls in need of authenticating */
@@ -114,9 +108,8 @@

} while (!die);

- /* and that's all */
- complete_and_exit(&rxrpc_krxsecd_dead, 0);

+ return 0;
} /* end rxrpc_krxsecd() */

/*****************************************************************************/
@@ -125,7 +118,10 @@
*/
int __init rxrpc_krxsecd_init(void)
{
- return kernel_thread(rxrpc_krxsecd, NULL, 0);
+ rxrpc_krxsecd_thread = kthread_run(rxrpc_krxsecd, NULL, "krxsecd");
+ if (IS_ERR(rxrpc_krxsecd_thread))
+ return PTR_ERR(rxrpc_krxsecd_thread);
+ return 0;

} /* end rxrpc_krxsecd_init() */

@@ -136,9 +132,7 @@
void rxrpc_krxsecd_kill(void)
{
rxrpc_krxsecd_die = 1;
- wake_up_all(&rxrpc_krxsecd_sleepq);
- wait_for_completion(&rxrpc_krxsecd_dead);
-
+ kthread_stop(rxrpc_krxsecd_thread);
} /* end rxrpc_krxsecd_kill() */

/*****************************************************************************/
@@ -197,7 +191,7 @@

spin_unlock(&rxrpc_krxsecd_initmsgq_lock);

- wake_up(&rxrpc_krxsecd_sleepq);
+ wake_up_process(rxrpc_krxsecd_thread);

_leave("");
} /* end rxrpc_krxsecd_queue_incoming_call() */
Index: linux-2.6/net/rxrpc/krxtimod.c
===================================================================
--- linux-2.6.orig/net/rxrpc/krxtimod.c 2006-02-14 16:23:31.000000000 +0100
+++ linux-2.6/net/rxrpc/krxtimod.c 2006-02-14 20:47:11.000000000 +0100
@@ -13,15 +13,14 @@
#include <linux/init.h>
#include <linux/sched.h>
#include <linux/completion.h>
+#include <linux/kthread.h>
#include <rxrpc/rxrpc.h>
#include <rxrpc/krxtimod.h>
#include <asm/errno.h>
#include "internal.h"

-static DECLARE_COMPLETION(krxtimod_alive);
-static DECLARE_COMPLETION(krxtimod_dead);
-static DECLARE_WAIT_QUEUE_HEAD(krxtimod_sleepq);
-static int krxtimod_die;
+
+static struct task_struct *rxrpc_krxtimod_thread;

static LIST_HEAD(krxtimod_list);
static DEFINE_SPINLOCK(krxtimod_lock);
@@ -34,15 +33,10 @@
*/
int rxrpc_krxtimod_start(void)
{
- int ret;
-
- ret = kernel_thread(krxtimod, NULL, 0);
- if (ret < 0)
- return ret;
-
- wait_for_completion(&krxtimod_alive);
-
- return ret;
+ rxrpc_krxtimod_thread = kthread_run(krxtimod, NULL, "krxtimod");
+ if (IS_ERR(rxrpc_krxtimod_thread))
+ return PTR_ERR(rxrpc_krxtimod_thread);
+ return 0;
} /* end rxrpc_krxtimod_start() */

/*****************************************************************************/
@@ -51,11 +45,7 @@
*/
void rxrpc_krxtimod_kill(void)
{
- /* get rid of my daemon */
- krxtimod_die = 1;
- wake_up(&krxtimod_sleepq);
- wait_for_completion(&krxtimod_dead);
-
+ kthread_stop(rxrpc_krxtimod_thread);
} /* end rxrpc_krxtimod_kill() */

/*****************************************************************************/
@@ -64,30 +54,22 @@
*/
static int krxtimod(void *arg)
{
- DECLARE_WAITQUEUE(myself, current);
-
rxrpc_timer_t *timer;

printk("Started krxtimod %d\n", current->pid);

- daemonize("krxtimod");
-
- complete(&krxtimod_alive);
-
/* loop around looking for things to attend to */
loop:
set_current_state(TASK_INTERRUPTIBLE);
- add_wait_queue(&krxtimod_sleepq, &myself);

for (;;) {
unsigned long jif;
long timeout;

/* deal with the server being asked to die */
- if (krxtimod_die) {
- remove_wait_queue(&krxtimod_sleepq, &myself);
+ if (kthread_should_stop()) {
_leave("");
- complete_and_exit(&krxtimod_dead, 0);
+ return 0;
}

try_to_freeze();
@@ -125,7 +107,6 @@
* entry
*/
immediate:
- remove_wait_queue(&krxtimod_sleepq, &myself);
set_current_state(TASK_RUNNING);

_debug("@@@ Begin Timeout of %p", timer);
@@ -171,7 +152,7 @@

spin_unlock(&krxtimod_lock);

- wake_up(&krxtimod_sleepq);
+ wake_up_process(rxrpc_krxtimod_thread);

_leave("");
} /* end rxrpc_krxtimod_add_timer() */
@@ -196,7 +177,7 @@

spin_unlock(&krxtimod_lock);

- wake_up(&krxtimod_sleepq);
+ wake_up_process(rxrpc_krxtimod_thread);

_leave(" = %d", ret);
return ret;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2006-02-14 20:54    [from the cache]
©2003-2011 Jasper Spaans