[PATCH 2/2] async: Add some documentation.

[PATCH 2/2] async: Add some documentation.

[Cornelia Huck]

Add some kerneldoc to the async interface.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>

---
 kernel/async.c |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

--- linux-2.6.orig/kernel/async.c
+++ linux-2.6/kernel/async.c
@@ -205,18 +205,43 @@ static async_cookie_t __async_schedule(a
 	return newcookie;
 }
 
+/**
+ * async_schedule - schedule a function for asynchronous execution
+ * @ptr: function to execute asynchronously
+ * @data: data pointer to pass to the function
+ *
+ * Returns an async_cookie_t that may be used for checkpointing later.
+ * Note: This function may be called from atomic or non-atomic contexts.
+ */
 async_cookie_t async_schedule(async_func_ptr *ptr, void *data)
 {
 	return __async_schedule(ptr, data, &async_pending);
 }
 EXPORT_SYMBOL_GPL(async_schedule);
 
+/**
+ * async_schedule_special - schedule a function for asynchronous execution with a special running queue
+ * @ptr: function to execute asynchronously
+ * @data: data pointer to pass to the function
+ * @running: list head to add to while running
+ *
+ * Returns an async_cookie_t that may be used for checkpointing later.
+ * @running may be used in the async_synchronize_*_special() functions
+ * to wait on a special running queue rather than on the global running
+ * queue.
+ * Note: This function may be called from atomic or non-atomic contexts.
+ */
 async_cookie_t async_schedule_special(async_func_ptr *ptr, void *data, struct list_head *running)
 {
 	return __async_schedule(ptr, data, running);
 }
 EXPORT_SYMBOL_GPL(async_schedule_special);
 
+/**
+ * async_synchronize_full - synchronize all asynchronous function calls
+ *
+ * This function waits until all asynchronous function calls have been done.
+ */
 void async_synchronize_full(void)
 {
 	do {
@@ -225,12 +250,27 @@ void async_synchronize_full(void)
 }
 EXPORT_SYMBOL_GPL(async_synchronize_full);
 
+/**
+ * async_synchronize_full_special - synchronize all asynchronous function calls for a running list
+ * @list: running list to synchronize on
+ *
+ * This function waits until all asynchronous function calls for the running
+ * list @list have been done.
+ */
 void async_synchronize_full_special(struct list_head *list)
 {
 	async_synchronize_cookie_special(next_cookie, list);
 }
 EXPORT_SYMBOL_GPL(async_synchronize_full_special);
 
+/**
+ * async_synchronize_cookie_special - synchronize asynchronous function calls on a running list with cookie checkpointing
+ * @cookie: async_cookie_t to use as checkpoint
+ * @running: running list to synchronize on
+ *
+ * This function waits until all asynchronous function calls for the running
+ * list @list submitted prior to @cookie have been done.
+ */
 void async_synchronize_cookie_special(async_cookie_t cookie, struct list_head *running)
 {
 	ktime_t starttime, delta, endtime;
@@ -252,6 +292,13 @@ void async_synchronize_cookie_special(as
 }
 EXPORT_SYMBOL_GPL(async_synchronize_cookie_special);
 
+/**
+ * async_synchronize_cookie - synchronize asynchronous function calls with cookie checkpointing
+ * @cookie: async_cookie_t to use as checkpoint
+ *
+ * This function waits until all asynchronous function calls prior to @cookie
+ * have been done.
+ */
 void async_synchronize_cookie(async_cookie_t cookie)
 {
 	async_synchronize_cookie_special(cookie, &async_running);

Re: [PATCH 2/2] async: Add some documentation.

[Arjan van de Ven]

On Tue, 13 Jan 2009 17:43:06 +0100
Cornelia Huck <cornelia.huck@de.ibm.com> wrote:


Thanks for providing this documentation!

Acked-by: Arjan van de Ven <arjan@linux.intel.com>

I'll send this to Linus/Andrew with the next batch


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

Re: [PATCH 2/2] async: Add some documentation.

[Dave Chinner]

On Tue, Jan 13, 2009 at 05:43:06PM +0100, Cornelia Huck wrote:
> Add some kerneldoc to the async interface.
> 
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> +/**
> + * async_schedule_special - schedule a function for asynchronous execution with a special running queue
> + * @ptr: function to execute asynchronously
> + * @data: data pointer to pass to the function
> + * @running: list head to add to while running
> + *
> + * Returns an async_cookie_t that may be used for checkpointing later.
> + * @running may be used in the async_synchronize_*_special() functions
> + * to wait on a special running queue rather than on the global running
> + * queue.
> + * Note: This function may be called from atomic or non-atomic contexts.
> + */
>  async_cookie_t async_schedule_special(async_func_ptr *ptr, void *data, struct list_head *running)

Rather than polishing a turd, can we rename this "special" stuff to
something more descriptive? I'm not the only person to complain
about this. How about async_schedule_list()?

After all, async_schedule_list() describes *exactly* how it is
different to async_schedule(), while the "_special" keywords really
suck when you consider code is supposed to be self documenting....

> +/**
> + * async_synchronize_cookie_special - synchronize asynchronous function calls on a running list with cookie checkpointing
> + * @cookie: async_cookie_t to use as checkpoint
> + * @running: running list to synchronize on

And I think that description proves my point about the real
meaning of "special" in this API.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 2/2] async: Add some documentation.

[Cornelia Huck]

On Wed, 14 Jan 2009 13:49:52 +1100,
Dave Chinner <david@fromorbit.com> wrote:

> On Tue, Jan 13, 2009 at 05:43:06PM +0100, Cornelia Huck wrote:
> > Add some kerneldoc to the async interface.
> > 
> > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > +/**
> > + * async_schedule_special - schedule a function for asynchronous execution with a special running queue
> > + * @ptr: function to execute asynchronously
> > + * @data: data pointer to pass to the function
> > + * @running: list head to add to while running
> > + *
> > + * Returns an async_cookie_t that may be used for checkpointing later.
> > + * @running may be used in the async_synchronize_*_special() functions
> > + * to wait on a special running queue rather than on the global running
> > + * queue.
> > + * Note: This function may be called from atomic or non-atomic contexts.
> > + */
> >  async_cookie_t async_schedule_special(async_func_ptr *ptr, void *data, struct list_head *running)
> 
> Rather than polishing a turd, can we rename this "special" stuff to
> something more descriptive? I'm not the only person to complain
> about this. How about async_schedule_list()?
> 
> After all, async_schedule_list() describes *exactly* how it is
> different to async_schedule(), while the "_special" keywords really
> suck when you consider code is supposed to be self documenting....

async_schedule_list() sounds better, agreed, but I'd prefer to change
that in a seperate patch.

Re: [PATCH 2/2] async: Add some documentation.

[Arjan van de Ven]

On Wed, 14 Jan 2009 11:24:50 +0100
Cornelia Huck <cornelia.huck@de.ibm.com> wrote:

> > Rather than polishing a turd, can we rename this "special" stuff to
> > something more descriptive? I'm not the only person to complain
> > about this. How about async_schedule_list()?
> > 
> > After all, async_schedule_list() describes *exactly* how it is
> > different to async_schedule(), while the "_special" keywords really
> > suck when you consider code is supposed to be self documenting....
> 
> async_schedule_list() sounds better, agreed, but I'd prefer to change
> that in a seperate patch.

I had it as that at first. But it is ugly; naming a function after its
arguments is useless; it should be named after what it does instead.

I buy that "special" is not a good name. Would "local" be better?
The name needs to convey that it is for a specific synchronization
context....


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

Re: [PATCH 2/2] async: Add some documentation.

[Dave Chinner]

On Sun, Jan 18, 2009 at 04:39:12PM -0800, Arjan van de Ven wrote:
> On Wed, 14 Jan 2009 11:24:50 +0100
> Cornelia Huck <cornelia.huck@de.ibm.com> wrote:
> 
> > > Rather than polishing a turd, can we rename this "special" stuff to
> > > something more descriptive? I'm not the only person to complain
> > > about this. How about async_schedule_list()?
> > > 
> > > After all, async_schedule_list() describes *exactly* how it is
> > > different to async_schedule(), while the "_special" keywords really
> > > suck when you consider code is supposed to be self documenting....
> > 
> > async_schedule_list() sounds better, agreed, but I'd prefer to change
> > that in a seperate patch.
> 
> I had it as that at first. But it is ugly; naming a function after its
> arguments is useless; it should be named after what it does instead.
> 
> I buy that "special" is not a good name. Would "local" be better?
> The name needs to convey that it is for a specific synchronization
> context....

Yeah, local is sounds ok - it's certainly more obvious
that it's a scope modifier for the synchronisation primitive.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 2/2] async: Add some documentation.

[Cornelia Huck]

On Mon, 19 Jan 2009 15:40:45 +1100,
Dave Chinner <david@fromorbit.com> wrote:

> On Sun, Jan 18, 2009 at 04:39:12PM -0800, Arjan van de Ven wrote:
> > On Wed, 14 Jan 2009 11:24:50 +0100
> > Cornelia Huck <cornelia.huck@de.ibm.com> wrote:
> > 
> > > > Rather than polishing a turd, can we rename this "special" stuff to
> > > > something more descriptive? I'm not the only person to complain
> > > > about this. How about async_schedule_list()?
> > > > 
> > > > After all, async_schedule_list() describes *exactly* how it is
> > > > different to async_schedule(), while the "_special" keywords really
> > > > suck when you consider code is supposed to be self documenting....
> > > 
> > > async_schedule_list() sounds better, agreed, but I'd prefer to change
> > > that in a seperate patch.
> > 
> > I had it as that at first. But it is ugly; naming a function after its
> > arguments is useless; it should be named after what it does instead.
> > 
> > I buy that "special" is not a good name. Would "local" be better?
> > The name needs to convey that it is for a specific synchronization
> > context....
> 
> Yeah, local is sounds ok - it's certainly more obvious
> that it's a scope modifier for the synchronisation primitive.

Hm, I don't like _local too much. How about _subset, or _context, or
_scope?

Re: [PATCH 2/2] async: Add some documentation.

[Arjan van de Ven]

On Mon, 19 Jan 2009 13:27:44 +0100
Cornelia Huck <cornelia.huck@de.ibm.com> wrote:

> > > 
> > > I had it as that at first. But it is ugly; naming a function
> > > after its arguments is useless; it should be named after what it
> > > does instead.
> > > 
> > > I buy that "special" is not a good name. Would "local" be better?
> > > The name needs to convey that it is for a specific synchronization
> > > context....
> > 
> > Yeah, local is sounds ok - it's certainly more obvious
> > that it's a scope modifier for the synchronisation primitive.
> 
> Hm, I don't like _local too much. How about _subset, or _context, or
> _scope?

or _domain ?

and phrase stuff such that you have synchronization domains?

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

Re: [PATCH 2/2] async: Add some documentation.

[Cornelia Huck]

On Mon, 19 Jan 2009 04:52:42 -0800,
Arjan van de Ven <arjan@infradead.org> wrote:

> On Mon, 19 Jan 2009 13:27:44 +0100
> Cornelia Huck <cornelia.huck@de.ibm.com> wrote:
> 
> > > > 
> > > > I had it as that at first. But it is ugly; naming a function
> > > > after its arguments is useless; it should be named after what it
> > > > does instead.
> > > > 
> > > > I buy that "special" is not a good name. Would "local" be better?
> > > > The name needs to convey that it is for a specific synchronization
> > > > context....
> > > 
> > > Yeah, local is sounds ok - it's certainly more obvious
> > > that it's a scope modifier for the synchronisation primitive.
> > 
> > Hm, I don't like _local too much. How about _subset, or _context, or
> > _scope?
> 
> or _domain ?
> 
> and phrase stuff such that you have synchronization domains?

I like that one best so far.

[PATCH] async: Rename _special -> _domain for clarity.

[Cornelia Huck]

Rename the async_*_special() functions to async_*_domain(), which
describes the purpose of these functions much better.
[Broke up long lines to silence checkpatch]

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>

---
 fs/super.c            |    4 ++--
 include/linux/async.h |    8 +++++---
 kernel/async.c        |   41 ++++++++++++++++++++++-------------------
 3 files changed, 29 insertions(+), 24 deletions(-)

--- linux-2.6.orig/include/linux/async.h
+++ linux-2.6/include/linux/async.h
@@ -17,9 +17,11 @@ typedef u64 async_cookie_t;
 typedef void (async_func_ptr) (void *data, async_cookie_t cookie);
 
 extern async_cookie_t async_schedule(async_func_ptr *ptr, void *data);
-extern async_cookie_t async_schedule_special(async_func_ptr *ptr, void *data, struct list_head *list);
+extern async_cookie_t async_schedule_domain(async_func_ptr *ptr, void *data,
+					    struct list_head *list);
 extern void async_synchronize_full(void);
-extern void async_synchronize_full_special(struct list_head *list);
+extern void async_synchronize_full_domain(struct list_head *list);
 extern void async_synchronize_cookie(async_cookie_t cookie);
-extern void async_synchronize_cookie_special(async_cookie_t cookie, struct list_head *list);
+extern void async_synchronize_cookie_domain(async_cookie_t cookie,
+					    struct list_head *list);
 
--- linux-2.6.orig/kernel/async.c
+++ linux-2.6/kernel/async.c
@@ -221,22 +221,23 @@ async_cookie_t async_schedule(async_func
 EXPORT_SYMBOL_GPL(async_schedule);
 
 /**
- * async_schedule_special - schedule a function for asynchronous execution with a special running queue
+ * async_schedule_domain - schedule a function for asynchronous execution within a certain domain
  * @ptr: function to execute asynchronously
  * @data: data pointer to pass to the function
- * @running: list head to add to while running
+ * @running: running list for the domain
  *
  * Returns an async_cookie_t that may be used for checkpointing later.
- * @running may be used in the async_synchronize_*_special() functions
- * to wait on a special running queue rather than on the global running
- * queue.
+ * @running may be used in the async_synchronize_*_domain() functions
+ * to wait within a certain synchronization domain rather than globally.
+ * A synchronization domain is specified via the running queue @running to use.
  * Note: This function may be called from atomic or non-atomic contexts.
  */
-async_cookie_t async_schedule_special(async_func_ptr *ptr, void *data, struct list_head *running)
+async_cookie_t async_schedule_domain(async_func_ptr *ptr, void *data,
+				     struct list_head *running)
 {
 	return __async_schedule(ptr, data, running);
 }
-EXPORT_SYMBOL_GPL(async_schedule_special);
+EXPORT_SYMBOL_GPL(async_schedule_domain);
 
 /**
  * async_synchronize_full - synchronize all asynchronous function calls
@@ -252,27 +253,29 @@ void async_synchronize_full(void)
 EXPORT_SYMBOL_GPL(async_synchronize_full);
 
 /**
- * async_synchronize_full_special - synchronize all asynchronous function calls for a running list
+ * async_synchronize_full_domain - synchronize all asynchronous function within a certain domain
  * @list: running list to synchronize on
  *
- * This function waits until all asynchronous function calls for the running
- * list @list have been done.
+ * This function waits until all asynchronous function calls for the
+ * synchronization domain specified by the running list @list have been done.
  */
-void async_synchronize_full_special(struct list_head *list)
+void async_synchronize_full_domain(struct list_head *list)
 {
-	async_synchronize_cookie_special(next_cookie, list);
+	async_synchronize_cookie_domain(next_cookie, list);
 }
-EXPORT_SYMBOL_GPL(async_synchronize_full_special);
+EXPORT_SYMBOL_GPL(async_synchronize_full_domain);
 
 /**
- * async_synchronize_cookie_special - synchronize asynchronous function calls on a running list with cookie checkpointing
+ * async_synchronize_cookie_domain - synchronize asynchronous function calls within a certain domain with cookie checkpointing
  * @cookie: async_cookie_t to use as checkpoint
  * @running: running list to synchronize on
  *
- * This function waits until all asynchronous function calls for the running
- * list @list submitted prior to @cookie have been done.
+ * This function waits until all asynchronous function calls for the
+ * synchronization domain specified by the running list @list submitted
+ * prior to @cookie have been done.
  */
-void async_synchronize_cookie_special(async_cookie_t cookie, struct list_head *running)
+void async_synchronize_cookie_domain(async_cookie_t cookie,
+				     struct list_head *running)
 {
 	ktime_t starttime, delta, endtime;
 
@@ -291,7 +294,7 @@ void async_synchronize_cookie_special(as
 			task_pid_nr(current), ktime_to_ns(delta) >> 10);
 	}
 }
-EXPORT_SYMBOL_GPL(async_synchronize_cookie_special);
+EXPORT_SYMBOL_GPL(async_synchronize_cookie_domain);
 
 /**
  * async_synchronize_cookie - synchronize asynchronous function calls with cookie checkpointing
@@ -302,7 +305,7 @@ EXPORT_SYMBOL_GPL(async_synchronize_cook
  */
 void async_synchronize_cookie(async_cookie_t cookie)
 {
-	async_synchronize_cookie_special(cookie, &async_running);
+	async_synchronize_cookie_domain(cookie, &async_running);
 }
 EXPORT_SYMBOL_GPL(async_synchronize_cookie);
 
--- linux-2.6.orig/fs/super.c
+++ linux-2.6/fs/super.c
@@ -301,7 +301,7 @@ void generic_shutdown_super(struct super
 		/*
 		 * wait for asynchronous fs operations to finish before going further
 		 */
-		async_synchronize_full_special(&sb->s_async_list);
+		async_synchronize_full_domain(&sb->s_async_list);
 
 		/* bad name - it should be evict_inodes() */
 		invalidate_inodes(sb);
@@ -470,7 +470,7 @@ restart:
 		sb->s_count++;
 		spin_unlock(&sb_lock);
 		down_read(&sb->s_umount);
-		async_synchronize_full_special(&sb->s_async_list);
+		async_synchronize_full_domain(&sb->s_async_list);
 		if (sb->s_root && (wait || sb->s_dirt))
 			sb->s_op->sync_fs(sb, wait);
 		up_read(&sb->s_umount);