[PATCH 1/2] kernel:async function call:introduce async_run_inatomic(v3)

[PATCH 1/2] kernel:async function call:introduce async_run_inatomic(v3)

[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 async_run_inatomic() uses a distinct running list in order to avoid
slowing down synchronization within the general domain.

The comment for async_run_inatomic is under guideline 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..adc0455 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
+ *
+ * The purpose of this function is to offer a simple way to schedule an
+ * asynchronous thread from an atomic context.
+ *
+ * Return zero one success, !zero on failured
+ * Note: async_run_inatomic() uses a distinct running list in order
+ * to avoid slowing down synchronization within the general domain.
+ * Since it does not return a cookie for checkpointing, it is for callers
+ * that don't need later synchronization.
+ */
+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

[PATCH 2/2] driver core:firmware:fix request_firmware_nowait

[tom.leiming]

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

request_firmware_nowait declares it can be called in non-sleep contexts,
but kthead_run called by request_firmware_nowait may sleep. So use
async_run_inatomic to start thread to request firmware.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/base/firmware_class.c |   18 ++++++++----------
 1 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index d186149..99cb21e 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -19,6 +19,7 @@
 #include <linux/kthread.h>
 #include <linux/highmem.h>
 #include <linux/firmware.h>
+#include <linux/async.h>
 #include "base.h"
 
 #define to_dev(obj) container_of(obj, struct device, kobj)
@@ -582,15 +583,15 @@ struct firmware_work {
 	int uevent;
 };
 
-static int
-request_firmware_work_func(void *arg)
+static void
+request_firmware_work_func(void *arg, async_cookie_t cookie)
 {
 	struct firmware_work *fw_work = arg;
 	const struct firmware *fw;
 	int ret;
 	if (!arg) {
 		WARN_ON(1);
-		return 0;
+		return;
 	}
 	ret = _request_firmware(&fw, fw_work->name, fw_work->device,
 		fw_work->uevent);
@@ -602,7 +603,6 @@ request_firmware_work_func(void *arg)
 	}
 	module_put(fw_work->module);
 	kfree(fw_work);
-	return ret;
 }
 
 /**
@@ -626,7 +626,7 @@ request_firmware_nowait(
 	const char *name, struct device *device, void *context,
 	void (*cont)(const struct firmware *fw, void *context))
 {
-	struct task_struct *task;
+	int ret;
 	struct firmware_work *fw_work = kmalloc(sizeof (struct firmware_work),
 						GFP_ATOMIC);
 
@@ -646,14 +646,12 @@ request_firmware_nowait(
 		.uevent = uevent,
 	};
 
-	task = kthread_run(request_firmware_work_func, fw_work,
-			    "firmware/%s", name);
-
-	if (IS_ERR(task)) {
+	ret = async_run_inatomic(request_firmware_work_func, fw_work);
+	if (ret) {
 		fw_work->cont(NULL, fw_work->context);
 		module_put(fw_work->module);
 		kfree(fw_work);
-		return PTR_ERR(task);
+		return ret;
 	}
 	return 0;
 }
-- 
1.6.0.GIT

Re: [PATCH 2/2] driver core:firmware:fix request_firmware_nowait

[Cornelia Huck]

On Sat, 16 May 2009 21:54:18 +0800,
tom.leiming@gmail.com wrote:

> @@ -646,14 +646,12 @@ request_firmware_nowait(
>  		.uevent = uevent,
>  	};
> 
> -	task = kthread_run(request_firmware_work_func, fw_work,
> -			    "firmware/%s", name);
> -
> -	if (IS_ERR(task)) {
> +	ret = async_run_inatomic(request_firmware_work_func, fw_work);
> +	if (ret) {
>  		fw_work->cont(NULL, fw_work->context);
>  		module_put(fw_work->module);
>  		kfree(fw_work);
> -		return PTR_ERR(task);
> +		return ret;

You change the return code from an usual error code to a boolean value
here...

>  	}
>  	return 0;
>  }
 

Re: [PATCH 1/2] kernel:async function call:introduce async_run_inatomic(v3)

[Cornelia Huck]

On Sat, 16 May 2009 21:54:17 +0800,
tom.leiming@gmail.com wrote:

> + /**
> + * 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
> + *
> + * The purpose of this function is to offer a simple way to schedule an
> + * asynchronous thread from an atomic context.
> + *
> + * Return zero one success, !zero on failured
> + * Note: async_run_inatomic() uses a distinct running list in order
> + * to avoid slowing down synchronization within the general domain.
> + * Since it does not return a cookie for checkpointing, it is for callers
> + * that don't need later synchronization.
> + */
> +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);
> +

While we can skip synchronization on cookies, we still need something
like async_run_synchronize() for module unloading (and possibly others).

Re: [PATCH 1/2] kernel:async function call:introduce async_run_inatomic(v3)

[Arjan van de Ven]

On Mon, 18 May 2009 13:10:54 +0200
Cornelia Huck <cornelia.huck@de.ibm.com> wrote:

> On Sat, 16 May 2009 21:54:17 +0800,
> tom.leiming@gmail.com wrote:
> 
> > + /**
> > + * 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
> > + *
> > + * The purpose of this function is to offer a simple way to
> > schedule an
> > + * asynchronous thread from an atomic context.
> > + *
> > + * Return zero one success, !zero on failured
> > + * Note: async_run_inatomic() uses a distinct running list in order
> > + * to avoid slowing down synchronization within the general domain.
> > + * Since it does not return a cookie for checkpointing, it is for
> > callers
> > + * that don't need later synchronization.
> > + */
> > +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);
> > +
> 
> While we can skip synchronization on cookies, we still need something
> like async_run_synchronize() for module unloading (and possibly
> others).

We don't have this problem if we use the existing naming and existing
standard domain... 
(I don't like the "run" part of the name, rather than schedule.. lets
keep the apis consistent please)


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