[PATCH] cpumask: add a few comments of cpumask functions

[PATCH] cpumask: add a few comments of cpumask functions

[Alex Shi]

Current few cpumask function purpose are not quite clear. Stupid
user like myself need to dig into details for clear function
purpose and return value.
Add few explanation for them is helpful.

Signed-off-by: Alex Shi <alex.shi@intel.com>
---
 include/linux/cpumask.h |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index a2c819d..8436e61 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -271,6 +271,7 @@ static inline void cpumask_clear_cpu(int cpu, struct cpumask *dstp)
  * cpumask_test_cpu - test for a cpu in a cpumask
  * @cpu: cpu number (< nr_cpu_ids)
  * @cpumask: the cpumask pointer
+ * return 1 if the 'cpu' is in the old 'cpumask', otherwise return 0
  *
  * No static inline type checking - see Subtlety (1) above.
  */
@@ -281,6 +282,7 @@ static inline void cpumask_clear_cpu(int cpu, struct cpumask *dstp)
  * cpumask_test_and_set_cpu - atomically test and set a cpu in a cpumask
  * @cpu: cpu number (< nr_cpu_ids)
  * @cpumask: the cpumask pointer
+ * return 1 if the 'cpu' is in the old 'cpumask', otherwise return 0
  *
  * test_and_set_bit wrapper for cpumasks.
  */
@@ -293,6 +295,7 @@ static inline int cpumask_test_and_set_cpu(int cpu, struct cpumask *cpumask)
  * cpumask_test_and_clear_cpu - atomically test and clear a cpu in a cpumask
  * @cpu: cpu number (< nr_cpu_ids)
  * @cpumask: the cpumask pointer
+ * return 1 if the 'cpu' is in the old 'cpumask', otherwise return 0
  *
  * test_and_clear_bit wrapper for cpumasks.
  */
@@ -324,6 +327,7 @@ static inline void cpumask_clear(struct cpumask *dstp)
  * @dstp: the cpumask result
  * @src1p: the first input
  * @src2p: the second input
