[PATCH] kernel:async function call:introduce async_run

[PATCH] kernel:async function call:introduce async_run

[tom.leiming]

From: Ming Lei <tom.leiming@gmail.com>

In fact, some async function call does not need to check, async_run
is right for this kind of function call. Also, async_run is very
suitable to used to start a thread in atomic context to do somthing,
for now there is no such kind of kernel api, eg. kthread_run
can not be started in atomic context.

This patch is againt my another patch:
   kernel/async.c:introduce async_schedule*_atomic
please review.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 include/linux/async.h |    1 +
 kernel/async.c        |   24 +++++++++++++++++++++++-
 2 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/include/linux/async.h b/include/linux/async.h
index ede9849..5390572 100644
--- a/include/linux/async.h
+++ b/include/linux/async.h
@@ -16,6 +16,7 @@
 typedef u64 async_cookie_t;
 typedef void (async_func_ptr) (void *data, async_cookie_t cookie);
 
+extern void async_run(async_func_ptr *ptr, void *data);
 extern async_cookie_t async_schedule(async_func_ptr *ptr, void *data);
 extern async_cookie_t async_schedule_domain(async_func_ptr *ptr, void *data,
 					    struct list_head *list);
diff --git a/kernel/async.c b/kernel/async.c
index 6bf565b..9cc4670 100644
--- a/kernel/async.c
+++ b/kernel/async.c
@@ -60,6 +60,7 @@ asynchronous and synchronous parts of the kernel.
 
 static async_cookie_t next_cookie = 1;
 
+#define MAX_COOKIE	(~0)
 #define MAX_THREADS	256
 #define MAX_WORK	32768
 
