public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] async: Handle kthread_run() return codes.
@ 2009-01-13 16:43 Cornelia Huck
  2009-01-13 16:51 ` Ben Dooks
  2009-01-13 20:34 ` Arjan van de Ven
  0 siblings, 2 replies; 5+ messages in thread
From: Cornelia Huck @ 2009-01-13 16:43 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: linux-kernel

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 <cornelia.huck@de.ibm.com>

---
 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;
 }
 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] async: Handle kthread_run() return codes.
  2009-01-13 16:43 [PATCH 1/2] async: Handle kthread_run() return codes Cornelia Huck
@ 2009-01-13 16:51 ` Ben Dooks
  2009-01-13 17:46   ` Cornelia Huck
  2009-01-13 20:34 ` Arjan van de Ven
  1 sibling, 1 reply; 5+ messages in thread
From: Ben Dooks @ 2009-01-13 16:51 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: Arjan van de Ven, linux-kernel

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 <cornelia.huck@de.ibm.com>
> 
> ---
>  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 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/

-- 
Ben (ben@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] async: Handle kthread_run() return codes.
  2009-01-13 16:51 ` Ben Dooks
@ 2009-01-13 17:46   ` Cornelia Huck
  0 siblings, 0 replies; 5+ messages in thread
From: Cornelia Huck @ 2009-01-13 17:46 UTC (permalink / raw)
  To: Ben Dooks; +Cc: Arjan van de Ven, linux-kernel

On Tue, 13 Jan 2009 16:51:15 +0000,
Ben Dooks <ben-linux@fluff.org> 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 <cornelia.huck@de.ibm.com>

---
 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;
 }
 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] async: Handle kthread_run() return codes.
  2009-01-13 16:43 [PATCH 1/2] async: Handle kthread_run() return codes Cornelia Huck
  2009-01-13 16:51 ` Ben Dooks
@ 2009-01-13 20:34 ` Arjan van de Ven
  2009-01-14 10:34   ` Cornelia Huck
  1 sibling, 1 reply; 5+ messages in thread
From: Arjan van de Ven @ 2009-01-13 20:34 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: linux-kernel

>  		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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] async: Handle kthread_run() return codes.
  2009-01-13 20:34 ` Arjan van de Ven
@ 2009-01-14 10:34   ` Cornelia Huck
  0 siblings, 0 replies; 5+ messages in thread
From: Cornelia Huck @ 2009-01-14 10:34 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: linux-kernel

On Tue, 13 Jan 2009 20:34:34 +0000,
Arjan van de Ven <arjan@infradead.org> 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...

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2009-01-14 10:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-13 16:43 [PATCH 1/2] async: Handle kthread_run() return codes Cornelia Huck
2009-01-13 16:51 ` Ben Dooks
2009-01-13 17:46   ` Cornelia Huck
2009-01-13 20:34 ` Arjan van de Ven
2009-01-14 10:34   ` Cornelia Huck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox