I'm writing a kernel module which creates a substantial amount of
kernel threads. After dropping some real stuff, the module is:
#include <linux/kernel.h>
#include <linux/sched.h>
#include <linux/kthread.h>
#include <linux/module.h>
#include <linux/slab.h>
MODULE_LICENSE("GPL");
static int nrthreads = 128;
module_param(nrthreads, int, 0644);
static int loopcount = 1024;
module_param(loopcount, int, 0644);
static int usehrtime = 0;
module_param(usehrtime, int, 0644);
static int slack = 50000;
module_param(slack, int, 0644);
static int msecs = 1;
module_param(msecs, int, 0644);
static DECLARE_COMPLETION(done);
static struct task_struct **threads;
static atomic_t nrunning;
static int test(void *unused)
{
int i;
ktime_t expires = ktime_set(0, msecs * NSEC_PER_MSEC);
for (i = 0; !kthread_should_stop() && i < loopcount; i++) {
if (usehrtime) {
set_current_state(TASK_UNINTERRUPTIBLE);
schedule_hrtimeout_range(&expires, slack, HRTIMER_MODE_REL);
}
else
schedule_timeout_uninterruptible(msecs_to_jiffies(msecs));
}
if (atomic_dec_and_test(&nrunning)) {
printk("last thread done\n");
complete(&done);
}
return 0;
}
static int __init testmod_init(void)
{
int i;
printk("test begin\n");
atomic_set(&nrunning, nrthreads);
threads = kmalloc(nrthreads * sizeof(struct task_struct *), GFP_KERNEL);
if (!threads)
return -ENOMEM;
for (i = 0; i < nrthreads; i++) {
threads[i] = kthread_run(test, NULL, "test/%d", i);
if (IS_ERR(threads[i])) {
int j, err = PTR_ERR(threads[i]);
for (j = 0; j < i; j++)
kthread_stop(threads[j]);
kfree(threads);
return err;
}
}
return 0;
}
static void __exit testmod_exit(void)
{
wait_for_completion(&done);
kfree(threads);
}
module_init(testmod_init);
module_exit(testmod_exit);
Usually it works as expected, at least from 8 to 128 threads.
But when I'm trying to run it a loop like:
while true; do insmod testmod.ko && rmmod testmod.ko; sleep 1; done
it's also possible to catch a very rare crash (ARM example):
Unable to handle kernel paging request at virtual address 7f1200c4
pgd = 80004000
[7f1200c4] *pgd=bdc28811, *pte=00000000, *ppte=00000000
Internal error: Oops: 80000007 [#1] PREEMPT SMP
Modules linked in: [last unloaded: testmod]
CPU: 1 Tainted: G O (3.3.0-rc2 #3)
PC is at 0x7f1200c4
LR is at __schedule+0x684/0x6e4
pc : [<7f1200c4>] lr : [<802c053c>] psr: 600f0113
sp : bf115f88 ip : 00000000 fp : 00000000
r10: 00000000 r9 : 00000000 r8 : 7f120394
r7 : 00000002 r6 : 00000400 r5 : 7f120204 r4 : bf114000
r3 : 00000000 r2 : bf115ec0 r1 : bf9220c0 r0 : 00000001
Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: 10c5387d Table: bfbcc04a DAC: 00000015
Process test/126 (pid: 10918, stack limit = 0xbf1142f8)
Stack: (0xbf115f88 to 0xbf116000)
5f80: 000f4240 00000000 bf213e4c 00000000 7f120000 00000013
5fa0: 00000000 80049228 00000000 00000000 00000000 00000000 00000000 00000000
5fc0: dead4ead ffffffff ffffffff 8048b2b8 00000000 00000000 8036a3f9 bf115fdc
5fe0: bf115fdc 271aee1c bf213e4c 8004919c 8000eabc 8000eabc bfefc811 bfefcc11
Code: bad PC value
Note the bad PC, and stack is just a nonsense. I suspect that the kernel calls
testmod_exit() and frees module memory _before_ all test/X threads are really
dead - i.e. the module memory is freed when at least one of the test/X threads
is somewhere in do_exit() or nearby. Is that possible? If yes, what's the better
way to ensure that all test/X threads are really gone at some point of
testmod_exit()?
An interesting thing is that I can't reproduce this fault with both printk()s
commented out. No ideas why.
Dmitry
Le mercredi 01 février 2012 à 20:09 +0400, Dmitry Antipov a écrit :
> I'm writing a kernel module which creates a substantial amount of
> kernel threads. After dropping some real stuff, the module is:
>
> #include <linux/kernel.h>
> #include <linux/sched.h>
> #include <linux/kthread.h>
> #include <linux/module.h>
> #include <linux/slab.h>
>
> MODULE_LICENSE("GPL");
>
> static int nrthreads = 128;
> module_param(nrthreads, int, 0644);
>
> static int loopcount = 1024;
> module_param(loopcount, int, 0644);
>
> static int usehrtime = 0;
> module_param(usehrtime, int, 0644);
>
> static int slack = 50000;
> module_param(slack, int, 0644);
>
> static int msecs = 1;
> module_param(msecs, int, 0644);
>
> static DECLARE_COMPLETION(done);
> static struct task_struct **threads;
> static atomic_t nrunning;
>
> static int test(void *unused)
> {
> int i;
> ktime_t expires = ktime_set(0, msecs * NSEC_PER_MSEC);
>
> for (i = 0; !kthread_should_stop() && i < loopcount; i++) {
> if (usehrtime) {
> set_current_state(TASK_UNINTERRUPTIBLE);
> schedule_hrtimeout_range(&expires, slack, HRTIMER_MODE_REL);
> }
> else
> schedule_timeout_uninterruptible(msecs_to_jiffies(msecs));
> }
>
> if (atomic_dec_and_test(&nrunning)) {
> printk("last thread done\n");
> complete(&done);
Race is here :
Here you allow testmod_exit() to continue, while _this_ thread has not
yet exited.
> }
And if this thread is preempted a litle bit, its code already was freed.
-> crash.
> return 0;
> }
>
> static int __init testmod_init(void)
> {
> int i;
>
> printk("test begin\n");
>
> atomic_set(&nrunning, nrthreads);
>
> threads = kmalloc(nrthreads * sizeof(struct task_struct *), GFP_KERNEL);
> if (!threads)
> return -ENOMEM;
>
> for (i = 0; i < nrthreads; i++) {
> threads[i] = kthread_run(test, NULL, "test/%d", i);
> if (IS_ERR(threads[i])) {
> int j, err = PTR_ERR(threads[i]);
>
> for (j = 0; j < i; j++)
> kthread_stop(threads[j]);
> kfree(threads);
> return err;
> }
> }
> return 0;
> }
>
> static void __exit testmod_exit(void)
> {
> wait_for_completion(&done);
> kfree(threads);
> }
>
> module_init(testmod_init);
> module_exit(testmod_exit);
>
> Usually it works as expected, at least from 8 to 128 threads.
> But when I'm trying to run it a loop like:
>
> while true; do insmod testmod.ko && rmmod testmod.ko; sleep 1; done
>
> it's also possible to catch a very rare crash (ARM example):
>
> Unable to handle kernel paging request at virtual address 7f1200c4
> pgd = 80004000
> [7f1200c4] *pgd=bdc28811, *pte=00000000, *ppte=00000000
> Internal error: Oops: 80000007 [#1] PREEMPT SMP
> Modules linked in: [last unloaded: testmod]
> CPU: 1 Tainted: G O (3.3.0-rc2 #3)
> PC is at 0x7f1200c4
> LR is at __schedule+0x684/0x6e4
> pc : [<7f1200c4>] lr : [<802c053c>] psr: 600f0113
> sp : bf115f88 ip : 00000000 fp : 00000000
> r10: 00000000 r9 : 00000000 r8 : 7f120394
> r7 : 00000002 r6 : 00000400 r5 : 7f120204 r4 : bf114000
> r3 : 00000000 r2 : bf115ec0 r1 : bf9220c0 r0 : 00000001
> Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
> Control: 10c5387d Table: bfbcc04a DAC: 00000015
> Process test/126 (pid: 10918, stack limit = 0xbf1142f8)
> Stack: (0xbf115f88 to 0xbf116000)
> 5f80: 000f4240 00000000 bf213e4c 00000000 7f120000 00000013
> 5fa0: 00000000 80049228 00000000 00000000 00000000 00000000 00000000 00000000
> 5fc0: dead4ead ffffffff ffffffff 8048b2b8 00000000 00000000 8036a3f9 bf115fdc
> 5fe0: bf115fdc 271aee1c bf213e4c 8004919c 8000eabc 8000eabc bfefc811 bfefcc11
> Code: bad PC value
>
> Note the bad PC, and stack is just a nonsense. I suspect that the kernel calls
> testmod_exit() and frees module memory _before_ all test/X threads are really
> dead - i.e. the module memory is freed when at least one of the test/X threads
> is somewhere in do_exit() or nearby. Is that possible? If yes, what's the better
> way to ensure that all test/X threads are really gone at some point of
> testmod_exit()?
>
> An interesting thing is that I can't reproduce this fault with both printk()s
> commented out. No ideas why.
>
> Dmitry
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On 02/01/2012 08:31 PM, Eric Dumazet wrote:
> Race is here :
>
> Here you allow testmod_exit() to continue, while _this_ thread has not
> yet exited.
>
> And if this thread is preempted a litle bit, its code already was freed.
> -> crash.
I realize this, but there was a second part of the question: what's the
better way to ensure that all test/X threads are really gone at some point of
testmod_exit()?
Thanks,
Dmitry
Le mercredi 01 février 2012 à 20:35 +0400, Dmitry Antipov a écrit :
> On 02/01/2012 08:31 PM, Eric Dumazet wrote:
>
> > Race is here :
> >
> > Here you allow testmod_exit() to continue, while _this_ thread has not
> > yet exited.
> >
> > And if this thread is preempted a litle bit, its code already was freed.
> > -> crash.
>
> I realize this, but there was a second part of the question: what's the
> better way to ensure that all test/X threads are really gone at some point of
> testmod_exit()?
>
You could use kthread_stop()
This way you can control all your kernel threads really exited before
module cleanup.
On 02/01/2012 09:16 PM, Eric Dumazet wrote:
>> I realize this, but there was a second part of the question: what's the
>> better way to ensure that all test/X threads are really gone at some point of
>> testmod_exit()?
>>
>
> You could use kthread_stop()
>
> This way you can control all your kernel threads really exited before
> module cleanup.
Hm... if I try something like:
static void __exit testmod_exit(void)
{
int i;
wait_for_completion(&done);
for (i = 0; i < nrthreads; i++)
kthread_stop(threads[i]);
kfree(threads);
}
typical result is:
Unable to handle kernel NULL pointer dereference at virtual address 00000000
pgd = bf25c000
[00000000] *pgd=bf266831, *pte=00000000, *ppte=00000000
Internal error: Oops: 17 [#1] PREEMPT SMP
Modules linked in: testmod(O-)
CPU: 1 Tainted: G O (3.3.0-rc2 #3)
PC is at exit_creds+0x14/0xb4
LR is at __put_task_struct+0x64/0xac
pc : [<8004ed3c>] lr : [<8002d254>] psr: 20070113
sp : bd871f10 ip : 00000000 fp : 00000000
r10: 00000000 r9 : bd870000 r8 : 8000db48
r7 : 00000081 r6 : 00000000 r5 : 00000000 r4 : bfa734c0
r3 : 00000000 r2 : bd871f20 r1 : 00000460 r0 : 00000000
Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
Control: 10c5387d Table: bf25c04a DAC: 00000015
Process rmmod (pid: 1330, stack limit = 0xbd8702f8)
Stack: (0xbd871f10 to 0xbd872000)
1f00: bd86f500 bfa734c0 bfa734c8 8002d254
1f20: bfa734c0 800492ac 00000001 7f0003b8 7f000228 7f000110 7f000268 00000000
1f40: 7e87f6d8 80073d48 7f000268 00000880 bd871f54 00000000 74736574 00646f6d
1f60: 00000000 76f01000 bf223280 8000db48 bd870000 00000000 bd871f8c 802c05d8
1f80: bf223300 bf2232d8 00000000 800adefc 00f01400 271aee1c 00000880 7e87f6d8
1fa0: 000120a8 8000d980 00000880 7e87f6d8 7e87f6d8 00000880 00009778 7e87f6cc
1fc0: 00000880 7e87f6d8 000120a8 00000081 7e87f88c 000120bc 76f06000 00000000
1fe0: 76e8e590 7e87f6d4 00008f5d 76e8e59c 800f0110 7e87f6d8 0daa7fca 8afaa89a
[<8004ed3c>] (exit_creds+0x14/0xb4) from [<8002d254>] (__put_task_struct+0x64/0xac)
[<8002d254>] (__put_task_struct+0x64/0xac) from [<800492ac>] (kthread_stop+0x74/0x7c)
[<800492ac>] (kthread_stop+0x74/0x7c) from [<7f000110>] (testmod_exit+0x2c/0x54 [testmod])
[<7f000110>] (testmod_exit+0x2c/0x54 [testmod]) from [<80073d48>] (sys_delete_module+0x1b8/0x26c)
[<80073d48>] (sys_delete_module+0x1b8/0x26c) from [<8000d980>] (ret_fast_syscall+0x0/0x30)
Code: e1a04000 e59032ec e3a05000 e59002e8 (e5933000)
I suppose that __put_task_struct() was called for the thread when is 'partially dead'
(because it's somewhere in do_exit() called by kthread() after returning from thread's
function), but not 'dead enough' to finalize it with free_task().
So, the question is still open.
Dmitry
Le jeudi 02 février 2012 à 10:13 +0400, Dmitry Antipov a écrit :
> On 02/01/2012 09:16 PM, Eric Dumazet wrote:
>
> >> I realize this, but there was a second part of the question: what's the
> >> better way to ensure that all test/X threads are really gone at some point of
> >> testmod_exit()?
> >>
> >
> > You could use kthread_stop()
> >
> > This way you can control all your kernel threads really exited before
> > module cleanup.
>
> Hm... if I try something like:
>
> static void __exit testmod_exit(void)
> {
> int i;
>
> wait_for_completion(&done);
> for (i = 0; i < nrthreads; i++)
> kthread_stop(threads[i]);
> kfree(threads);
> }
>
> typical result is:
> I suppose that __put_task_struct() was called for the thread when is 'partially dead'
> (because it's somewhere in do_exit() called by kthread() after returning from thread's
> function), but not 'dead enough' to finalize it with free_task().
>
> So, the question is still open.
>
Really you need to better understand how all this works.
Remove the wait_for_completion(), this brings nothing at all, as you
already discovered.
Then you need cooperation from worker threads : they must wait for
kthread_should_stop(), or else your kthread_stop(arg) pass an already
freed "arg" memory block.
Take the time to read kernel/kthread.c and function kthread_stop()
Le jeudi 02 février 2012 à 10:13 +0400, Dmitry Antipov a écrit :
> On 02/01/2012 09:16 PM, Eric Dumazet wrote:
>
> >> I realize this, but there was a second part of the question: what's the
> >> better way to ensure that all test/X threads are really gone at some point of
> >> testmod_exit()?
> >>
> >
> > You could use kthread_stop()
> >
> > This way you can control all your kernel threads really exited before
> > module cleanup.
>
> Hm... if I try something like:
>
> static void __exit testmod_exit(void)
> {
> int i;
>
> wait_for_completion(&done);
> for (i = 0; i < nrthreads; i++)
> kthread_stop(threads[i]);
> kfree(threads);
> }
Try following code :
#include <linux/kernel.h>
#include <linux/sched.h>
#include <linux/kthread.h>
#include <linux/module.h>
#include <linux/slab.h>
MODULE_LICENSE("GPL");
static int nrthreads = 128;
module_param(nrthreads, int, 0644);
static int loopcount = 1024;
module_param(loopcount, int, 0644);
static int usehrtime = 0;
module_param(usehrtime, int, 0644);
static int slack = 50000;
module_param(slack, int, 0644);
static int msecs = 1;
module_param(msecs, int, 0644);
static struct task_struct **threads;
static int test(void *unused)
{
int i;
ktime_t expires = ktime_set(0, msecs * NSEC_PER_MSEC);
for (i = 0; !kthread_should_stop() && i < loopcount; i++) {
if (usehrtime) {
set_current_state(TASK_UNINTERRUPTIBLE);
schedule_hrtimeout_range(&expires, slack, HRTIMER_MODE_REL);
}
else
schedule_timeout_uninterruptible(msecs_to_jiffies(msecs));
}
return 0;
}
static int __init testmod_init(void)
{
int i;
printk("test begin\n");
threads = kmalloc(nrthreads * sizeof(struct task_struct *), GFP_KERNEL);
if (!threads)
return -ENOMEM;
for (i = 0; i < nrthreads; i++) {
threads[i] = kthread_create(test, NULL, "test/%d", i);
if (IS_ERR(threads[i])) {
int j, err = PTR_ERR(threads[i]);
for (j = 0; j < i; j++)
kthread_stop(threads[j]);
kfree(threads);
return err;
}
}
for (i = 0; i < nrthreads; i++) {
get_task_struct(threads[i]);
wake_up_process(threads[i]);
}
return 0;
}
static void __exit testmod_exit(void)
{
int i;
for (i = 0; i < nrthreads; i++) {
kthread_stop(threads[i]);
put_task_struct(threads[i]);
}
kfree(threads);
}
module_init(testmod_init);
module_exit(testmod_exit);
On 02/02/2012 11:45 AM, Eric Dumazet wrote:
> Remove the wait_for_completion(), this brings nothing at all, as you
> already discovered.
I want to get the module 'locked' until all threads are completed.
> Then you need cooperation from worker threads : they must wait for
> kthread_should_stop(), or else your kthread_stop(arg) pass an already
> freed "arg" memory block.
I designed the cooperation in another way: after each successful call
of X = kthread_run(...), I do get_task_struct(X), and the module exit
code is something like:
...
wait_for_completion(&done);
for (i = 0; i < nrthreads; i++) {
kthread_stop(threads[i]);
put_task_struct(threads[i]);
}
kfree(threads);
...
The crash is go away, so I believe this is reasonable. Anyway, it would
be nice to have a debug option to detect such a suspicious errors.
Thanks,
Dmitry
On 02/02/2012 01:15 PM, Eric Dumazet wrote:
>
> Try following code :
>
I already did it myself (except redundant wake_up_process(), which
is performed by kthread_run() anyway).
Thanks,
Dmitry
Le jeudi 02 février 2012 à 13:22 +0400, Dmitry Antipov a écrit :
> On 02/02/2012 01:15 PM, Eric Dumazet wrote:
>
> >
> > Try following code :
> >
>
> I already did it myself (except redundant wake_up_process(), which
> is performed by kthread_run() anyway).
Then its racy, unless you also changed the way your worker threads exit.
By the time kthread_run() returns, child thread can already be gone.
This is why I use kthread_create() : to be able to get_task_struct() so
that task_struct cannot disappear, even if the child exits really fast :
int worker(void *arg)
{
return 0;
}
On Thu, 02 Feb 2012 10:58:18 +0100, Eric Dumazet <[email protected]> wrote:
> Le jeudi 02 février 2012 à 13:22 +0400, Dmitry Antipov a écrit :
> > On 02/02/2012 01:15 PM, Eric Dumazet wrote:
> >
> > >
> > > Try following code :
> > >
> >
> > I already did it myself (except redundant wake_up_process(), which
> > is performed by kthread_run() anyway).
>
> Then its racy, unless you also changed the way your worker threads exit.
See complete_and_exit().
Thanks,
Rusty.