2009-01-13 16:43:58

by Cornelia Huck

[permalink] [raw]
Subject: [PATCH 2/2] async: Add some documentation.

Add some kerneldoc to the async interface.

Signed-off-by: Cornelia Huck <[email protected]>

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


2009-01-14 02:50:48

by Dave Chinner

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

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 <[email protected]>
> +/**
> + * 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
[email protected]

2009-01-14 08:59:54

by Arjan van de Ven

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

On Tue, 13 Jan 2009 17:43:06 +0100
Cornelia Huck <[email protected]> wrote:


Thanks for providing this documentation!

Acked-by: Arjan van de Ven <[email protected]>

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

2009-01-14 10:25:11

by Cornelia Huck

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

On Wed, 14 Jan 2009 13:49:52 +1100,
Dave Chinner <[email protected]> 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 <[email protected]>
> > +/**
> > + * 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.

2009-01-19 00:37:18

by Arjan van de Ven

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

On Wed, 14 Jan 2009 11:24:50 +0100
Cornelia Huck <[email protected]> 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

2009-01-19 04:41:00

by Dave Chinner

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

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 <[email protected]> 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
[email protected]

2009-01-19 12:28:51

by Cornelia Huck

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

On Mon, 19 Jan 2009 15:40:45 +1100,
Dave Chinner <[email protected]> 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 <[email protected]> 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?

2009-01-19 12:50:42

by Arjan van de Ven

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

On Mon, 19 Jan 2009 13:27:44 +0100
Cornelia Huck <[email protected]> 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

2009-01-19 13:10:42

by Cornelia Huck

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

On Mon, 19 Jan 2009 04:52:42 -0800,
Arjan van de Ven <[email protected]> wrote:

> On Mon, 19 Jan 2009 13:27:44 +0100
> Cornelia Huck <[email protected]> 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.

2009-01-20 14:31:44

by Cornelia Huck

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

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 <[email protected]>

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