@@ -207,7 +208,10 @@ static async_cookie_t __async_schedule(async_func_ptr *ptr, void *data, \
 	entry->running = running;
 
 	spin_lock_irqsave(&async_lock, flags);
-	newcookie = entry->cookie = next_cookie++;
+	if (atomic == 1)
+		newcookie = entry->cookie = next_cookie++;
+	else
+		newcookie = entry->cookie = MAX_COOKIE;
 	list_add_tail(&entry->list, &async_pending);
 	atomic_inc(&entry_count);
 	spin_unlock_irqrestore(&async_lock, flags);
@@ -216,6 +220,24 @@ static async_cookie_t __async_schedule(async_func_ptr *ptr, void *data, \
 }
 
 /**
+ * async_run - schedule a function for asynchronous execution
+ * @ptr: function to execute asynchronously
+ * @data: data pointer to pass to the function
+ *
+ * Note:we do not allocate a cookie for this kind of aysnchronous
+ * function to decrease the wait time of async_synchronize_full().
+ * In fact, some function does not need to check, async_run is right
+ * for this kind of function call. Also, async_run is very suitable to
+ * start a thread to do somthing in atomic context,for now there is no such
+ * kind of kernel api, eg. kthread_run can not be run in atomic context.
+ */
+void async_run(async_func_ptr *ptr, void *data)
+{
+	__async_schedule(ptr, data, &async_running, 2);
+}
+EXPORT_SYMBOL_GPL(async_run);
+
+/**
  * async_schedule - schedule a function for asynchronous execution
  * @ptr: function to execute asynchronously
  * @data: data pointer to pass to the function
-- 
1.6.0.GIT

Re: [PATCH] kernel:async function call:introduce async_run

[Cornelia Huck]

On Wed, 13 May 2009 08:33:49 +0800,
tom.leiming@gmail.com wrote:

> From: Ming Lei <tom.leiming@gmail.com>
> 
> In fact, some async function call does not need to check, async_run
> is right for this kind of function call. Also, async_run is very
> suitable to used to start a thread in atomic context to do somthing,
> for now there is no such kind of kernel api, eg. kthread_run
> can not be started in atomic context.

Could you rework your explanation a bit? If I understand correctly, you
want to introduce a way to queue an async thread for those callers that
don't want to synchronize on cookies.

> 
> This patch is againt my another patch:
>    kernel/async.c:introduce async_schedule*_atomic
> please review.
> 
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> ---
>  include/linux/async.h |    1 +
>  kernel/async.c        |   24 +++++++++++++++++++++++-
>  2 files changed, 24 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/async.h b/include/linux/async.h
> index ede9849..5390572 100644
> --- a/include/linux/async.h
> +++ b/include/linux/async.h
> @@ -16,6 +16,7 @@
>  typedef u64 async_cookie_t;
>  typedef void (async_func_ptr) (void *data, async_cookie_t cookie);
> 
> +extern void async_run(async_func_ptr *ptr, void *data);
>  extern async_cookie_t async_schedule(async_func_ptr *ptr, void *data);
>  extern async_cookie_t async_schedule_domain(async_func_ptr *ptr, void *data,
>  					    struct list_head *list);
> diff --git a/kernel/async.c b/kernel/async.c
> index 6bf565b..9cc4670 100644
> --- a/kernel/async.c
> +++ b/kernel/async.c
> @@ -60,6 +60,7 @@ asynchronous and synchronous parts of the kernel.
> 
>  static async_cookie_t next_cookie = 1;
> 
> +#define MAX_COOKIE	(~0)
>  #define MAX_THREADS	256
>  #define MAX_WORK	32768
> 
> @@ -207,7 +208,10 @@ static async_cookie_t __async_schedule(async_func_ptr *ptr, void *data, \
>  	entry->running = running;
> 
>  	spin_lock_irqsave(&async_lock, flags);
> -	newcookie = entry->cookie = next_cookie++;
> +	if (atomic == 1)
> +		newcookie = entry->cookie = next_cookie++;
> +	else
> +		newcookie = entry->cookie = MAX_COOKIE;

This confuses me. Why do you change the behaviour for atomic == 0?

>  	list_add_tail(&entry->list, &async_pending);
>  	atomic_inc(&entry_count);
>  	spin_unlock_irqrestore(&async_lock, flags);
> @@ -216,6 +220,24 @@ static async_cookie_t __async_schedule(async_func_ptr *ptr, void *data, \
>  }
> 
>  /**
> + * async_run - schedule a function for asynchronous execution
> + * @ptr: function to execute asynchronously
> + * @data: data pointer to pass to the function
> + *
> + * Note:we do not allocate a cookie for this kind of aysnchronous
> + * function to decrease the wait time of async_synchronize_full().

But async_synchronize_full() still waits for list_empty(&async_running)
- so what does this buy us?

> + * In fact, some function does not need to check, async_run is right
> + * for this kind of function call. Also, async_run is very suitable to
> + * start a thread to do somthing in atomic context,for now there is no such
> + * kind of kernel api, eg. kthread_run can not be run in atomic context.

Hm...
"The purpose of this function is to offer a simple way to schedule an
asynchronous thread, especially from an atomic context."
Would that describe async_run() better?

Doesn't this function need a return code since queueing the async work
can fail?

> + */
> +void async_run(async_func_ptr *ptr, void *data)
> +{
> +	__async_schedule(ptr, data, &async_running, 2);

I don't like the overloading of the "atomic" value - if I causually
looked at the declaration of __async_schedule(), I'd think it would be
a kind of boolean value...

> +}
> +EXPORT_SYMBOL_GPL(async_run);
> +
> +/**
>   * async_schedule - schedule a function for asynchronous execution
>   * @ptr: function to execute asynchronously
>   * @data: data pointer to pass to the function

Re: [PATCH] kernel:async function call:introduce async_run

[Ming Lei]

2009/5/13 Cornelia Huck <cornelia.huck@de.ibm.com>:
> On Wed, 13 May 2009 08:33:49 +0800,
> tom.leiming@gmail.com wrote:
>
>> From: Ming Lei <tom.leiming@gmail.com>
>>
>> In fact, some async function call does not need to check, async_run
>> is right for this kind of function call. Also, async_run is very
>> suitable to used to start a thread in atomic context to do somthing,
>> for now there is no such kind of kernel api, eg. kthread_run
>> can not be started in atomic context.
>
> Could you rework your explanation a bit? If I understand correctly, you
> want to introduce a way to queue an async thread for those callers that
> don't want to synchronize on cookies.
>
>>
>> This patch is againt my another patch:
>>    kernel/async.c:introduce async_schedule*_atomic
>> please review.
>>
>> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
>> ---
>>  include/linux/async.h |    1 +
>>  kernel/async.c        |   24 +++++++++++++++++++++++-
>>  2 files changed, 24 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/linux/async.h b/include/linux/async.h
>> index ede9849..5390572 100644
>> --- a/include/linux/async.h
>> +++ b/include/linux/async.h
>> @@ -16,6 +16,7 @@
>>  typedef u64 async_cookie_t;
>>  typedef void (async_func_ptr) (void *data, async_cookie_t cookie);
>>
>> +extern void async_run(async_func_ptr *ptr, void *data);
>>  extern async_cookie_t async_schedule(async_func_ptr *ptr, void *data);
>>  extern async_cookie_t async_schedule_domain(async_func_ptr *ptr, void *data,
>>                                           struct list_head *list);
>> diff --git a/kernel/async.c b/kernel/async.c
>> index 6bf565b..9cc4670 100644
>> --- a/kernel/async.c
>> +++ b/kernel/async.c
>> @@ -60,6 +60,7 @@ asynchronous and synchronous parts of the kernel.
>>
>>  static async_cookie_t next_cookie = 1;
>>
>> +#define MAX_COOKIE   (~0)
>>  #define MAX_THREADS  256
>>  #define MAX_WORK     32768
>>
>> @@ -207,7 +208,10 @@ static async_cookie_t __async_schedule(async_func_ptr *ptr, void *data, \
>>       entry->running = running;
>>
>>       spin_lock_irqsave(&async_lock, flags);
>> -     newcookie = entry->cookie = next_cookie++;
>> +     if (atomic == 1)
>> +             newcookie = entry->cookie = next_cookie++;
>> +     else
>> +             newcookie = entry->cookie = MAX_COOKIE;
>
> This confuses me. Why do you change the behaviour for atomic == 0?

Yes, it needs to be fixed.

>
>>       list_add_tail(&entry->list, &async_pending);
>>       atomic_inc(&entry_count);
>>       spin_unlock_irqrestore(&async_lock, flags);
>> @@ -216,6 +220,24 @@ static async_cookie_t __async_schedule(async_func_ptr *ptr, void *data, \
>>  }
>>
>>  /**
>> + * async_run - schedule a function for asynchronous execution
>> + * @ptr: function to execute asynchronously
>> + * @data: data pointer to pass to the function
>> + *
>> + * Note:we do not allocate a cookie for this kind of aysnchronous
>> + * function to decrease the wait time of async_synchronize_full().
>
> But async_synchronize_full() still waits for list_empty(&async_running)
> - so what does this buy us?

I mean it can decrease the wait time for other async function.
async_schedule() still can be used to do such thing, but may lead to a
slower boot.  It is the main
purpose of the patch.

>
>> + * In fact, some function does not need to check, async_run is right
>> + * for this kind of function call. Also, async_run is very suitable to
>> + * start a thread to do somthing in atomic context,for now there is no such
>> + * kind of kernel api, eg. kthread_run can not be run in atomic context.
>
> Hm...
> "The purpose of this function is to offer a simple way to schedule an
> asynchronous thread, especially from an atomic context."
> Would that describe async_run() better?

Yes, very good, please forgive my poor english.

Thanks!

-- 
Lei Ming

Re: [PATCH] kernel:async function call:introduce async_run

[Cornelia Huck]

On Wed, 13 May 2009 20:56:40 +0800,
Ming Lei <tom.leiming@gmail.com> wrote:

> 2009/5/13 Cornelia Huck <cornelia.huck@de.ibm.com>:
> > On Wed, 13 May 2009 08:33:49 +0800,
> > tom.leiming@gmail.com wrote:

> >>  /**
> >> + * async_run - schedule a function for asynchronous execution
> >> + * @ptr: function to execute asynchronously
> >> + * @data: data pointer to pass to the function
> >> + *
> >> + * Note:we do not allocate a cookie for this kind of aysnchronous
> >> + * function to decrease the wait time of async_synchronize_full().
> >
> > But async_synchronize_full() still waits for list_empty(&async_running)
> > - so what does this buy us?
> 
> I mean it can decrease the wait time for other async function.
> async_schedule() still can be used to do such thing, but may lead to a
> slower boot.  It is the main
> purpose of the patch.

I see how this can affect places calling async_synchronize_cookie(),
but the function will still end up on async_running. If you don't want
async_synchronize_full() waiting for these functions, couldn't you use
your own running list?

(Oh, and I just thought about it a bit further:
- somebody calls async_run() -> function with cookie = MAX_COOKIE will be
  lowest_in_progress at some point in time
- somebody else calls async_schedule() -> cookie = n
- we wait with async_synchronize_cookie(n) - which returns since
  MAX_COOKIE >= n, which is probably not what we want)

Re: [PATCH] kernel:async function call:introduce async_run

[Ming Lei]

2009/5/13 Cornelia Huck <cornelia.huck@de.ibm.com>:
> On Wed, 13 May 2009 20:56:40 +0800,
> Ming Lei <tom.leiming@gmail.com> wrote:
>
>> 2009/5/13 Cornelia Huck <cornelia.huck@de.ibm.com>:
>> > On Wed, 13 May 2009 08:33:49 +0800,
>> > tom.leiming@gmail.com wrote:
>
>> >>  /**
>> >> + * async_run - schedule a function for asynchronous execution
>> >> + * @ptr: function to execute asynchronously
>> >> + * @data: data pointer to pass to the function
>> >> + *
>> >> + * Note:we do not allocate a cookie for this kind of aysnchronous
>> >> + * function to decrease the wait time of async_synchronize_full().
>> >
>> > But async_synchronize_full() still waits for list_empty(&async_running)
>> > - so what does this buy us?
>>
>> I mean it can decrease the wait time for other async function.
>> async_schedule() still can be used to do such thing, but may lead to a
>> slower boot.  It is the main
>> purpose of the patch.
>
> I see how this can affect places calling async_synchronize_cookie(),
> but the function will still end up on async_running. If you don't want
> async_synchronize_full() waiting for these functions, couldn't you use
> your own running list?
>
> (Oh, and I just thought about it a bit further:
> - somebody calls async_run() -> function with cookie = MAX_COOKIE will be
>  lowest_in_progress at some point in time
> - somebody else calls async_schedule() -> cookie = n
> - we wait with async_synchronize_cookie(n) - which returns since
>  MAX_COOKIE >= n, which is probably not what we want)
>
yes , you are right, I'll think about it.



-- 
Lei Ming

Re: [PATCH] kernel:async function call:introduce async_run

[Arjan van de Ven]

On Wed, 13 May 2009 08:33:49 +0800
tom.leiming@gmail.com wrote:

> From: Ming Lei <tom.leiming@gmail.com>
> 
> In fact, some async function call does not need to check, async_run
> is right for this kind of function call. Also, async_run is very
> suitable to used to start a thread in atomic context to do somthing,
> for now there is no such kind of kernel api, eg. kthread_run
> can not be started in atomic context.

I don't think it's worth complicating the cookie allocation;
I don't assume that what you're scheduling is going to take a really
long time...

if it does it's better to make a private domain for it.

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