Home » Mailing lists » Devel » [PATCH] Check for error returned by kthread_create on creating journal thread
| [PATCH] Check for error returned by kthread_create on creating journal thread [message #12033] |
Mon, 16 April 2007 07:40  |
xemul
Messages: 248
Registered: November 2005
|
Senior Member |
|
|
If the thread failed to create the subsequent wait_event
will hang forever.
This is likely to happen if kernel hits max_threads limit.
Will be critical for virtualization systems that limit the
number of tasks and kernel memory usage within the container.
--- ./fs/jbd/journal.c.jbdthreads 2007-04-16 11:17:36.000000000 +0400
+++ ./fs/jbd/journal.c 2007-04-16 11:30:09.000000000 +0400
@@ -211,10 +211,16 @@ end_loop:
return 0;
}
-static void journal_start_thread(journal_t *journal)
+static int journal_start_thread(journal_t *journal)
{
- kthread_run(kjournald, journal, "kjournald");
+ struct task_struct *t;
+
+ t = kthread_run(kjournald, journal, "kjournald");
+ if (IS_ERR(t))
+ return PTR_ERR(t);
+
wait_event(journal->j_wait_done_commit, journal->j_task != 0);
+ return 0;
}
static void journal_kill_thread(journal_t *journal)
@@ -840,8 +846,7 @@ static int journal_reset(journal_t *jour
/* Add the dynamic fields and write it to disk. */
journal_update_superblock(journal, 1);
- journal_start_thread(journal);
- return 0;
+ return journal_start_thread(journal);
}
/**
--- ./fs/jbd2/journal.c.jbdthreads 2007-04-16 11:17:36.000000000 +0400
+++ ./fs/jbd2/journal.c 2007-04-16 11:27:45.000000000 +0400
@@ -211,10 +211,16 @@ end_loop:
return 0;
}
-static void jbd2_journal_start_thread(journal_t *journal)
+static int jbd2_journal_start_thread(journal_t *journal)
{
- kthread_run(kjournald2, journal, "kjournald2");
+ struct task_struct *t;
+
+ t = kthread_run(kjournald2, journal, "kjournald2");
+ if (IS_ERR(t))
+ return PTR_ERR(t);
+
wait_event(journal->j_wait_done_commit, journal->j_task != 0);
+ return 0;
}
static void journal_kill_thread(journal_t *journal)
@@ -840,8 +846,7 @@ static int journal_reset(journal_t *jour
/* Add the dynamic fields and write it to disk. */
jbd2_journal_update_superblock(journal, 1);
- jbd2_journal_start_thread(journal);
- return 0;
+ return jbd2_journal_start_thread(journal);
}
/**
|
|
|
|
| Re: [PATCH] Check for error returned by kthread_create on creating journal thread [message #12039 is a reply to message #12033] |
Mon, 16 April 2007 10:21  |
Christoph Hellwig
Messages: 59
Registered: April 2006
|
Member |
|
|
On Mon, Apr 16, 2007 at 11:41:14AM +0400, Pavel Emelianov wrote:
> If the thread failed to create the subsequent wait_event
> will hang forever.
>
> This is likely to happen if kernel hits max_threads limit.
>
> Will be critical for virtualization systems that limit the
> number of tasks and kernel memory usage within the container.
> --- ./fs/jbd/journal.c.jbdthreads 2007-04-16 11:17:36.000000000 +0400
> +++ ./fs/jbd/journal.c 2007-04-16 11:30:09.000000000 +0400
> @@ -211,10 +211,16 @@ end_loop:
> return 0;
> }
>
> -static void journal_start_thread(journal_t *journal)
> +static int journal_start_thread(journal_t *journal)
> {
> - kthread_run(kjournald, journal, "kjournald");
> + struct task_struct *t;
> +
> + t = kthread_run(kjournald, journal, "kjournald");
> + if (IS_ERR(t))
> + return PTR_ERR(t);
> +
> wait_event(journal->j_wait_done_commit, journal->j_task != 0);
Note that this wait_event should exist at all, and the return
value of kthread_run should be assigned to journal->j_task. Also
the code doesn't use the kthread primitives in other places leading
to crufty code.
|
|
|
|
| Re: [PATCH] Check for error returned by kthread_create on creating journal thread [message #12041 is a reply to message #12039] |
Mon, 16 April 2007 11:07 |
xemul
Messages: 248
Registered: November 2005
|
Senior Member |
|
|
Christoph Hellwig wrote:
> On Mon, Apr 16, 2007 at 11:41:14AM +0400, Pavel Emelianov wrote:
>> If the thread failed to create the subsequent wait_event
>> will hang forever.
>>
>> This is likely to happen if kernel hits max_threads limit.
>>
>> Will be critical for virtualization systems that limit the
>> number of tasks and kernel memory usage within the container.
>
>> --- ./fs/jbd/journal.c.jbdthreads 2007-04-16 11:17:36.000000000 +0400
>> +++ ./fs/jbd/journal.c 2007-04-16 11:30:09.000000000 +0400
>> @@ -211,10 +211,16 @@ end_loop:
>> return 0;
>> }
>>
>> -static void journal_start_thread(journal_t *journal)
>> +static int journal_start_thread(journal_t *journal)
>> {
>> - kthread_run(kjournald, journal, "kjournald");
>> + struct task_struct *t;
>> +
>> + t = kthread_run(kjournald, journal, "kjournald");
>> + if (IS_ERR(t))
>> + return PTR_ERR(t);
>> +
>> wait_event(journal->j_wait_done_commit, journal->j_task != 0);
>
> Note that this wait_event should exist at all, and the return
Should NOT you mean?
> value of kthread_run should be assigned to journal->j_task. Also
> the code doesn't use the kthread primitives in other places leading
> to crufty code.
Well, this could be done with a separate patch, I think.
> -
> 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/
>
|
|
|
|
| Re: [PATCH] Check for error returned by kthread_create on creating journal thread [message #12042 is a reply to message #12041] |
Mon, 16 April 2007 11:10 |
Christoph Hellwig
Messages: 59
Registered: April 2006
|
Member |
|
|
On Mon, Apr 16, 2007 at 03:10:42PM +0400, Pavel Emelianov wrote:
> Christoph Hellwig wrote:
> > On Mon, Apr 16, 2007 at 11:41:14AM +0400, Pavel Emelianov wrote:
> >> If the thread failed to create the subsequent wait_event
> >> will hang forever.
> >>
> >> This is likely to happen if kernel hits max_threads limit.
> >>
> >> Will be critical for virtualization systems that limit the
> >> number of tasks and kernel memory usage within the container.
> >
> >> --- ./fs/jbd/journal.c.jbdthreads 2007-04-16 11:17:36.000000000 +0400
> >> +++ ./fs/jbd/journal.c 2007-04-16 11:30:09.000000000 +0400
> >> @@ -211,10 +211,16 @@ end_loop:
> >> return 0;
> >> }
> >>
> >> -static void journal_start_thread(journal_t *journal)
> >> +static int journal_start_thread(journal_t *journal)
> >> {
> >> - kthread_run(kjournald, journal, "kjournald");
> >> + struct task_struct *t;
> >> +
> >> + t = kthread_run(kjournald, journal, "kjournald");
> >> + if (IS_ERR(t))
> >> + return PTR_ERR(t);
> >> +
> >> wait_event(journal->j_wait_done_commit, journal->j_task != 0);
> >
> > Note that this wait_event should exist at all, and the return
>
> Should NOT you mean?
Umm, yes - of course :)
> > value of kthread_run should be assigned to journal->j_task. Also
> > the code doesn't use the kthread primitives in other places leading
> > to crufty code.
>
> Well, this could be done with a separate patch, I think.
|
|
|
|
|
|
| Re: [PATCH] Check for error returned by kthread_create on creating journal thread [message #12247 is a reply to message #12220] |
Mon, 23 April 2007 06:13 |
xemul
Messages: 248
Registered: November 2005
|
Senior Member |
|
|
Andrew Morton wrote:
> On Mon, 16 Apr 2007 11:41:14 +0400
> Pavel Emelianov <xemul@sw.ru> wrote:
>
>> If the thread failed to create the subsequent wait_event
>> will hang forever.
>>
>> This is likely to happen if kernel hits max_threads limit.
>>
>> Will be critical for virtualization systems that limit the
>> number of tasks and kernel memory usage within the container.
>>
>>
>> [diff-jbd-check-start-journal-thread-return-value text/plain (1.7KB)]
>> --- ./fs/jbd/journal.c.jbdthreads 2007-04-16 11:17:36.000000000 +0400
>> +++ ./fs/jbd/journal.c 2007-04-16 11:30:09.000000000 +0400
>> @@ -211,10 +211,16 @@ end_loop:
>> return 0;
>> }
>>
>> -static void journal_start_thread(journal_t *journal)
>> +static int journal_start_thread(journal_t *journal)
>> {
>> - kthread_run(kjournald, journal, "kjournald");
>> + struct task_struct *t;
>> +
>> + t = kthread_run(kjournald, journal, "kjournald");
>> + if (IS_ERR(t))
>> + return PTR_ERR(t);
>> +
>> wait_event(journal->j_wait_done_commit, journal->j_task != 0);
>> + return 0;
>> }
>
> Thanks. Please don't forget those Signed-off-by:s
I will :)
> I assume that you runtime tested this and that the mount failed in
> an appropriate fashion?
>
Yes. This was easy to test as we can force kthread_run() to fail
at the moment we want with the help of the beancounters ;)
Moreover this patch lives in our tree for a year or so in both
stable and development branches.
|
|
|
|
Goto Forum:
Current Time: Tue May 14 06:09:29 GMT 2024
Total time taken to generate the page: 0.01370 seconds
|
OpenVZ Forum
Members
Search
Help
Register
Login
Home