+ * if *dstp is empty, return 0, otherwise return 1
  */
 static inline int cpumask_and(struct cpumask *dstp,
 			       const struct cpumask *src1p,
@@ -365,6 +369,7 @@ static inline void cpumask_xor(struct cpumask *dstp,
  * @dstp: the cpumask result
  * @src1p: the first input
  * @src2p: the second input
+ * if *dstp is empty, return 0, otherwise return 1
  */
 static inline int cpumask_andnot(struct cpumask *dstp,
 				  const struct cpumask *src1p,
@@ -414,6 +419,7 @@ static inline bool cpumask_intersects(const struct cpumask *src1p,
  * cpumask_subset - (*src1p & ~*src2p) == 0
  * @src1p: the first input
  * @src2p: the second input
+ * return 1 if the *src1p is the subset of *src2p, otherwise return 0
  */
 static inline int cpumask_subset(const struct cpumask *src1p,
 				 const struct cpumask *src2p)
-- 
1.7.5.4

Re: [PATCH] cpumask: add a few comments of cpumask functions

[Srivatsa S. Bhat]

On 05/28/2012 02:32 PM, Alex Shi wrote:

> Current few cpumask function purpose are not quite clear. Stupid
> user like myself need to dig into details for clear function
> purpose and return value.


You can just see how it is used elsewhere and figure it out ;-)
Anyway, in principle, I don't have any objections to adding comments
that are actually helpful. But I don't think all the comments this
patch adds fall into that category..

> Add few explanation for them is helpful.
> 
> Signed-off-by: Alex Shi <alex.shi@intel.com>
> ---
>  include/linux/cpumask.h |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
> index a2c819d..8436e61 100644
> --- a/include/linux/cpumask.h
> +++ b/include/linux/cpumask.h
> @@ -271,6 +271,7 @@ static inline void cpumask_clear_cpu(int cpu, struct cpumask *dstp)
>   * cpumask_test_cpu - test for a cpu in a cpumask
>   * @cpu: cpu number (< nr_cpu_ids)
>   * @cpumask: the cpumask pointer
> + * return 1 if the 'cpu' is in the old 'cpumask', otherwise return 0
>   *


s/return/Returns

What do you mean by "old" cpumask?

>   * No static inline type checking - see Subtlety (1) above.
>   */
> @@ -281,6 +282,7 @@ static inline void cpumask_clear_cpu(int cpu, struct cpumask *dstp)
>   * cpumask_test_and_set_cpu - atomically test and set a cpu in a cpumask
>   * @cpu: cpu number (< nr_cpu_ids)
>   * @cpumask: the cpumask pointer
> + * return 1 if the 'cpu' is in the old 'cpumask', otherwise return 0
>   *


"Test and Set" already has a very well-defined and well-known meaning.. Not sure if the
comment adds any extra value..

>   * test_and_set_bit wrapper for cpumasks.
>   */
> @@ -293,6 +295,7 @@ static inline int cpumask_test_and_set_cpu(int cpu, struct cpumask *cpumask)
>   * cpumask_test_and_clear_cpu - atomically test and clear a cpu in a cpumask
>   * @cpu: cpu number (< nr_cpu_ids)
>   * @cpumask: the cpumask pointer
> + * return 1 if the 'cpu' is in the old 'cpumask', otherwise return 0
>   *


Same here. Pretty well known meaning.

>   * test_and_clear_bit wrapper for cpumasks.
>   */
> @@ -324,6 +327,7 @@ static inline void cpumask_clear(struct cpumask *dstp)
>   * @dstp: the cpumask result
>   * @src1p: the first input
>   * @src2p: the second input
> + * if *dstp is empty, return 0, otherwise return 1


I don't think anybody would be interested in the return value of this function!
The functionality (rather than the return value) is what is more interesting
and useful, and is already well-documented.

>   */
>  static inline int cpumask_and(struct cpumask *dstp,
>  			       const struct cpumask *src1p,
> @@ -365,6 +369,7 @@ static inline void cpumask_xor(struct cpumask *dstp,
>   * @dstp: the cpumask result
>   * @src1p: the first input
>   * @src2p: the second input
> + * if *dstp is empty, return 0, otherwise return 1


Same here. Functionality is more interesting.

>   */
>  static inline int cpumask_andnot(struct cpumask *dstp,
>  				  const struct cpumask *src1p,
> @@ -414,6 +419,7 @@ static inline bool cpumask_intersects(const struct cpumask *src1p,
>   * cpumask_subset - (*src1p & ~*src2p) == 0
>   * @src1p: the first input
>   * @src2p: the second input
> + * return 1 if the *src1p is the subset of *src2p, otherwise return 0


s/return/Returns
(I think the function name itself is pretty descriptive, but anyway...)

>   */
>  static inline int cpumask_subset(const struct cpumask *src1p,
>  				 const struct cpumask *src2p)


 
Regards,
Srivatsa S. Bhat

Re: [PATCH] cpumask: add a few comments of cpumask functions

[Alex Shi]

On 05/28/2012 08:47 PM, Srivatsa S. Bhat wrote:

> On 05/28/2012 02:32 PM, Alex Shi wrote:
> 
>> Current few cpumask function purpose are not quite clear. Stupid
>> user like myself need to dig into details for clear function
>> purpose and return value.
> 
> 
> You can just see how it is used elsewhere and figure it out ;-)
> Anyway, in principle, I don't have any objections to adding comments
> that are actually helpful. But I don't think all the comments this
> patch adds fall into that category..
> 
>> Add few explanation for them is helpful.
>>
>> Signed-off-by: Alex Shi <alex.shi@intel.com>
>> ---
>>  include/linux/cpumask.h |    6 ++++++
>>  1 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
>> index a2c819d..8436e61 100644
>> --- a/include/linux/cpumask.h
>> +++ b/include/linux/cpumask.h
>> @@ -271,6 +271,7 @@ static inline void cpumask_clear_cpu(int cpu, struct cpumask *dstp)
>>   * cpumask_test_cpu - test for a cpu in a cpumask
>>   * @cpu: cpu number (< nr_cpu_ids)
>>   * @cpumask: the cpumask pointer
>> + * return 1 if the 'cpu' is in the old 'cpumask', otherwise return 0
>>   *
> 
> 
> s/return/Returns
> 
> What do you mean by "old" cpumask?




Thanks for comments!
Should be "the old bitmap of cpumask"?

> 
>>   * No static inline type checking - see Subtlety (1) above.
>>   */
>> @@ -281,6 +282,7 @@ static inline void cpumask_clear_cpu(int cpu, struct cpumask *dstp)
>>   * cpumask_test_and_set_cpu - atomically test and set a cpu in a cpumask
>>   * @cpu: cpu number (< nr_cpu_ids)
>>   * @cpumask: the cpumask pointer
>> + * return 1 if the 'cpu' is in the old 'cpumask', otherwise return 0
>>   *
> 
> 
> "Test and Set" already has a very well-defined and well-known meaning.. Not sure if the
> comment adds any extra value..




Yes, but it is still helpful for newbie. :)

> 
>>   * test_and_set_bit wrapper for cpumasks.
>>   */
>> @@ -293,6 +295,7 @@ static inline int cpumask_test_and_set_cpu(int cpu, struct cpumask *cpumask)
>>   * cpumask_test_and_clear_cpu - atomically test and clear a cpu in a cpumask
>>   * @cpu: cpu number (< nr_cpu_ids)
>>   * @cpumask: the cpumask pointer
>> + * return 1 if the 'cpu' is in the old 'cpumask', otherwise return 0
>>   *
> 
> 
> Same here. Pretty well known meaning.
> 
>>   * test_and_clear_bit wrapper for cpumasks.
>>   */
>> @@ -324,6 +327,7 @@ static inline void cpumask_clear(struct cpumask *dstp)
>>   * @dstp: the cpumask result
>>   * @src1p: the first input
>>   * @src2p: the second input
>> + * if *dstp is empty, return 0, otherwise return 1
> 
> 
> I don't think anybody would be interested in the return value of this function!
> The functionality (rather than the return value) is what is more interesting
> and useful, and is already well-documented.




:) it is true none is using the return value, but why not to have a comments 
if we have this return value.

> 
>>   */
>>  static inline int cpumask_and(struct cpumask *dstp,
>>  			       const struct cpumask *src1p,
>> @@ -365,6 +369,7 @@ static inline void cpumask_xor(struct cpumask *dstp,
>>   * @dstp: the cpumask result
>>   * @src1p: the first input
>>   * @src2p: the second input
>> + * if *dstp is empty, return 0, otherwise return 1
> 
> 
> Same here. Functionality is more interesting.
> 
>>   */
>>  static inline int cpumask_andnot(struct cpumask *dstp,
>>  				  const struct cpumask *src1p,
>> @@ -414,6 +419,7 @@ static inline bool cpumask_intersects(const struct cpumask *src1p,
>>   * cpumask_subset - (*src1p & ~*src2p) == 0
>>   * @src1p: the first input
>>   * @src2p: the second input
>> + * return 1 if the *src1p is the subset of *src2p, otherwise return 0
> 
> 
> s/return/Returns
> (I think the function name itself is pretty descriptive, but anyway...)




Just this function make me confusing, because I missed the '== 0' in 
"*src1p & ~*src2p) == 0", than I thought scr1p should be the mother mask
 and scr2p is daughter. :)

> 
>>   */
>>  static inline int cpumask_subset(const struct cpumask *src1p,
>>  				 const struct cpumask *src2p)
> 
> 
>  
> Regards,
> Srivatsa S. Bhat



Thanks for all comments, I updated the patch according to your some suggestion.
Anyway, I don't mind if the patch is picked up. Maybe there is only one stupid 
guy in the world. :)
----------

>From caf6e32ec4f335064f067904f24fb9f9f90312df Mon Sep 17 00:00:00 2001
From: Alex Shi <alex.shi@intel.com>
Date: Mon, 28 May 2012 15:53:51 +0800
Subject: [PATCH] cpumask: add a few comments of cpumask functions

Current few cpumask functions' purposes are not quite clear. Stupid
user like myself need to dig into details for clear function
purpose and return value.
Add few explanation for them is helpful.

Signed-off-by: Alex Shi <alex.shi@intel.com>
---
 include/linux/cpumask.h |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index a2c819d..177e5e8 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -271,6 +271,7 @@ static inline void cpumask_clear_cpu(int cpu, struct cpumask *dstp)
  * cpumask_test_cpu - test for a cpu in a cpumask
  * @cpu: cpu number (< nr_cpu_ids)
  * @cpumask: the cpumask pointer
+ * Return 1 if the 'cpu' is in the old bitmap of 'cpumask', otherwise return 0
  *
  * No static inline type checking - see Subtlety (1) above.
  */
@@ -281,6 +282,7 @@ static inline void cpumask_clear_cpu(int cpu, struct cpumask *dstp)
  * cpumask_test_and_set_cpu - atomically test and set a cpu in a cpumask
  * @cpu: cpu number (< nr_cpu_ids)
  * @cpumask: the cpumask pointer
+ * Return 1 if the 'cpu' is in the old bitmap of 'cpumask', otherwise return 0
  *
  * test_and_set_bit wrapper for cpumasks.
  */
@@ -293,6 +295,7 @@ static inline int cpumask_test_and_set_cpu(int cpu, struct cpumask *cpumask)
  * cpumask_test_and_clear_cpu - atomically test and clear a cpu in a cpumask
  * @cpu: cpu number (< nr_cpu_ids)
  * @cpumask: the cpumask pointer
+ * Return 1 if the 'cpu' is in the old bitmap of 'cpumask', otherwise return 0
  *
  * test_and_clear_bit wrapper for cpumasks.
  */
@@ -324,6 +327,7 @@ static inline void cpumask_clear(struct cpumask *dstp)
  * @dstp: the cpumask result
  * @src1p: the first input
  * @src2p: the second input
+ * If *dstp is empty, return 0, otherwise return 1
  */
 static inline int cpumask_and(struct cpumask *dstp,
 			       const struct cpumask *src1p,
@@ -365,6 +369,7 @@ static inline void cpumask_xor(struct cpumask *dstp,
  * @dstp: the cpumask result
  * @src1p: the first input
  * @src2p: the second input
+ * If *dstp is empty, return 0, otherwise return 1
  */
 static inline int cpumask_andnot(struct cpumask *dstp,
 				  const struct cpumask *src1p,
@@ -414,6 +419,7 @@ static inline bool cpumask_intersects(const struct cpumask *src1p,
  * cpumask_subset - (*src1p & ~*src2p) == 0
  * @src1p: the first input
  * @src2p: the second input
+ * Return 1 if the *src1p is the subset of *src2p, otherwise return 0
  */
 static inline int cpumask_subset(const struct cpumask *src1p,
 				 const struct cpumask *src2p)
-- 
1.7.5.4

Re: [PATCH] cpumask: add a few comments of cpumask functions

[Alex Shi]

>

> Thanks for all comments, I updated the patch according to your some suggestion.
> Anyway, I don't mind if the patch is picked up. Maybe there is only one stupid 
> guy in the world. :)
> ----------




Sorry, missed 's' for verb's singular form in third personal.

---
>From e33d4fb4a6730832c08979f2cfdf65795cefd08a Mon Sep 17 00:00:00 2001
From: Alex Shi <alex.shi@intel.com>
Date: Mon, 28 May 2012 15:53:51 +0800
Subject: [PATCH] cpumask: add a few comments of cpumask functions

Current few cpumask functions' purposes are not quite clear. Stupid
user like myself need to dig into details for clear function
purpose and return value.
Add few explanation for them is helpful.

Signed-off-by: Alex Shi <alex.shi@intel.com>
---
 include/linux/cpumask.h |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index a2c819d..7fb07ac 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -271,6 +271,7 @@ static inline void cpumask_clear_cpu(int cpu, struct cpumask *dstp)
  * cpumask_test_cpu - test for a cpu in a cpumask
  * @cpu: cpu number (< nr_cpu_ids)
  * @cpumask: the cpumask pointer
+ * Returns 1 if the 'cpu' is in the old bitmap of 'cpumask', otherwise returns 0
  *
  * No static inline type checking - see Subtlety (1) above.
  */
@@ -281,6 +282,7 @@ static inline void cpumask_clear_cpu(int cpu, struct cpumask *dstp)
  * cpumask_test_and_set_cpu - atomically test and set a cpu in a cpumask
  * @cpu: cpu number (< nr_cpu_ids)
  * @cpumask: the cpumask pointer
+ * Returns 1 if the 'cpu' is in the old bitmap of 'cpumask', otherwise returns 0
  *
  * test_and_set_bit wrapper for cpumasks.
  */
@@ -293,6 +295,7 @@ static inline int cpumask_test_and_set_cpu(int cpu, struct cpumask *cpumask)
  * cpumask_test_and_clear_cpu - atomically test and clear a cpu in a cpumask
  * @cpu: cpu number (< nr_cpu_ids)
  * @cpumask: the cpumask pointer
+ * Returns 1 if the 'cpu' is in the old bitmap of 'cpumask', otherwise returns 0
  *
  * test_and_clear_bit wrapper for cpumasks.
  */
@@ -324,6 +327,7 @@ static inline void cpumask_clear(struct cpumask *dstp)
  * @dstp: the cpumask result
  * @src1p: the first input
  * @src2p: the second input
+ * If *dstp is empty, returns 0, otherwise returns 1
  */
 static inline int cpumask_and(struct cpumask *dstp,
 			       const struct cpumask *src1p,
@@ -365,6 +369,7 @@ static inline void cpumask_xor(struct cpumask *dstp,
  * @dstp: the cpumask result
  * @src1p: the first input
  * @src2p: the second input
+ * If *dstp is empty, returns 0, otherwise returns 1
  */
 static inline int cpumask_andnot(struct cpumask *dstp,
 				  const struct cpumask *src1p,
@@ -414,6 +419,7 @@ static inline bool cpumask_intersects(const struct cpumask *src1p,
  * cpumask_subset - (*src1p & ~*src2p) == 0
  * @src1p: the first input
  * @src2p: the second input
+ * Returns 1 if the *src1p is the subset of *src2p, otherwise returns 0
  */
 static inline int cpumask_subset(const struct cpumask *src1p,
 				 const struct cpumask *src2p)
-- 
1.7.5.4

Re: [PATCH] cpumask: add a few comments of cpumask functions

[Srivatsa S. Bhat]

On 05/28/2012 07:19 PM, Alex Shi wrote:

> On 05/28/2012 08:47 PM, Srivatsa S. Bhat wrote:
> 
>> On 05/28/2012 02:32 PM, Alex Shi wrote:
>>
>>> Current few cpumask function purpose are not quite clear. Stupid
>>> user like myself need to dig into details for clear function
>>> purpose and return value.
>>
>>
>> You can just see how it is used elsewhere and figure it out ;-)
>> Anyway, in principle, I don't have any objections to adding comments
>> that are actually helpful. But I don't think all the comments this
>> patch adds fall into that category..
>>
>>> Add few explanation for them is helpful.
>>>
>>> Signed-off-by: Alex Shi <alex.shi@intel.com>
>>> ---
>>>  include/linux/cpumask.h |    6 ++++++
>>>  1 files changed, 6 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
>>> index a2c819d..8436e61 100644
>>> --- a/include/linux/cpumask.h
>>> +++ b/include/linux/cpumask.h
>>> @@ -271,6 +271,7 @@ static inline void cpumask_clear_cpu(int cpu, struct cpumask *dstp)
>>>   * cpumask_test_cpu - test for a cpu in a cpumask
>>>   * @cpu: cpu number (< nr_cpu_ids)
>>>   * @cpumask: the cpumask pointer
>>> + * return 1 if the 'cpu' is in the old 'cpumask', otherwise return 0
>>>   *
>>
>>
>> s/return/Returns
>>
>> What do you mean by "old" cpumask?
> 
> Thanks for comments!
> Should be "the old bitmap of cpumask"?
> 


No, there is no "old" or "new" cpumask here because this function doesn't
change the cpumask. It just checks if that bit is set.
So something like:
Returns 1 if 'cpu' is set in 'cpumask', else returns 0.

Regards,
Srivatsa S. Bhat

Re: [PATCH] cpumask: add a few comments of cpumask functions

[Alex Shi]

>>> s/return/Returns
>>>
>>> What do you mean by "old" cpumask?
>>
>> Thanks for comments!
>> Should be "the old bitmap of cpumask"?
>>
> 
> 
> No, there is no "old" or "new" cpumask here because this function doesn't
> change the cpumask. It just checks if that bit is set.
> So something like:
> Returns 1 if 'cpu' is set in 'cpumask', else returns 0.
> 



Sure, thanks! and comments changed again following your style.
Is the following new update patch ok?


>From 1daa52caf1ba83eea0975a9250becd85dd5a5315 Mon Sep 17 00:00:00 2001
From: Alex Shi <alex.shi@intel.com>
Date: Mon, 28 May 2012 22:23:51 +0800
Subject: [PATCH] cpumask: add a few comments of cpumask functions

Current few cpumask functions' purposes are not quite clear. Stupid
user like myself needs to dig into details for clear function
purpose and return value.
Add few explanation for them is helpful.

Thanks for Srivatsa's comments and correction!

Signed-off-by: Alex Shi <alex.shi@intel.com>
---
 include/linux/cpumask.h |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index a2c819d..170c5af 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -271,6 +271,7 @@ static inline void cpumask_clear_cpu(int cpu, struct cpumask *dstp)
  * cpumask_test_cpu - test for a cpu in a cpumask
  * @cpu: cpu number (< nr_cpu_ids)
  * @cpumask: the cpumask pointer
+ * Returns 1 if 'cpu' is set in 'cpumask', else returns 0
  *
  * No static inline type checking - see Subtlety (1) above.
  */
@@ -281,6 +282,7 @@ static inline void cpumask_clear_cpu(int cpu, struct cpumask *dstp)
  * cpumask_test_and_set_cpu - atomically test and set a cpu in a cpumask
  * @cpu: cpu number (< nr_cpu_ids)
  * @cpumask: the cpumask pointer
+ * Returns 1 if 'cpu' is set in old bitmap of 'cpumask', else returns 0
  *
  * test_and_set_bit wrapper for cpumasks.
  */
@@ -293,6 +295,7 @@ static inline int cpumask_test_and_set_cpu(int cpu, struct cpumask *cpumask)
  * cpumask_test_and_clear_cpu - atomically test and clear a cpu in a cpumask
  * @cpu: cpu number (< nr_cpu_ids)
  * @cpumask: the cpumask pointer
+ * Returns 1 if 'cpu' is set in old bitmap of 'cpumask', else returns 0
  *
  * test_and_clear_bit wrapper for cpumasks.
  */
@@ -324,6 +327,7 @@ static inline void cpumask_clear(struct cpumask *dstp)
  * @dstp: the cpumask result
  * @src1p: the first input
  * @src2p: the second input
+ * If *dstp is empty, returns 0, else returns 1
  */
 static inline int cpumask_and(struct cpumask *dstp,
 			       const struct cpumask *src1p,
@@ -365,6 +369,7 @@ static inline void cpumask_xor(struct cpumask *dstp,
  * @dstp: the cpumask result
  * @src1p: the first input
  * @src2p: the second input
+ * If *dstp is empty, returns 0, else returns 1
  */
 static inline int cpumask_andnot(struct cpumask *dstp,
 				  const struct cpumask *src1p,
@@ -414,6 +419,7 @@ static inline bool cpumask_intersects(const struct cpumask *src1p,
  * cpumask_subset - (*src1p & ~*src2p) == 0
  * @src1p: the first input
  * @src2p: the second input
+ * Returns 1 if *src1p is a subset of *src2p, else returns 0
  */
 static inline int cpumask_subset(const struct cpumask *src1p,
 				 const struct cpumask *src2p)
-- 
1.7.5.4

Re: [PATCH] cpumask: add a few comments of cpumask functions

[Alex Shi]

On 05/29/2012 09:16 AM, Alex Shi wrote:

> 
>>>> s/return/Returns
>>>>
>>>> What do you mean by "old" cpumask?
>>>
>>> Thanks for comments!
>>> Should be "the old bitmap of cpumask"?
>>>
>>
>>
>> No, there is no "old" or "new" cpumask here because this function doesn't
>> change the cpumask. It just checks if that bit is set.
>> So something like:
>> Returns 1 if 'cpu' is set in 'cpumask', else returns 0.
>>
> 
> 
> 
> Sure, thanks! and comments changed again following your style.
> Is the following new update patch ok?


Any further comments for this patch? :)

> 
> 
> From 1daa52caf1ba83eea0975a9250becd85dd5a5315 Mon Sep 17 00:00:00 2001
> From: Alex Shi <alex.shi@intel.com>
> Date: Mon, 28 May 2012 22:23:51 +0800
> Subject: [PATCH] cpumask: add a few comments of cpumask functions
> 
> Current few cpumask functions' purposes are not quite clear. Stupid
> user like myself needs to dig into details for clear function
> purpose and return value.
> Add few explanation for them is helpful.
> 
> Thanks for Srivatsa's comments and correction!
> 
> Signed-off-by: Alex Shi <alex.shi@intel.com>
> ---
>  include/linux/cpumask.h |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
> index a2c819d..170c5af 100644
> --- a/include/linux/cpumask.h
> +++ b/include/linux/cpumask.h
> @@ -271,6 +271,7 @@ static inline void cpumask_clear_cpu(int cpu, struct cpumask *dstp)
>   * cpumask_test_cpu - test for a cpu in a cpumask
>   * @cpu: cpu number (< nr_cpu_ids)
>   * @cpumask: the cpumask pointer
> + * Returns 1 if 'cpu' is set in 'cpumask', else returns 0
>   *
>   * No static inline type checking - see Subtlety (1) above.
>   */
> @@ -281,6 +282,7 @@ static inline void cpumask_clear_cpu(int cpu, struct cpumask *dstp)
>   * cpumask_test_and_set_cpu - atomically test and set a cpu in a cpumask
>   * @cpu: cpu number (< nr_cpu_ids)
>   * @cpumask: the cpumask pointer
> + * Returns 1 if 'cpu' is set in old bitmap of 'cpumask', else returns 0
>   *
>   * test_and_set_bit wrapper for cpumasks.
>   */
> @@ -293,6 +295,7 @@ static inline int cpumask_test_and_set_cpu(int cpu, struct cpumask *cpumask)
>   * cpumask_test_and_clear_cpu - atomically test and clear a cpu in a cpumask
>   * @cpu: cpu number (< nr_cpu_ids)
>   * @cpumask: the cpumask pointer
> + * Returns 1 if 'cpu' is set in old bitmap of 'cpumask', else returns 0
>   *
>   * test_and_clear_bit wrapper for cpumasks.
>   */
> @@ -324,6 +327,7 @@ static inline void cpumask_clear(struct cpumask *dstp)
>   * @dstp: the cpumask result
>   * @src1p: the first input
>   * @src2p: the second input
> + * If *dstp is empty, returns 0, else returns 1
>   */
>  static inline int cpumask_and(struct cpumask *dstp,
>  			       const struct cpumask *src1p,
> @@ -365,6 +369,7 @@ static inline void cpumask_xor(struct cpumask *dstp,
>   * @dstp: the cpumask result
>   * @src1p: the first input
>   * @src2p: the second input
> + * If *dstp is empty, returns 0, else returns 1
>   */
>  static inline int cpumask_andnot(struct cpumask *dstp,
>  				  const struct cpumask *src1p,
> @@ -414,6 +419,7 @@ static inline bool cpumask_intersects(const struct cpumask *src1p,
>   * cpumask_subset - (*src1p & ~*src2p) == 0
>   * @src1p: the first input
>   * @src2p: the second input
> + * Returns 1 if *src1p is a subset of *src2p, else returns 0
>   */
>  static inline int cpumask_subset(const struct cpumask *src1p,
>  				 const struct cpumask *src2p)

Re: [PATCH] cpumask: add a few comments of cpumask functions

[KOSAKI Motohiro]

On Wed, May 30, 2012 at 6:40 AM, Alex Shi <alex.shi@intel.com> wrote:
> On 05/29/2012 09:16 AM, Alex Shi wrote:
>
>>
>>>>> s/return/Returns
>>>>>
>>>>> What do you mean by "old" cpumask?
>>>>
>>>> Thanks for comments!
>>>> Should be "the old bitmap of cpumask"?
>>>>
>>>
>>>
>>> No, there is no "old" or "new" cpumask here because this function doesn't
>>> change the cpumask. It just checks if that bit is set.
>>> So something like:
>>> Returns 1 if 'cpu' is set in 'cpumask', else returns 0.
>>>
>>
>>
>>
>> Sure, thanks! and comments changed again following your style.
>> Is the following new update patch ok?
>
>
> Any further comments for this patch? :)

Acked-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>

Re: [PATCH] cpumask: add a few comments of cpumask functions

[Alex Shi]

On 05/30/2012 06:49 PM, KOSAKI Motohiro wrote:

> On Wed, May 30, 2012 at 6:40 AM, Alex Shi <alex.shi@intel.com> wrote:
>> On 05/29/2012 09:16 AM, Alex Shi wrote:
>>
>>>
>>>>>> s/return/Returns
>>>>>>
>>>>>> What do you mean by "old" cpumask?
>>>>>
>>>>> Thanks for comments!
>>>>> Should be "the old bitmap of cpumask"?
>>>>>
>>>>
>>>>
>>>> No, there is no "old" or "new" cpumask here because this function doesn't
>>>> change the cpumask. It just checks if that bit is set.
>>>> So something like:
>>>> Returns 1 if 'cpu' is set in 'cpumask', else returns 0.
>>>>
>>>
>>>
>>>
>>> Sure, thanks! and comments changed again following your style.
>>> Is the following new update patch ok?
>>
>>
>> Any further comments for this patch? :)
> 
> Acked-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>


Thanks!
Does someone like to pick up this patch? :)

Re: [PATCH] cpumask: add a few comments of cpumask functions

[Rusty Russell]

On Mon, 28 May 2012 21:49:43 +0800, Alex Shi <alex.shi@intel.com> wrote:
> On 05/28/2012 08:47 PM, Srivatsa S. Bhat wrote:
> > "Test and Set" already has a very well-defined and well-known meaning.. Not sure if the
> > comment adds any extra value..
> 
> Yes, but it is still helpful for newbie. :)

I wonder if we should just make it clear that you should see
linux/bitmap.h, as these are designed to be exactly synonymous.

Thanks,
Rusty.

Re: [PATCH] cpumask: add a few comments of cpumask functions

[Rusty Russell]

On Thu, 31 May 2012 16:15:25 +0800, Alex Shi <alex.shi@intel.com> wrote:
> On 05/30/2012 06:49 PM, KOSAKI Motohiro wrote:
> 
> > On Wed, May 30, 2012 at 6:40 AM, Alex Shi <alex.shi@intel.com> wrote:
> >> On 05/29/2012 09:16 AM, Alex Shi wrote:
> >>
> >>>
> >>>>>> s/return/Returns
> >>>>>>
> >>>>>> What do you mean by "old" cpumask?
> >>>>>
> >>>>> Thanks for comments!
> >>>>> Should be "the old bitmap of cpumask"?
> >>>>>
> >>>>
> >>>>
> >>>> No, there is no "old" or "new" cpumask here because this function doesn't
> >>>> change the cpumask. It just checks if that bit is set.
> >>>> So something like:
> >>>> Returns 1 if 'cpu' is set in 'cpumask', else returns 0.
> >>>>
> >>>
> >>>
> >>>
> >>> Sure, thanks! and comments changed again following your style.
> >>> Is the following new update patch ok?
> >>
> >>
> >> Any further comments for this patch? :)
> > 
> > Acked-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> 
> 
> Thanks!
> Does someone like to pick up this patch? :)

Applied, thanks!

Cheers,
Rusty.

Re: [PATCH] cpumask: add a few comments of cpumask functions

[Andrew Morton]

On Mon, 28 May 2012 22:23:51 +0800
Alex Shi <alex.shi@intel.com> wrote:

> Current few cpumask functions' purposes are not quite clear. Stupid
> user like myself need to dig into details for clear function
> purpose and return value.
> Add few explanation for them is helpful.
> 

It appears that Rusty has applied at least some of this patch to
linux-next.  Without reading it ;)

> --- a/include/linux/cpumask.h
> +++ b/include/linux/cpumask.h
> @@ -271,6 +271,7 @@ static inline void cpumask_clear_cpu(int cpu, struct cpumask *dstp)
>   * cpumask_test_cpu - test for a cpu in a cpumask
>   * @cpu: cpu number (< nr_cpu_ids)
>   * @cpumask: the cpumask pointer
> + * Returns 1 if the 'cpu' is in the old bitmap of 'cpumask', otherwise returns 0

In kerneldoc we refer to function arguments by prefixing them with a
'@', not by surrounding them with single quotes.  So this should be

* Returns 1 if @cpu is in the old bitmap of @cpumask, otherwise returns 0


And the same applies to the other comments.  So can you please grab the
latest linux-next, prepare a fixup patch and also check that the patch
is complete - not all of your changes have been applied.

Re: [PATCH] cpumask: add a few comments of cpumask functions

[Alex Shi]

>

> And the same applies to the other comments.  So can you please grab the
> latest linux-next, prepare a fixup patch and also check that the patch
> is complete - not all of your changes have been applied.
> 



Thanks for reminder! I just know this rule now. :)

===
>From 7b227fd186f0abc4b5fb1b7b768253d489f1ed56 Mon Sep 17 00:00:00 2001
From: Alex Shi <alex.shi@intel.com>
Date: Mon, 16 Jul 2012 09:52:33 +0800
Subject: [PATCH 1/2] cpumask: fix kernel-doc incompatible comments

commit c4549cd0d77 introduced some kernel-doc incompatible comments
format, that make kernel-doc output mess and looks strange. like
the part of man cpumask_subset, the DESCRIPTION part merged into
ARGUMENTS incorrectly:

ARGUMENTS
       src1p       the first input

       src2p       the second input Returns 1 if *src1p is a subset of *src2p, else returns 0

Signed-off-by: Alex Shi <alex.shi@intel.com>
---
 include/linux/cpumask.h |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index 170c5af..8bf1c27 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -271,7 +271,8 @@ static inline void cpumask_clear_cpu(int cpu, struct cpumask *dstp)
  * cpumask_test_cpu - test for a cpu in a cpumask
  * @cpu: cpu number (< nr_cpu_ids)
  * @cpumask: the cpumask pointer
- * Returns 1 if 'cpu' is set in 'cpumask', else returns 0
+ *
+ * Returns 1 if @cpu is set in @cpumask, else returns 0
  *
  * No static inline type checking - see Subtlety (1) above.
  */
@@ -282,7 +283,8 @@ static inline void cpumask_clear_cpu(int cpu, struct cpumask *dstp)
  * cpumask_test_and_set_cpu - atomically test and set a cpu in a cpumask
  * @cpu: cpu number (< nr_cpu_ids)
  * @cpumask: the cpumask pointer
- * Returns 1 if 'cpu' is set in old bitmap of 'cpumask', else returns 0
+ *
+ * Returns 1 if @cpu is set in old bitmap of @cpumask, else returns 0
  *
  * test_and_set_bit wrapper for cpumasks.
  */
@@ -295,7 +297,8 @@ static inline int cpumask_test_and_set_cpu(int cpu, struct cpumask *cpumask)
  * cpumask_test_and_clear_cpu - atomically test and clear a cpu in a cpumask
  * @cpu: cpu number (< nr_cpu_ids)
  * @cpumask: the cpumask pointer
- * Returns 1 if 'cpu' is set in old bitmap of 'cpumask', else returns 0
+ *
+ * Returns 1 if @cpu is set in old bitmap of @cpumask, else returns 0
  *
  * test_and_clear_bit wrapper for cpumasks.
  */
@@ -327,7 +330,8 @@ static inline void cpumask_clear(struct cpumask *dstp)
  * @dstp: the cpumask result
  * @src1p: the first input
  * @src2p: the second input
- * If *dstp is empty, returns 0, else returns 1
+ *
+ * If *@dstp is empty, returns 0, else returns 1
  */
 static inline int cpumask_and(struct cpumask *dstp,
 			       const struct cpumask *src1p,
@@ -369,7 +373,8 @@ static inline void cpumask_xor(struct cpumask *dstp,
  * @dstp: the cpumask result
  * @src1p: the first input
  * @src2p: the second input
- * If *dstp is empty, returns 0, else returns 1
+ *
+ * If *@dstp is empty, returns 0, else returns 1
  */
 static inline int cpumask_andnot(struct cpumask *dstp,
 				  const struct cpumask *src1p,
@@ -419,7 +424,8 @@ static inline bool cpumask_intersects(const struct cpumask *src1p,
  * cpumask_subset - (*src1p & ~*src2p) == 0
  * @src1p: the first input
  * @src2p: the second input
- * Returns 1 if *src1p is a subset of *src2p, else returns 0
+ *
+ * Returns 1 if *@src1p is a subset of *@src2p, else returns 0
  */
 static inline int cpumask_subset(const struct cpumask *src1p,
 				 const struct cpumask *src2p)
-- 
1.7.5.4

Re: [PATCH] cpumask: add a few comments of cpumask functions

[Rusty Russell]

On Mon, 16 Jul 2012 10:34:46 +0800, Alex Shi <alex.shi@intel.com> wrote:
> >
> 
> > And the same applies to the other comments.  So can you please grab the
> > latest linux-next, prepare a fixup patch and also check that the patch
> > is complete - not all of your changes have been applied.
> > 
> 
> 
> 
> Thanks for reminder! I just know this rule now. :)

Thanks, folded.

As long as it's readable, I don't care.

If it's not all there, did I miss an update?

Cheers,
Rusty.

Re: [PATCH] cpumask: add a few comments of cpumask functions

[Alex Shi]

On 07/16/2012 03:35 PM, Rusty Russell wrote:

> On Mon, 16 Jul 2012 10:34:46 +0800, Alex Shi <alex.shi@intel.com> wrote:
>>>
>>
>>> And the same applies to the other comments.  So can you please grab the
>>> latest linux-next, prepare a fixup patch and also check that the patch
>>> is complete - not all of your changes have been applied.
>>>
>>
>>
>>
>> Thanks for reminder! I just know this rule now. :)
> 
> Thanks, folded.
> 
> As long as it's readable, I don't care.
> 
> If it's not all there, did I miss an update?


No. all contents in my original patch were checked in.

> 
> Cheers,
> Rusty.