pktgen (was Re: tests of kernel modules)

15 messages, 5 authors, 2007-01-02 · open the first message on its own page

pktgen (was Re: tests of kernel modules)

From: Alexey Dobriyan <hidden>
Date: 2006-11-21 21:22:54

[CCing netdev, bug in pktgen]

	[build modular pktgen]
	while true; do modprobe pktgen && rmmod pktgen; done

	BUG: warning at fs/proc/generic.c:732/remove_proc_entry()
	 [<c016a7ad>] remove_proc_entry+0x161/0x1ca
	 [<e19969ab>] pg_cleanup+0xd5/0xdc [pktgen]
         [<c011fa3e>] autoremove_wake_function+0x0/0x35
	 [<c01280bd>] sys_delete_module+0x162/0x189
	 [<c0136500>] remove_vma+0x31/0x36
	 [<c0136df7>] do_munmap+0x193/0x1ac
	 [<c0102829>] sysenter_past_esp+0x56/0x79
	 [<c02d007b>] fn_hash_delete+0x4f/0x1c7

On Tue, Nov 21, 2006 at 09:36:46PM +0100, Pavol Gono wrote:
I am going to add two more:
for i in 1 2 3 4 5 ; do modprobe pktgen ; rmmod pktgen ; done
Looks like it creates /proc/net/pktgen/kpktgen_%i but forgets to remove
them.

Re: pktgen

From: David Miller <davem@davemloft.net>
Date: 2006-11-28 23:33:18

From: Alexey Dobriyan <redacted>
Date: Wed, 22 Nov 2006 00:22:51 +0300
[CCing netdev, bug in pktgen]

	[build modular pktgen]
	while true; do modprobe pktgen && rmmod pktgen; done

	BUG: warning at fs/proc/generic.c:732/remove_proc_entry()
	 [<c016a7ad>] remove_proc_entry+0x161/0x1ca
	 [<e19969ab>] pg_cleanup+0xd5/0xdc [pktgen]
         [<c011fa3e>] autoremove_wake_function+0x0/0x35
	 [<c01280bd>] sys_delete_module+0x162/0x189
	 [<c0136500>] remove_vma+0x31/0x36
	 [<c0136df7>] do_munmap+0x193/0x1ac
	 [<c0102829>] sysenter_past_esp+0x56/0x79
	 [<c02d007b>] fn_hash_delete+0x4f/0x1c7

On Tue, Nov 21, 2006 at 09:36:46PM +0100, Pavol Gono wrote:
quoted
I am going to add two more:
for i in 1 2 3 4 5 ; do modprobe pktgen ; rmmod pktgen ; done
Looks like it creates /proc/net/pktgen/kpktgen_%i but forgets to remove
them.
It's pretty careful to delete all of the entries under
/proc/net/pktgen/.

When the module is brought down, it walks the list of threads
and brings them down by setting T_TERMINATE in t->control.
This makes the thread break out of it's loop and run:

	pktgen_stop(t);
	pktgen_rem_all_ifs(t);
	pktgen_rem_thread(t);

pktgen_rem_all_ifs() will delete all device entries in
/proc/net/pktgen/

Next, pktgen_rem_thread() will kill off the entry for
/proc/net/pktgen/kpkgen_%i

Finally, the top-level module remove code will delete the
control file right before it tries to kill off the directory
via:

	/* Clean up proc file system */
	remove_proc_entry(PGCTRL, pg_proc_dir);
	proc_net_remove(PG_PROC_DIR);

So I don't see any bugs that could cause this.

One possibility is that the thread's t->name is being
corrupted somehow, that would make the remove_proc_entry()
fail and cause the behavior you see.  But I can't see anything
obvious that might do that either.

Re: pktgen

From: Alexey Dobriyan <hidden>
Date: 2006-11-29 20:04:41

On Tue, Nov 28, 2006 at 03:33:25PM -0800, David Miller wrote:
From: Alexey Dobriyan <redacted>
Date: Wed, 22 Nov 2006 00:22:51 +0300
quoted
[CCing netdev, bug in pktgen]

	[build modular pktgen]
	while true; do modprobe pktgen && rmmod pktgen; done

	BUG: warning at fs/proc/generic.c:732/remove_proc_entry()
	 [<c016a7ad>] remove_proc_entry+0x161/0x1ca
	 [<e19969ab>] pg_cleanup+0xd5/0xdc [pktgen]
         [<c011fa3e>] autoremove_wake_function+0x0/0x35
	 [<c01280bd>] sys_delete_module+0x162/0x189
	 [<c0136500>] remove_vma+0x31/0x36
	 [<c0136df7>] do_munmap+0x193/0x1ac
	 [<c0102829>] sysenter_past_esp+0x56/0x79
	 [<c02d007b>] fn_hash_delete+0x4f/0x1c7

