headers
Kevin Diggs | 20 Aug 03:27

{NOT a PATCH} Corrections please ...

Hi,

	It was recommended that I use a completion in a driver I am working on. 
While figuring out how to use one, I noticed that there was no kernel 
doc block comments. I am trying to add them. I would rather not have to 
respin the patch for corrections.

--- include/linux/completion.h.orig	2008-08-13 00:56:52.000000000 -0700
+++ include/linux/completion.h	2008-08-19 17:47:49.000000000 -0700
@@ -10,6 +10,14 @@

  #include <linux/wait.h>

+/**
+ * struct completion - structure used to maintain state for a "completion"
+ *
+ * This is the opaque structure used to maintain the state for a 
"completion".
+ * See also:  complete(), wait_for_completion() (and friends _timeout,
+ * _interruptible, _interruptible_timeout, and _killable), 
init_completion(),
+ * and macros DECLARE_COMPLETION() and INIT_COMPLETION().
+ */
  struct completion {
  	unsigned int done;
  	wait_queue_head_t wait;
@@ -21,6 +29,14 @@
  #define COMPLETION_INITIALIZER_ONSTACK(work) \
  	({ init_completion(&work); work; })
(Continue reading)

Permalink | Reply |
headers
Dave Chinner | 20 Aug 03:52
Favicon

Re: {NOT a PATCH} Corrections please ...

On Tue, Aug 19, 2008 at 06:30:11PM -0700, Kevin Diggs wrote:
> Hi,
>
> 	It was recommended that I use a completion in a driver I am working on.  
> While figuring out how to use one, I noticed that there was no kernel  
> doc block comments. I am trying to add them. I would rather not have to  
> respin the patch for corrections.

Rather than documenting exactly how the queuing and wakeup occurs on
all functions, you should document it once. i.e. that completions
currently use FIFO queuing. It is probably best to do this at the
definition of the struct completion.

The reason is that if the implementation changes (e.g. to support
priorities and inheritence) the comments are then incorrect and
then there's lots of comments to remove^Wchange.

Cheers,

Dave.
--

-- 
Dave Chinner
david <at> fromorbit.com
Permalink | Reply |
headers
Ingo Molnar | 20 Aug 11:59
Favicon

Re: {NOT a PATCH} Corrections please ...


* Kevin Diggs <kevdig <at> hypersurf.com> wrote:

> Hi,
>
> 	It was recommended that I use a completion in a driver I am 
> working on.  While figuring out how to use one, I noticed that there 
> was no kernel doc block comments. I am trying to add them. I would 
> rather not have to respin the patch for corrections.

the text looks good to me - but many of the lines are line-wrapped 
(longer than 80 chars) - which should be avoided for free-flowing text.

	Ingo
Permalink | Reply |
headers
Stefan Richter | 20 Aug 12:57

Re: {NOT a PATCH} Corrections please ...

> * Kevin Diggs <kevdig <at> hypersurf.com> wrote:
>> 	It was recommended that I use a completion in a driver I am 
>> working on.  While figuring out how to use one,

Kevin, as a side note:  LDD3 chapter 5 pages 114 ff has some guidance.
http://lwn.net/Kernel/LDD3/
I'm only pointing to it for the unlikely case that you didn't come
across LDD3 yet.  The dead tree version is affordable too, and more
comfortable to read than the online version.
--

-- 
Stefan Richter
-=====-==--- =--- =-=--
http://arcgraph.de/sr/
Permalink | Reply |
headers
Kevin Diggs | 20 Aug 21:59

Re: {NOT a PATCH} Corrections please ...

Stefan Richter wrote:
>>* Kevin Diggs <kevdig <at> hypersurf.com> wrote:
>>
>>>	It was recommended that I use a completion in a driver I am 
>>>working on.  While figuring out how to use one,
> 
> 
> Kevin, as a side note:  LDD3 chapter 5 pages 114 ff has some guidance.
> http://lwn.net/Kernel/LDD3/
> I'm only pointing to it for the unlikely case that you didn't come
> across LDD3 yet.  The dead tree version is affordable too, and more
> comfortable to read than the online version.

Oh! I have that! That is how I figured out that complete() is used to
signal that the wait thingies don't have to wait anymore.

Here is the latest (maybe line wrap is set to 99 so it won't be goofed
anymore?):

--- include/linux/completion.h.orig	2008-08-13 00:56:52.000000000 -0700
+++ include/linux/completion.h	2008-08-20 01:47:21.000000000 -0700
@@ -10,6 +10,18 @@

  #include <linux/wait.h>

+/**
+ * struct completion - structure used to maintain state for a "completion"
+ *
+ * This is the opaque structure used to maintain the state for a "completion".
+ * Completions currently use a FIFO to queue threads that have to wait for
(Continue reading)

Permalink | Reply |
headers
Ingo Molnar | 21 Aug 12:30
Favicon

Re: {NOT a PATCH} Corrections please ...


* Kevin Diggs <kevdig <at> hypersurf.com> wrote:

> Here is the latest (maybe line wrap is set to 99 so it won't be goofed 
> anymore?):

the patch wont apply as it has whitespace corruption (double spaces). 
Please check Documentation/email-clients.txt.

	Ingo
Permalink | Reply |
headers
Kevin Diggs | 21 Aug 21:47

Re: {NOT a PATCH} Corrections please ...

Ingo Molnar wrote:
> * Kevin Diggs <kevdig <at> hypersurf.com> wrote:
> 
> 
>>Here is the latest (maybe line wrap is set to 99 so it won't be goofed 
>>anymore?):
> 
> 
> the patch wont apply as it has whitespace corruption (double spaces). 
> Please check Documentation/email-clients.txt.
> 
> 	Ingo
> 
> 
The double spacing between the diffs? That will prevent it from applying?

I'm still just trying to get the content correct.

Who should this patch be sent to when it is ready? Also, I think I remember
reading in submitting patches that patches should be cced to the general
linux-kernel list. This correct?

kevin
Permalink | Reply |
headers
Ingo Molnar | 22 Aug 09:02
Favicon

Re: {NOT a PATCH} Corrections please ...


* Kevin Diggs <kevdig <at> hypersurf.com> wrote:

> Ingo Molnar wrote:
>> * Kevin Diggs <kevdig <at> hypersurf.com> wrote:
>>
>>
>>> Here is the latest (maybe line wrap is set to 99 so it won't be 
>>> goofed anymore?):
>>
>>
>> the patch wont apply as it has whitespace corruption (double spaces).  
>> Please check Documentation/email-clients.txt.
>>
>> 	Ingo
>>
>>
> The double spacing between the diffs? That will prevent it from 
> applying?

No. It's a per line corruption. Instead of:

"+<space>code"

your mail had:

"+<space><space>code"

i usually fix that up for small patch but this wasnt a smaller patch :)
(Continue reading)

Permalink | Reply |

Gmane