[PATCH] kernel:async function call:introduce async_run_inatomic(v2)

[PATCH] kernel:async function call:introduce async_run_inatomic(v2)

[tom.leiming]

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

The purpose of this function is to offer a simple way to schedule an
asynchronous thread from an atomic context, because there are not any
functions which can create and start a kernel thread in atomic contexts
now. We use async_running_no_sync as running list to make callers that
don't want to synchronize on cookies, so can avoid some side effects for
async_synchronize_full().

Part of note for async_run_inatomic and some guideline is from Cornelia Huck.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 include/linux/async.h |    2 ++
 kernel/async.c        |   23 +++++++++++++++++++++++
 2 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/include/linux/async.h b/include/linux/async.h
index 18ce92b..9ff62af 100644
--- a/include/linux/async.h
+++ b/include/linux/async.h
@@ -16,6 +16,8 @@
 typedef u64 async_cookie_t;
 typedef void (async_func_ptr) (void *data, async_cookie_t cookie);
 
+extern int async_run_inatomic(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 8663f5b..cb527b3 100644
--- a/kernel/async.c
+++ b/kernel/async.c
@@ -65,6 +65,7 @@ static async_cookie_t next_cookie = 1;
 
 static LIST_HEAD(async_pending);
 static LIST_HEAD(async_running);
+static LIST_HEAD(async_running_no_sync);
 static DEFINE_SPINLOCK(async_lock);
 
 static int async_enabled = 0;
@@ -222,6 +223,28 @@ static async_cookie_t __async_schedule(async_func_ptr *ptr, void *data,
 	return newcookie;
 }
 
+ /**
+ * async_run_inatomic - in atomic contexts schedule a function for
+ * 			asynchronous execution
+ *
+ * @ptr: function to execute asynchronously
+ * @data: data pointer to pass to the function
+ *
+ * Return zero if success, or else return others if failured
+ * Note:
+ * The purpose of this function is to offer a simple way to schedule an
+ * asynchronous thread from an atomic context, because there are not any
+ * functions which can create and start a kernel thread in atomic contexts
+ * now. We use async_running_no_sync as running list to make callers that
+ * don't want to synchronize on cookies, so can avoid some side effects for
+ * async_synchronize_full().
+ */
+int async_run_inatomic(async_func_ptr *ptr, void *data)
+{
+	return !__async_schedule(ptr, data, &async_running_no_sync, 1);
+}
+EXPORT_SYMBOL_GPL(async_run_inatomic);
+
 /**
  * async_schedule - schedule a function for asynchronous execution
  * @ptr: function to execute asynchronously
-- 
1.6.0.GIT

Re: [PATCH] kernel:async function call:introduce async_run_inatomic(v2)

[Cornelia Huck]

On Wed, 13 May 2009 23:51:39 +0800,
tom.leiming@gmail.com wrote:

> From: Ming Lei <tom.leiming@gmail.com>
> 
> The purpose of this function is to offer a simple way to schedule an
> asynchronous thread from an atomic context, because there are not any
> functions which can create and start a kernel thread in atomic contexts
> now. We use async_running_no_sync as running list to make callers that
> don't want to synchronize on cookies, so can avoid some side effects for
> async_synchronize_full().
> 
> Part of note for async_run_inatomic and some guideline is from Cornelia Huck.
> 
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
>  include/linux/async.h |    2 ++
>  kernel/async.c        |   23 +++++++++++++++++++++++
>  2 files changed, 25 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/async.h b/include/linux/async.h
> index 18ce92b..9ff62af 100644
> --- a/include/linux/async.h
> +++ b/include/linux/async.h
> @@ -16,6 +16,8 @@
>  typedef u64 async_cookie_t;
>  typedef void (async_func_ptr) (void *data, async_cookie_t cookie);
> 
> +extern int async_run_inatomic(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 8663f5b..cb527b3 100644
> --- a/kernel/async.c
> +++ b/kernel/async.c
> @@ -65,6 +65,7 @@ static async_cookie_t next_cookie = 1;
> 
>  static LIST_HEAD(async_pending);
>  static LIST_HEAD(async_running);
> +static LIST_HEAD(async_running_no_sync);
>  static DEFINE_SPINLOCK(async_lock);
> 
>  static int async_enabled = 0;
> @@ -222,6 +223,28 @@ static async_cookie_t __async_schedule(async_func_ptr *ptr, void *data,
>  	return newcookie;
>  }
> 
> + /**
> + * async_run_inatomic - in atomic contexts schedule a function for
> + * 			asynchronous execution

I'm not 100% sure whether kerneldoc likes split lines for the
description.

"async_run_inatomic - schedule a function for asynchronous execution from atomic context without checkpointing"
- but that's really a bit long...

> + *
> + * @ptr: function to execute asynchronously
> + * @data: data pointer to pass to the function
> + *
> + * Return zero if success, or else return others if failured

Returns zero on success, !zero on failure

> + * Note:

I'd move the description of the function's purpose before Note:.

> + * The purpose of this function is to offer a simple way to schedule an
> + * asynchronous thread from an atomic context, because there are not any
> + * functions which can create and start a kernel thread in atomic contexts
> + * now. 

I don't think the second part of the sentence is needed, this should
only be mentioned in the patch description. I'd rather add

"Since it does not return a cookie for checkpointing, it is for callers
that don't need later synchronization."

> We use async_running_no_sync as running list to make callers that
> + * don't want to synchronize on cookies, so can avoid some side effects for
> + * async_synchronize_full().

"async_run_inatomic() uses a distinct running list in order to avoid
slowing down synchronization within the general domain."

(The cookie will still be checked when __lowest_in_progress() returns
the cookie for the async_pending list, but I think that is negligible -
and it would be a general issue for special domains.)

> + */
> +int async_run_inatomic(async_func_ptr *ptr, void *data)
> +{
> +	return !__async_schedule(ptr, data, &async_running_no_sync, 1);
> +}
> +EXPORT_SYMBOL_GPL(async_run_inatomic);
> +
>  /**
>   * async_schedule - schedule a function for asynchronous execution
>   * @ptr: function to execute asynchronously

Btw: Do you have any specific use cases for this function in mind?

Re: [PATCH] kernel:async function call:introduce async_run_inatomic(v2)

[Frederic Weisbecker]

On Wed, May 13, 2009 at 07:11:44PM +0200, Cornelia Huck wrote:
> On Wed, 13 May 2009 23:51:39 +0800,
> tom.leiming@gmail.com wrote:
> 
> > From: Ming Lei <tom.leiming@gmail.com>
> > 
> > The purpose of this function is to offer a simple way to schedule an
> > asynchronous thread from an atomic context, because there are not any
> > functions which can create and start a kernel thread in atomic contexts
> > now. We use async_running_no_sync as running list to make callers that
> > don't want to synchronize on cookies, so can avoid some side effects for
> > async_synchronize_full().
> > 
> > Part of note for async_run_inatomic and some guideline is from Cornelia Huck.
> > 
> > Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > ---
> >  include/linux/async.h |    2 ++
> >  kernel/async.c        |   23 +++++++++++++++++++++++
> >  2 files changed, 25 insertions(+), 0 deletions(-)
> > 
> > diff --git a/include/linux/async.h b/include/linux/async.h
> > index 18ce92b..9ff62af 100644
> > --- a/include/linux/async.h
> > +++ b/include/linux/async.h
> > @@ -16,6 +16,8 @@
> >  typedef u64 async_cookie_t;
> >  typedef void (async_func_ptr) (void *data, async_cookie_t cookie);
> > 
> > +extern int async_run_inatomic(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 8663f5b..cb527b3 100644
> > --- a/kernel/async.c
> > +++ b/kernel/async.c
> > @@ -65,6 +65,7 @@ static async_cookie_t next_cookie = 1;
> > 
> >  static LIST_HEAD(async_pending);
> >  static LIST_HEAD(async_running);
> > +static LIST_HEAD(async_running_no_sync);
> >  static DEFINE_SPINLOCK(async_lock);
> > 
> >  static int async_enabled = 0;
> > @@ -222,6 +223,28 @@ static async_cookie_t __async_schedule(async_func_ptr *ptr, void *data,
> >  	return newcookie;
> >  }
> > 
> > + /**
> > + * async_run_inatomic - in atomic contexts schedule a function for
> > + * 			asynchronous execution
> 
> I'm not 100% sure whether kerneldoc likes split lines for the
> description.
> 
> "async_run_inatomic - schedule a function for asynchronous execution from atomic context without checkpointing"
> - but that's really a bit long...
> 
> > + *
> > + * @ptr: function to execute asynchronously
> > + * @data: data pointer to pass to the function
> > + *
> > + * Return zero if success, or else return others if failured
> 
> Returns zero on success, !zero on failure
> 
> > + * Note:
> 
> I'd move the description of the function's purpose before Note:.
> 
> > + * The purpose of this function is to offer a simple way to schedule an
> > + * asynchronous thread from an atomic context, because there are not any
> > + * functions which can create and start a kernel thread in atomic contexts
> > + * now. 
> 
> I don't think the second part of the sentence is needed, this should
> only be mentioned in the patch description. I'd rather add
> 
> "Since it does not return a cookie for checkpointing, it is for callers
> that don't need later synchronization."
> 
> > We use async_running_no_sync as running list to make callers that
> > + * don't want to synchronize on cookies, so can avoid some side effects for
> > + * async_synchronize_full().
> 
> "async_run_inatomic() uses a distinct running list in order to avoid
> slowing down synchronization within the general domain."
> 
> (The cookie will still be checked when __lowest_in_progress() returns
> the cookie for the async_pending list, but I think that is negligible -
> and it would be a general issue for special domains.)
> 
> > + */
> > +int async_run_inatomic(async_func_ptr *ptr, void *data)
> > +{
> > +	return !__async_schedule(ptr, data, &async_running_no_sync, 1);
> > +}
> > +EXPORT_SYMBOL_GPL(async_run_inatomic);
> > +
> >  /**
> >   * async_schedule - schedule a function for asynchronous execution
> >   * @ptr: function to execute asynchronously
> 
> Btw: Do you have any specific use cases for this function in mind?


I have some usecases in mind concerning async_schedule_inatomic()
because it still provides a way to synchronize against pending async
jobs. Especially it could replace some lazy workqueues which receive
rare events.

But I wonder if async_run_inatomic() is really matching any common
pattern inside the kernel.
It seems to me rare to never need waiting for an async job completion.

Frederic.

Re: [PATCH] kernel:async function call:introduce async_run_inatomic(v2)

[Arjan van de Ven]

On Thu, 14 May 2009 05:19:39 +0200
Frederic Weisbecker <fweisbec@gmail.com> wrote:

> But I wonder if async_run_inatomic() is really matching any common
> pattern inside the kernel.
> It seems to me rare to never need waiting for an async job completion.

there are some very good reasons to have the option to wait for all;
module unloading comes to mind for example.

I'd not like it to automatically punt all async "inatomic" work to a
separate space; that should be an option for the caller.
If the work is not going to take long, it's actually GOOD that you can
globally synchronize for it...

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

Re: [PATCH] kernel:async function call:introduce async_run_inatomic(v2)

[Cornelia Huck]

On Sun, 17 May 2009 14:03:58 -0700,
Arjan van de Ven <arjan@infradead.org> wrote:

> there are some very good reasons to have the option to wait for all;
> module unloading comes to mind for example.

So we would need async_run_synchronize() as something that waits on the
special domain for those cases...

> 
> I'd not like it to automatically punt all async "inatomic" work to a
> separate space; that should be an option for the caller.

I think the idea is only to push the "simple" cases to that domain; a
normal "inatomic" function that allows a domain to be specified should
be added as well.

Re: [PATCH] kernel:async function call:introduce async_run_inatomic(v2)

[Ming Lei]

2009/5/14 Cornelia Huck <cornelia.huck@de.ibm.com>:
> On Wed, 13 May 2009 23:51:39 +0800,
> tom.leiming@gmail.com wrote:
>
>> From: Ming Lei <tom.leiming@gmail.com>
>>
>> The purpose of this function is to offer a simple way to schedule an
>> asynchronous thread from an atomic context, because there are not any
>> functions which can create and start a kernel thread in atomic contexts
>> now. We use async_running_no_sync as running list to make callers that
>> don't want to synchronize on cookies, so can avoid some side effects for
>> async_synchronize_full().
>>
>> Part of note for async_run_inatomic and some guideline is from Cornelia Huck.
>>
>> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
>> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
>> ---
>>  include/linux/async.h |    2 ++
>>  kernel/async.c        |   23 +++++++++++++++++++++++
>>  2 files changed, 25 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/async.h b/include/linux/async.h
>> index 18ce92b..9ff62af 100644
>> --- a/include/linux/async.h
>> +++ b/include/linux/async.h
>> @@ -16,6 +16,8 @@
>>  typedef u64 async_cookie_t;
>>  typedef void (async_func_ptr) (void *data, async_cookie_t cookie);
>>
>> +extern int async_run_inatomic(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 8663f5b..cb527b3 100644
>> --- a/kernel/async.c
>> +++ b/kernel/async.c
>> @@ -65,6 +65,7 @@ static async_cookie_t next_cookie = 1;
>>
>>  static LIST_HEAD(async_pending);
>>  static LIST_HEAD(async_running);
>> +static LIST_HEAD(async_running_no_sync);
>>  static DEFINE_SPINLOCK(async_lock);
>>
>>  static int async_enabled = 0;
>> @@ -222,6 +223,28 @@ static async_cookie_t __async_schedule(async_func_ptr *ptr, void *data,
>>       return newcookie;
>>  }
>>
>> + /**
>> + * async_run_inatomic - in atomic contexts schedule a function for
>> + *                   asynchronous execution
>
> I'm not 100% sure whether kerneldoc likes split lines for the
> description.
>
> "async_run_inatomic - schedule a function for asynchronous execution from atomic context without checkpointing"
> - but that's really a bit long...
>
>> + *
>> + * @ptr: function to execute asynchronously
>> + * @data: data pointer to pass to the function
>> + *
>> + * Return zero if success, or else return others if failured
>
> Returns zero on success, !zero on failure
>
>> + * Note:
>
> I'd move the description of the function's purpose before Note:.
>
>> + * The purpose of this function is to offer a simple way to schedule an
>> + * asynchronous thread from an atomic context, because there are not any
>> + * functions which can create and start a kernel thread in atomic contexts
>> + * now.
>
> I don't think the second part of the sentence is needed, this should
> only be mentioned in the patch description. I'd rather add
>
> "Since it does not return a cookie for checkpointing, it is for callers
> that don't need later synchronization."
>
>> We use async_running_no_sync as running list to make callers that
>> + * don't want to synchronize on cookies, so can avoid some side effects for
>> + * async_synchronize_full().
>
> "async_run_inatomic() uses a distinct running list in order to avoid
> slowing down synchronization within the general domain."
>
> (The cookie will still be checked when __lowest_in_progress() returns
> the cookie for the async_pending list, but I think that is negligible -
> and it would be a general issue for special domains.)

Yes, it still needs some synchronization before the non-sync function
entry is moved
into async_running_no_sync, but it is negligible really.

>
>> + */
>> +int async_run_inatomic(async_func_ptr *ptr, void *data)
>> +{
>> +     return !__async_schedule(ptr, data, &async_running_no_sync, 1);
>> +}
>> +EXPORT_SYMBOL_GPL(async_run_inatomic);
>> +
>>  /**
>>   * async_schedule - schedule a function for asynchronous execution
>>   * @ptr: function to execute asynchronously
>
> Btw: Do you have any specific use cases for this function in mind?
>

For example request_firmware_nowait(), this function declares it can be
called in contexts where it is not possible to sleep, but really can
not be called
in irq contexts, because the function calls kthread_run to do firmware loading.

-- 
Lei Ming