2009-01-13 16:43:41

by Cornelia Huck

[permalink] [raw]
Subject: [PATCH 1/2] async: Handle kthread_run() return codes.

If we fail to create the manager thread, fall back to non-fastboot.
If we fail to create an async thread, try again when the manager
thread runs again.

Signed-off-by: Cornelia Huck <[email protected]>

---
kernel/async.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)

--- linux-2.6.orig/kernel/async.c
+++ linux-2.6/kernel/async.c
@@ -315,11 +315,14 @@ static int async_manager_thread(void *un
ec = atomic_read(&entry_count);

while (tc < ec && tc < MAX_THREADS) {
- kthread_run(async_thread, NULL, "async/%i", tc);
+ if (IS_ERR(kthread_run(async_thread, NULL, "async/%i",
+ tc)))
+ /* Try again later. */
+ goto schedule;
atomic_inc(&thread_count);
tc++;
}
-
+schedule:
schedule();
}
remove_wait_queue(&async_new, &wq);
@@ -330,7 +333,9 @@ static int async_manager_thread(void *un
static int __init async_init(void)
{
if (async_enabled)
- kthread_run(async_manager_thread, NULL, "async/mgr");
+ if (IS_ERR(kthread_run(async_manager_thread, NULL,
+ "async/mgr")))
+ async_enabled = 0;
return 0;
}


2009-01-13 16:51:34

by Ben Dooks

[permalink] [raw]
Subject: Re: [PATCH 1/2] async: Handle kthread_run() return codes.

On Tue, Jan 13, 2009 at 05:43:04PM +0100, Cornelia Huck wrote:
> If we fail to create the manager thread, fall back to non-fastboot.
> If we fail to create an async thread, try again when the manager
> thread runs again.
>
> Signed-off-by: Cornelia Huck <[email protected]>
>
> ---
> kernel/async.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> --- linux-2.6.orig/kernel/async.c
> +++ linux-2.6/kernel/async.c
> @@ -315,11 +315,14 @@ static int async_manager_thread(void *un
> ec = atomic_read(&entry_count);
>
> while (tc < ec && tc < MAX_THREADS) {

why not add the following to stop the wrapping of kthread_run:

+ struct task_struct *kt;
kt = kthread_run(async_thread, NULL, "async/%i", tc);
if (IS_ERR(kt))
goto schedule;

also, I belive the goto isn't necessary, just break out of the while

if (IS_ERR(kt))
break;

> - kthread_run(async_thread, NULL, "async/%i", tc);
> + if (IS_ERR(kthread_run(async_thread, NULL, "async/%i",
> + tc)))
> + /* Try again later. */
> + goto schedule;
> atomic_inc(&thread_count);
> tc++;
> }
> -
> +schedule:
> schedule();
> }
> remove_wait_queue(&async_new, &wq);
> @@ -330,7 +333,9 @@ static int async_manager_thread(void *un
> static int __init async_init(void)
> {
> if (async_enabled)
> - kthread_run(async_manager_thread, NULL, "async/mgr");
> + if (IS_ERR(kthread_run(async_manager_thread, NULL,
> + "async/mgr")))
> + async_enabled = 0;
> return 0;
> }
>
> --
> 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/

--
Ben ([email protected], http://www.fluff.org/)

'a smiley only costs 4 bytes'

2009-01-13 17:46:56

by Cornelia Huck

[permalink] [raw]
Subject: Re: [PATCH 1/2] async: Handle kthread_run() return codes.

On Tue, 13 Jan 2009 16:51:15 +0000,
Ben Dooks <[email protected]> wrote:

> > @@ -315,11 +315,14 @@ static int async_manager_thread(void *un
> > ec = atomic_read(&entry_count);
> >
> > while (tc < ec && tc < MAX_THREADS) {
>
> why not add the following to stop the wrapping of kthread_run:
>
> + struct task_struct *kt;
> kt = kthread_run(async_thread, NULL, "async/%i", tc);

Hm, I don't think avoiding wrapping is worth adding another variable.

> if (IS_ERR(kt))
> goto schedule;
>
> also, I belive the goto isn't necessary, just break out of the while
>
> if (IS_ERR(kt))
> break;

OK, that's a bit nicer. Updated patch follows.


If we fail to create the manager thread, fall back to non-fastboot.
If we fail to create an async thread, try again when the manager
thread runs again.

Signed-off-by: Cornelia Huck <[email protected]>

---
kernel/async.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

--- linux-2.6.orig/kernel/async.c
+++ linux-2.6/kernel/async.c
@@ -315,7 +315,10 @@ static int async_manager_thread(void *un
ec = atomic_read(&entry_count);

while (tc < ec && tc < MAX_THREADS) {
- kthread_run(async_thread, NULL, "async/%i", tc);
+ if (IS_ERR(kthread_run(async_thread, NULL, "async/%i",
+ tc)))
+ /* Try again later. */
+ break;
atomic_inc(&thread_count);
tc++;
}
@@ -330,7 +333,9 @@ static int async_manager_thread(void *un
static int __init async_init(void)
{
if (async_enabled)
- kthread_run(async_manager_thread, NULL, "async/mgr");
+ if (IS_ERR(kthread_run(async_manager_thread, NULL,
+ "async/mgr")))
+ async_enabled = 0;
return 0;
}

2009-01-14 08:59:36

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH 1/2] async: Handle kthread_run() return codes.

> ec = atomic_read(&entry_count);
>
> while (tc < ec && tc < MAX_THREADS) {
> - kthread_run(async_thread, NULL, "async/%i",
> tc);
> + if (IS_ERR(kthread_run(async_thread, NULL,
> "async/%i",
> + tc)))
> + /* Try again later. */
> + goto schedule;

I do not like this recovery to be honest. an msleep() followed by a
"continue" is probably much better.


> @@ -330,7 +333,9 @@ static int async_manager_thread(void *un
> static int __init async_init(void)
> {
> if (async_enabled)
> - kthread_run(async_manager_thread, NULL, "async/mgr");
> + if (IS_ERR(kthread_run(async_manager_thread, NULL,
> + "async/mgr")))
> + async_enabled = 0;

hmm maybe; it might make more sense to set it to 0 here, and have the
thread itself set the variable to 1..... that way we KNOW the thread is
running for sure..

--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2009-01-14 10:35:18

by Cornelia Huck

[permalink] [raw]
Subject: Re: [PATCH 1/2] async: Handle kthread_run() return codes.

On Tue, 13 Jan 2009 20:34:34 +0000,
Arjan van de Ven <[email protected]> wrote:

> > ec = atomic_read(&entry_count);
> >
> > while (tc < ec && tc < MAX_THREADS) {
> > - kthread_run(async_thread, NULL, "async/%i",
> > tc);
> > + if (IS_ERR(kthread_run(async_thread, NULL,
> > "async/%i",
> > + tc)))
> > + /* Try again later. */
> > + goto schedule;
>
> I do not like this recovery to be honest. an msleep() followed by a
> "continue" is probably much better.

Will change.

>
>
> > @@ -330,7 +333,9 @@ static int async_manager_thread(void *un
> > static int __init async_init(void)
> > {
> > if (async_enabled)
> > - kthread_run(async_manager_thread, NULL, "async/mgr");
> > + if (IS_ERR(kthread_run(async_manager_thread, NULL,
> > + "async/mgr")))
> > + async_enabled = 0;
>
> hmm maybe; it might make more sense to set it to 0 here, and have the
> thread itself set the variable to 1..... that way we KNOW the thread is
> running for sure..

That would look a bit strange to me.

setup_async sets async_enabled -> async_init unsets it again ->
async_manager_thread sets it again

If anything, we could use two variables (use_async and async_enabled),
but that smells of overengeneering to me...