On Tue, Nov 21, 2006 at 09:36:46PM +0100, Pavol Gono wrote:
quoted
I am going to add two more:
for i in 1 2 3 4 5 ; do modprobe pktgen ; rmmod pktgen ; done
Looks like it creates /proc/net/pktgen/kpktgen_%i but forgets to remove
them.
It's pretty careful to delete all of the entries under
/proc/net/pktgen/.

When the module is brought down, it walks the list of threads
and brings them down by setting T_TERMINATE in t->control.
Looks like worker thread strategically clears it if scheduled at wrong
moment.
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -3292,7 +3292,6 @@ static void pktgen_thread_worker(struct
 
 	init_waitqueue_head(&t->queue);
 
-	t->control &= ~(T_TERMINATE);
 	t->control &= ~(T_RUN);
 	t->control &= ~(T_STOP);
 	t->control &= ~(T_REMDEVALL);
This makes the thread break out of it's loop and run:

	pktgen_stop(t);
	pktgen_rem_all_ifs(t);
	pktgen_rem_thread(t);
Kernel seeems to survive, but when I hit Ctrl+C after half
a minute backtrace is back being the very last dmesg lines.

Re: pktgen

From: David Miller <davem@davemloft.net>
Date: 2006-11-30 01:49:45

From: Alexey Dobriyan <redacted>
Date: Wed, 29 Nov 2006 23:04:37 +0300
quoted hunk
Looks like worker thread strategically clears it if scheduled at wrong
moment.
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -3292,7 +3292,6 @@ static void pktgen_thread_worker(struct
 
 	init_waitqueue_head(&t->queue);
 
-	t->control &= ~(T_TERMINATE);
 	t->control &= ~(T_RUN);
 	t->control &= ~(T_STOP);
 	t->control &= ~(T_REMDEVALL);
Good catch Alexey.  Did you rerun the load/unload test with
this fix applied?  If it fixes things, I'll merge it.

Thanks!

Re: pktgen

From: Alexey Dobriyan <hidden>
Date: 2006-11-30 07:30:48

On 11/30/06, David Miller [off-list ref] wrote:
From: Alexey Dobriyan <redacted>
Date: Wed, 29 Nov 2006 23:04:37 +0300
quoted
Looks like worker thread strategically clears it if scheduled at wrong
moment.
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -3292,7 +3292,6 @@ static void pktgen_thread_worker(struct

 	init_waitqueue_head(&t->queue);

-	t->control &= ~(T_TERMINATE);
 	t->control &= ~(T_RUN);
 	t->control &= ~(T_STOP);
 	t->control &= ~(T_REMDEVALL);
Good catch Alexey.  Did you rerun the load/unload test with
this fix applied?  If it fixes things, I'll merge it.
Well, yes, it fixes things, except Ctrl+C getting you out of
modprobe/rmmod loop will spit
backtrace again. And other flags: T_RUN, T_STOP. Clearance is not
needed due to kZalloc and
create bugs as demostrated.

Give me some time.

Re: pktgen

From: Robert Olsson <hidden>
Date: 2006-11-30 08:45:52


Hello!

Seems you found a race when rmmod is done before it's fully started

Try:
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 733d86d..ac0b4b1 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -160,7 +160,7 @@
 #include <asm/div64.h>		/* do_div */
 #include <asm/timex.h>
 
-#define VERSION  "pktgen v2.68: Packet Generator for packet performance testing.\n"
+#define VERSION  "pktgen v2.69: Packet Generator for packet performance testing.\n"
 
 /* #define PG_DEBUG(a) a */
 #define PG_DEBUG(a)
@@ -3673,6 +3673,8 @@ static void __exit pg_cleanup(void)
 	struct list_head *q, *n;
 	wait_queue_head_t queue;
 	init_waitqueue_head(&queue);
+	
+	schedule_timeout_interruptible(msecs_to_jiffies(125));
 
 	/* Stop all interfaces & threads */
 

for i in 1 2 3 4 5 ; do modprobe pktgen ; rmmod pktgen ; done

In dmesg
pktgen v2.69: Packet Generator for packet performance testing.
pktgen v2.69: Packet Generator for packet performance testing.
pktgen v2.69: Packet Generator for packet performance testing.
pktgen v2.69: Packet Generator for packet performance testing.
pktgen v2.69: Packet Generator for packet performance testing.

Cheers.
					--ro



Alexey Dobriyan writes:
 > On 11/30/06, David Miller [off-list ref] wrote:
 > > From: Alexey Dobriyan [off-list ref]
 > > Date: Wed, 29 Nov 2006 23:04:37 +0300
 > >
 > > > Looks like worker thread strategically clears it if scheduled at wrong
 > > > moment.
 > > >
 > > > --- a/net/core/pktgen.c
 > > > +++ b/net/core/pktgen.c
 > > > @@ -3292,7 +3292,6 @@ static void pktgen_thread_worker(struct
 > > >
 > > >  	init_waitqueue_head(&t->queue);
 > > >
 > > > -	t->control &= ~(T_TERMINATE);
 > > >  	t->control &= ~(T_RUN);
 > > >  	t->control &= ~(T_STOP);
 > > >  	t->control &= ~(T_REMDEVALL);
 > >
 > > Good catch Alexey.  Did you rerun the load/unload test with
 > > this fix applied?  If it fixes things, I'll merge it.
 > 
 > Well, yes, it fixes things, except Ctrl+C getting you out of
 > modprobe/rmmod loop will spit
 > backtrace again. And other flags: T_RUN, T_STOP. Clearance is not
 > needed due to kZalloc and
 > create bugs as demostrated.
 > 
 > Give me some time.
 > -
 > To unsubscribe from this list: send the line "unsubscribe netdev" in
 > the body of a message to majordomo@vger.kernel.org
 > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: pktgen

From: Ben Greear <hidden>
Date: 2006-11-30 17:32:21

Robert Olsson wrote:
quoted hunk
Hello!

Seems you found a race when rmmod is done before it's fully started

Try:
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 733d86d..ac0b4b1 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -160,7 +160,7 @@
 #include <asm/div64.h>		/* do_div */
 #include <asm/timex.h>
 
-#define VERSION  "pktgen v2.68: Packet Generator for packet performance testing.\n"
+#define VERSION  "pktgen v2.69: Packet Generator for packet performance testing.\n"
 
 /* #define PG_DEBUG(a) a */
 #define PG_DEBUG(a)
@@ -3673,6 +3673,8 @@ static void __exit pg_cleanup(void)
 	struct list_head *q, *n;
 	wait_queue_head_t queue;
 	init_waitqueue_head(&queue);
+	
+	schedule_timeout_interruptible(msecs_to_jiffies(125));
 
 	/* Stop all interfaces & threads */
 
  
That strikes me as a hack..surely there is a better method than just adding
a sleep??

Ben

-- 
Ben Greear [off-list ref] 
Candela Technologies Inc  http://www.candelatech.com

Re: pktgen

From: David Miller <davem@davemloft.net>
Date: 2006-12-01 04:14:20

From: Ben Greear <redacted>
Date: Thu, 30 Nov 2006 09:33:43 -0800
Robert Olsson wrote:
quoted
@@ -3673,6 +3673,8 @@ static void __exit pg_cleanup(void)
 	struct list_head *q, *n;
 	wait_queue_head_t queue;
 	init_waitqueue_head(&queue);
+	
+	schedule_timeout_interruptible(msecs_to_jiffies(125));
 
 	/* Stop all interfaces & threads */
 
That strikes me as a hack..surely there is a better method than just adding
a sleep??
Agreed.

Robert, please fix this by using a completion so that we can
wait for the threads to start up, something like this:

...
#include <linux/completion.h>
...

struct pktgen_thread_info {
       struct pktgen_thread *t;
       struct completion *c;
};

static void pktgen_thread_worker(struct pktgen_thread_info *info)
{
	struct pktgen_thread *t = info->t;
 ...
	__set_current_state(TASK_INTERRUPTIBLE);
	mb();

	complete(info->c);
 ...
}

static int __init pktgen_create_thread(const char *name, int cpu)
{
 ...
	struct pktgen_thread_info info;
	struct complation started;
 ...
        init_completion(&started);

	info.t = t;
	info.c = &started;

	err = kernel_thread((void *)pktgen_thread_worker, (void *)t,
			  CLONE_FS | CLONE_FILES | CLONE_SIGHAND);
	if (err < 0) {
		printk("pktgen: kernel_thread() failed for cpu %d\n", t->cpu);
		remove_proc_entry(t->name, pg_proc_dir);
		list_del(&t->th_list);
		kfree(t);
		return err;
	}

	wait_for_completion(&started);
	return 0;
}

We can horse around with fixing the initial t->control bit flipping
in pktgen_thread_worker() in a future changeset if we want.

Re: pktgen

From: Robert Olsson <hidden>
Date: 2006-12-01 08:14:16

David Miller writes:

 > Agreed.
 > 
 > Robert, please fix this by using a completion so that we can
 > wait for the threads to start up, something like this:

Included. It passes my test but Alexey and others test.
Cheers.
				--ro
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 733d86d..a630a73 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -147,6 +147,7 @@
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 #include <linux/wait.h>
+#include <linux/completion.h>
 #include <linux/etherdevice.h>
 #include <net/checksum.h>
 #include <net/ipv6.h>
@@ -160,7 +161,7 @@
 #include <asm/div64.h>		/* do_div */
 #include <asm/timex.h>
 
-#define VERSION  "pktgen v2.68: Packet Generator for packet performance testing.\n"
+#define VERSION  "pktgen v2.69: Packet Generator for packet performance testing.\n"
 
 /* #define PG_DEBUG(a) a */
 #define PG_DEBUG(a)
@@ -206,6 +207,11 @@ static struct proc_dir_entry *pg_proc_di
 #define VLAN_TAG_SIZE(x) ((x)->vlan_id == 0xffff ? 0 : 4)
 #define SVLAN_TAG_SIZE(x) ((x)->svlan_id == 0xffff ? 0 : 4)
 
+struct pktgen_thread_info {
+       struct pktgen_thread *t;
+       struct completion *c;
+};
+
 struct flow_state {
 	__u32 cur_daddr;
 	int count;
@@ -3264,10 +3270,11 @@ out:;
  * Main loop of the thread goes here
  */
 
-static void pktgen_thread_worker(struct pktgen_thread *t)
+static void pktgen_thread_worker(struct pktgen_thread_info *info)
 {
 	DEFINE_WAIT(wait);
 	struct pktgen_dev *pkt_dev = NULL;
+	struct pktgen_thread *t = info->t;
 	int cpu = t->cpu;
 	sigset_t tmpsig;
 	u32 max_before_softirq;
@@ -3307,6 +3314,8 @@ static void pktgen_thread_worker(struct 
 	__set_current_state(TASK_INTERRUPTIBLE);
 	mb();
 
+        complete(info->c);
+
 	while (1) {
 
 		__set_current_state(TASK_RUNNING);
@@ -3518,6 +3527,8 @@ static struct pktgen_thread *__init pktg
 static int __init pktgen_create_thread(const char *name, int cpu)
 {
 	int err;
+	struct pktgen_thread_info info;
+        struct completion started;
 	struct pktgen_thread *t = NULL;
 	struct proc_dir_entry *pe;
 
@@ -3558,7 +3569,11 @@ static int __init pktgen_create_thread(c
 
 	t->removed = 0;
 
-	err = kernel_thread((void *)pktgen_thread_worker, (void *)t,
+	init_completion(&started);
+        info.t = t;
+        info.c = &started;
+
+	err = kernel_thread((void *)pktgen_thread_worker, (void *)&info,
 			  CLONE_FS | CLONE_FILES | CLONE_SIGHAND);
 	if (err < 0) {
 		printk("pktgen: kernel_thread() failed for cpu %d\n", t->cpu);
@@ -3568,6 +3583,7 @@ static int __init pktgen_create_thread(c
 		return err;
 	}
 
+	wait_for_completion(&started);
 	return 0;
 }
 

Re: pktgen

From: Christoph Hellwig <hch@infradead.org>
Date: 2006-12-01 08:22:34

On Thu, Nov 30, 2006 at 08:14:23PM -0800, David Miller wrote:
Agreed.

Robert, please fix this by using a completion so that we can
wait for the threads to start up, something like this:
No, that's wrong aswell :)  Please use the kthread_ API that takes
care of all this.  kernel_thread is going away mid-term, so you'd
have to do this work anyway.

Re: pktgen

From: Alexey Dobriyan <hidden>
Date: 2006-12-01 09:51:54

On 12/1/06, Robert Olsson [off-list ref] wrote:
David Miller writes:
 > Agreed.
 >
 > Robert, please fix this by using a completion so that we can
 > wait for the threads to start up, something like this:

Included. It passes my test but Alexey and others test.
Confused now. Is my "t->control &= ~(T_TERMINATE);" fix deprecated by
completions?
quoted hunk
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -147,6 +147,7 @@
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 #include <linux/wait.h>
+#include <linux/completion.h>
 #include <linux/etherdevice.h>
 #include <net/checksum.h>
 #include <net/ipv6.h>
@@ -160,7 +161,7 @@
 #include <asm/div64.h>		/* do_div */
 #include <asm/timex.h>

-#define VERSION  "pktgen v2.68: Packet Generator for packet performance
testing.\n"
+#define VERSION  "pktgen v2.69: Packet Generator for packet performance
testing.\n"

 /* #define PG_DEBUG(a) a */
 #define PG_DEBUG(a)
@@ -206,6 +207,11 @@ static struct proc_dir_entry *pg_proc_di
 #define VLAN_TAG_SIZE(x) ((x)->vlan_id == 0xffff ? 0 : 4)
 #define SVLAN_TAG_SIZE(x) ((x)->svlan_id == 0xffff ? 0 : 4)

+struct pktgen_thread_info {
+       struct pktgen_thread *t;
+       struct completion *c;
+};
+
 struct flow_state {
 	__u32 cur_daddr;
 	int count;
@@ -3264,10 +3270,11 @@ out:;
  * Main loop of the thread goes here
  */

-static void pktgen_thread_worker(struct pktgen_thread *t)
+static void pktgen_thread_worker(struct pktgen_thread_info *info)
 {
 	DEFINE_WAIT(wait);
 	struct pktgen_dev *pkt_dev = NULL;
+	struct pktgen_thread *t = info->t;
 	int cpu = t->cpu;
 	sigset_t tmpsig;
 	u32 max_before_softirq;
@@ -3307,6 +3314,8 @@ static void pktgen_thread_worker(struct
 	__set_current_state(TASK_INTERRUPTIBLE);
 	mb();

+        complete(info->c);
+
 	while (1) {

 		__set_current_state(TASK_RUNNING);
@@ -3518,6 +3527,8 @@ static struct pktgen_thread *__init pktg
 static int __init pktgen_create_thread(const char *name, int cpu)
 {
 	int err;
+	struct pktgen_thread_info info;
+        struct completion started;
 	struct pktgen_thread *t = NULL;
 	struct proc_dir_entry *pe;
@@ -3558,7 +3569,11 @@ static int __init pktgen_create_thread(c

 	t->removed = 0;

-	err = kernel_thread((void *)pktgen_thread_worker, (void *)t,
+	init_completion(&started);
+        info.t = t;
+        info.c = &started;
+
+	err = kernel_thread((void *)pktgen_thread_worker, (void *)&info,
 			  CLONE_FS | CLONE_FILES | CLONE_SIGHAND);
 	if (err < 0) {
 		printk("pktgen: kernel_thread() failed for cpu %d\n", t->cpu);
@@ -3568,6 +3583,7 @@ static int __init pktgen_create_thread(c
 		return err;
 	}

+	wait_for_completion(&started);
 	return 0;
 }

Re: pktgen

From: Robert Olsson <hidden>
Date: 2006-12-01 17:18:31

Alexey Dobriyan writes:

 > 
 > Confused now. Is my "t->control &= ~(T_TERMINATE);" fix deprecated by
 > completions?
 
 It's not needed with completion patch as this does the job a bit more
 mainstream. The T_TERMINATE seems to work well I've tested on machine
 with CPU:s. Only once I noticed that only 2 of 4 threads got started 
 but it could be something else...

 And the race is hardly seen with any real use of pktgen.. .Let's hear
 what others... and completions was out-of-date.

 Cheers.
					--ro
 

Re: pktgen

From: David Miller <davem@davemloft.net>
Date: 2006-12-01 23:17:31

From: Christoph Hellwig <hch@infradead.org>
Date: Fri, 1 Dec 2006 08:22:25 +0000
On Thu, Nov 30, 2006 at 08:14:23PM -0800, David Miller wrote:
quoted
Agreed.

Robert, please fix this by using a completion so that we can
wait for the threads to start up, something like this:
No, that's wrong aswell :)  Please use the kthread_ API that takes
care of all this.  kernel_thread is going away mid-term, so you'd
have to do this work anyway.
I was going to suggest this Christophe, but I wanted a change small
and easy enough to verify in order to merge the fix into the -stable
branch.  Converting the thing over to kthread would make the change
more risky in such a context.

Re: pktgen

From: David Miller <davem@davemloft.net>
Date: 2006-12-01 23:25:45

From: "Alexey Dobriyan" <redacted>
Date: Fri, 1 Dec 2006 12:51:53 +0300
On 12/1/06, Robert Olsson [off-list ref] wrote:
quoted
David Miller writes:
 > Agreed.
 >
 > Robert, please fix this by using a completion so that we can
 > wait for the threads to start up, something like this:

Included. It passes my test but Alexey and others test.
Confused now. Is my "t->control &= ~(T_TERMINATE);" fix deprecated by
completions?
The completions solve the bug completely.

But later on we should integate a change that eliminates
these spurious t->control bit clears at the start of the
pktgen thread execution, it just isn't needed to fix the
bug so we can make it later.

Re: pktgen

From: David Miller <davem@davemloft.net>
Date: 2007-01-02 04:53:32

From: Robert Olsson <redacted>
Date: Fri, 1 Dec 2006 09:14:01 +0100
David Miller writes:

 > Agreed.
 > 
 > Robert, please fix this by using a completion so that we can
 > wait for the threads to start up, something like this:

Included. It passes my test but Alexey and others test.
Ok, I'm going to put Robert's version of the fix into 2.6.19
and previous -stable branches.

But for 2.6.20 I'd like to do the following, based upon
Christoph's suggestion to convert to kthread.

Can folks please give this a spin?  I've tested that the
module loads and unloads properly, the threads startup
and shutdown, but that's it.

Thanks!

commit b3010a665cc33596fbf4be3fc6c3c5c80aeefb65
Author: David S. Miller [off-list ref]
Date:   Mon Jan 1 20:51:53 2007 -0800

    [PKTGEN]: Convert to kthread API.
    
    Based upon a suggestion from Christoph Hellwig.
    
    This fixes various races in module load/unload handling
    too.
    
    Signed-off-by: David S. Miller [off-list ref]
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 1897a3a..04d4b93 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -148,6 +148,7 @@
 #include <linux/seq_file.h>
 #include <linux/wait.h>
 #include <linux/etherdevice.h>
+#include <linux/kthread.h>
 #include <net/checksum.h>
 #include <net/ipv6.h>
 #include <net/addrconf.h>
@@ -360,8 +361,7 @@ struct pktgen_thread {
 	spinlock_t if_lock;
 	struct list_head if_list;	/* All device here */
 	struct list_head th_list;
-	int removed;
-	char name[32];
+	struct task_struct *tsk;
 	char result[512];
 	u32 max_before_softirq;	/* We'll call do_softirq to prevent starvation. */
 
@@ -1689,7 +1689,7 @@ static int pktgen_thread_show(struct seq_file *seq, void *v)
 	BUG_ON(!t);
 
 	seq_printf(seq, "Name: %s  max_before_softirq: %d\n",
-		   t->name, t->max_before_softirq);
+		   t->tsk->comm, t->max_before_softirq);
 
 	seq_printf(seq, "Running: ");
 
@@ -3112,7 +3112,7 @@ static void pktgen_rem_thread(struct pktgen_thread *t)
 {
 	/* Remove from the thread list */
 
-	remove_proc_entry(t->name, pg_proc_dir);
+	remove_proc_entry(t->tsk->comm, pg_proc_dir);
 
 	mutex_lock(&pktgen_thread_lock);
 
@@ -3260,58 +3260,40 @@ out:;
  * Main loop of the thread goes here
  */
 
-static void pktgen_thread_worker(struct pktgen_thread *t)
+static int pktgen_thread_worker(void *arg)
 {
 	DEFINE_WAIT(wait);
+	struct pktgen_thread *t = arg;
 	struct pktgen_dev *pkt_dev = NULL;
 	int cpu = t->cpu;
-	sigset_t tmpsig;
 	u32 max_before_softirq;
 	u32 tx_since_softirq = 0;
 
-	daemonize("pktgen/%d", cpu);
-
-	/* Block all signals except SIGKILL, SIGSTOP and SIGTERM */
-
-	spin_lock_irq(&current->sighand->siglock);
-	tmpsig = current->blocked;
-	siginitsetinv(&current->blocked,
-		      sigmask(SIGKILL) | sigmask(SIGSTOP) | sigmask(SIGTERM));
-
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
-
-	/* Migrate to the right CPU */
-	set_cpus_allowed(current, cpumask_of_cpu(cpu));
-	if (smp_processor_id() != cpu)
-		BUG();
+	BUG_ON(smp_processor_id() != cpu);
 
 	init_waitqueue_head(&t->queue);
 
-	t->control &= ~(T_TERMINATE);
-	t->control &= ~(T_RUN);
-	t->control &= ~(T_STOP);
-	t->control &= ~(T_REMDEVALL);
-	t->control &= ~(T_REMDEV);
-
 	t->pid = current->pid;
 
 	PG_DEBUG(printk("pktgen: starting pktgen/%d:  pid=%d\n", cpu, current->pid));
 
 	max_before_softirq = t->max_before_softirq;
 
-	__set_current_state(TASK_INTERRUPTIBLE);
-	mb();
+	set_current_state(TASK_INTERRUPTIBLE);
 
-	while (1) {
-
-		__set_current_state(TASK_RUNNING);
+	while (!kthread_should_stop()) {
+		pkt_dev = next_to_run(t);
 
-		/*
-		 * Get next dev to xmit -- if any.
-		 */
+		if (!pkt_dev &&
+		    (t->control & (T_STOP | T_RUN | T_REMDEVALL | T_REMDEV))
+		    == 0) {
+			prepare_to_wait(&(t->queue), &wait,
+					TASK_INTERRUPTIBLE);
+			schedule_timeout(HZ / 10);
+			finish_wait(&(t->queue), &wait);
+		}
 
-		pkt_dev = next_to_run(t);
+		__set_current_state(TASK_RUNNING);
 
 		if (pkt_dev) {
 
@@ -3329,21 +3311,8 @@ static void pktgen_thread_worker(struct pktgen_thread *t)
 					do_softirq();
 				tx_since_softirq = 0;
 			}
-		} else {
-			prepare_to_wait(&(t->queue), &wait, TASK_INTERRUPTIBLE);
-			schedule_timeout(HZ / 10);
-			finish_wait(&(t->queue), &wait);
 		}
 
-		/*
-		 * Back from sleep, either due to the timeout or signal.
-		 * We check if we have any "posted" work for us.
-		 */
-
-		if (t->control & T_TERMINATE || signal_pending(current))
-			/* we received a request to terminate ourself */
-			break;
-
 		if (t->control & T_STOP) {
 			pktgen_stop(t);
 			t->control &= ~(T_STOP);
@@ -3364,20 +3333,19 @@ static void pktgen_thread_worker(struct pktgen_thread *t)
 			t->control &= ~(T_REMDEV);
 		}
 
-		if (need_resched())
-			schedule();
+		set_current_state(TASK_INTERRUPTIBLE);
 	}
 
-	PG_DEBUG(printk("pktgen: %s stopping all device\n", t->name));
+	PG_DEBUG(printk("pktgen: %s stopping all device\n", t->tsk->comm));
 	pktgen_stop(t);
 
-	PG_DEBUG(printk("pktgen: %s removing all device\n", t->name));
+	PG_DEBUG(printk("pktgen: %s removing all device\n", t->tsk->comm));
 	pktgen_rem_all_ifs(t);
 
-	PG_DEBUG(printk("pktgen: %s removing thread.\n", t->name));
+	PG_DEBUG(printk("pktgen: %s removing thread.\n", t->tsk->comm));
 	pktgen_rem_thread(t);
 
-	t->removed = 1;
+	return 0;
 }
 
 static struct pktgen_dev *pktgen_find_dev(struct pktgen_thread *t,
@@ -3495,37 +3463,11 @@ static int pktgen_add_device(struct pktgen_thread *t, const char *ifname)
 	return add_dev_to_thread(t, pkt_dev);
 }
 
-static struct pktgen_thread *__init pktgen_find_thread(const char *name)
+static int __init pktgen_create_thread(int cpu)
 {
 	struct pktgen_thread *t;
-
-	mutex_lock(&pktgen_thread_lock);
-
-	list_for_each_entry(t, &pktgen_threads, th_list)
-		if (strcmp(t->name, name) == 0) {
-			mutex_unlock(&pktgen_thread_lock);
-			return t;
-		}
-
-	mutex_unlock(&pktgen_thread_lock);
-	return NULL;
-}
-
-static int __init pktgen_create_thread(const char *name, int cpu)
-{
-	int err;
-	struct pktgen_thread *t = NULL;
 	struct proc_dir_entry *pe;
-
-	if (strlen(name) > 31) {
-		printk("pktgen: ERROR:  Thread name cannot be more than 31 characters.\n");
-		return -EINVAL;
-	}
-
-	if (pktgen_find_thread(name)) {
-		printk("pktgen: ERROR: thread: %s already exists\n", name);
-		return -EINVAL;
-	}
+	struct task_struct *p;
 
 	t = kzalloc(sizeof(struct pktgen_thread), GFP_KERNEL);
 	if (!t) {
@@ -3533,14 +3475,29 @@ static int __init pktgen_create_thread(const char *name, int cpu)
 		return -ENOMEM;
 	}
 
-	strcpy(t->name, name);
 	spin_lock_init(&t->if_lock);
 	t->cpu = cpu;
 
-	pe = create_proc_entry(t->name, 0600, pg_proc_dir);
+	INIT_LIST_HEAD(&t->if_list);
+
+	list_add_tail(&t->th_list, &pktgen_threads);
+
+	p = kthread_create(pktgen_thread_worker, t, "kpktgend_%d", cpu);
+	if (IS_ERR(p)) {
+		printk("pktgen: kernel_thread() failed for cpu %d\n", t->cpu);
+		list_del(&t->th_list);
+		kfree(t);
+		return PTR_ERR(p);
+	}
+	kthread_bind(p, cpu);
+	t->tsk = p;
+
+	pe = create_proc_entry(t->tsk->comm, 0600, pg_proc_dir);
 	if (!pe) {
 		printk("pktgen: cannot create %s/%s procfs entry.\n",
-		       PG_PROC_DIR, t->name);
+		       PG_PROC_DIR, t->tsk->comm);
+		kthread_stop(p);
+		list_del(&t->th_list);
 		kfree(t);
 		return -EINVAL;
 	}
@@ -3548,21 +3505,7 @@ static int __init pktgen_create_thread(const char *name, int cpu)
 	pe->proc_fops = &pktgen_thread_fops;
 	pe->data = t;
 
-	INIT_LIST_HEAD(&t->if_list);
-
-	list_add_tail(&t->th_list, &pktgen_threads);
-
-	t->removed = 0;
-
-	err = kernel_thread((void *)pktgen_thread_worker, (void *)t,
-			  CLONE_FS | CLONE_FILES | CLONE_SIGHAND);
-	if (err < 0) {
-		printk("pktgen: kernel_thread() failed for cpu %d\n", t->cpu);
-		remove_proc_entry(t->name, pg_proc_dir);
-		list_del(&t->th_list);
-		kfree(t);
-		return err;
-	}
+	wake_up_process(p);
 
 	return 0;
 }
@@ -3643,10 +3586,8 @@ static int __init pg_init(void)
 
 	for_each_online_cpu(cpu) {
 		int err;
-		char buf[30];
 
-		sprintf(buf, "kpktgend_%i", cpu);
-		err = pktgen_create_thread(buf, cpu);
+		err = pktgen_create_thread(cpu);
 		if (err)
 			printk("pktgen: WARNING: Cannot create thread for cpu %d (%d)\n",
 					cpu, err);
@@ -3674,9 +3615,8 @@ static void __exit pg_cleanup(void)
 
 	list_for_each_safe(q, n, &pktgen_threads) {
 		t = list_entry(q, struct pktgen_thread, th_list);
-		t->control |= (T_TERMINATE);
-
-		wait_event_interruptible_timeout(queue, (t->removed == 1), HZ);
+		kthread_stop(t->tsk);
+		kfree(t);
 	}
 
 	/* Un-register us from receiving netdevice events */
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help