* [PATCH] cpumask: add a few comments of cpumask functions @ 2012-05-28 9:02 Alex Shi 2012-05-28 12:47 ` Srivatsa S. Bhat 0 siblings, 1 reply; 15+ messages in thread From: Alex Shi @ 2012-05-28 9:02 UTC (permalink / raw) To: alex.shi, rusty Cc: akpm, paul.gortmaker, kosaki.motohiro, srivatsa.bhat, linux-kernel 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] cpumask: add a few comments of cpumask functions 2012-05-28 9:02 [PATCH] cpumask: add a few comments of cpumask functions Alex Shi @ 2012-05-28 12:47 ` Srivatsa S. Bhat 2012-05-28 13:49 ` Alex Shi 0 siblings, 1 reply; 15+ messages in thread From: Srivatsa S. Bhat @ 2012-05-28 12:47 UTC (permalink / raw) To: Alex Shi; +Cc: rusty, akpm, paul.gortmaker, kosaki.motohiro, linux-kernel 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] cpumask: add a few comments of cpumask functions 2012-05-28 12:47 ` Srivatsa S. Bhat @ 2012-05-28 13:49 ` Alex Shi 2012-05-28 14:23 ` Alex Shi ` (2 more replies) 0 siblings, 3 replies; 15+ messages in thread From: Alex Shi @ 2012-05-28 13:49 UTC (permalink / raw) To: Srivatsa S. Bhat Cc: rusty, akpm, paul.gortmaker, kosaki.motohiro, linux-kernel 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] cpumask: add a few comments of cpumask functions 2012-05-28 13:49 ` Alex Shi @ 2012-05-28 14:23 ` Alex Shi 2012-07-13 22:37 ` Andrew Morton 2012-05-28 15:25 ` Srivatsa S. Bhat 2012-06-04 1:36 ` Rusty Russell 2 siblings, 1 reply; 15+ messages in thread From: Alex Shi @ 2012-05-28 14:23 UTC (permalink / raw) To: Srivatsa S. Bhat Cc: rusty, akpm, paul.gortmaker, kosaki.motohiro, linux-kernel > > 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] cpumask: add a few comments of cpumask functions 2012-05-28 14:23 ` Alex Shi @ 2012-07-13 22:37 ` Andrew Morton 2012-07-16 2:34 ` Alex Shi 0 siblings, 1 reply; 15+ messages in thread From: Andrew Morton @ 2012-07-13 22:37 UTC (permalink / raw) To: Alex Shi Cc: Srivatsa S. Bhat, rusty, paul.gortmaker, kosaki.motohiro, linux-kernel 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] cpumask: add a few comments of cpumask functions 2012-07-13 22:37 ` Andrew Morton @ 2012-07-16 2:34 ` Alex Shi 2012-07-16 7:35 ` Rusty Russell 0 siblings, 1 reply; 15+ messages in thread From: Alex Shi @ 2012-07-16 2:34 UTC (permalink / raw) To: Andrew Morton Cc: Srivatsa S. Bhat, rusty, paul.gortmaker, kosaki.motohiro, linux-kernel > > 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] cpumask: add a few comments of cpumask functions 2012-07-16 2:34 ` Alex Shi @ 2012-07-16 7:35 ` Rusty Russell 2012-07-16 8:01 ` Alex Shi 0 siblings, 1 reply; 15+ messages in thread From: Rusty Russell @ 2012-07-16 7:35 UTC (permalink / raw) To: Alex Shi, Andrew Morton Cc: Srivatsa S. Bhat, paul.gortmaker, kosaki.motohiro, linux-kernel 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] cpumask: add a few comments of cpumask functions 2012-07-16 7:35 ` Rusty Russell @ 2012-07-16 8:01 ` Alex Shi 0 siblings, 0 replies; 15+ messages in thread From: Alex Shi @ 2012-07-16 8:01 UTC (permalink / raw) To: Rusty Russell Cc: Andrew Morton, Srivatsa S. Bhat, paul.gortmaker, kosaki.motohiro, linux-kernel 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] cpumask: add a few comments of cpumask functions 2012-05-28 13:49 ` Alex Shi 2012-05-28 14:23 ` Alex Shi @ 2012-05-28 15:25 ` Srivatsa S. Bhat 2012-05-29 1:16 ` Alex Shi 2012-06-04 1:36 ` Rusty Russell 2 siblings, 1 reply; 15+ messages in thread From: Srivatsa S. Bhat @ 2012-05-28 15:25 UTC (permalink / raw) To: Alex Shi; +Cc: rusty, akpm, paul.gortmaker, kosaki.motohiro, linux-kernel 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] cpumask: add a few comments of cpumask functions 2012-05-28 15:25 ` Srivatsa S. Bhat @ 2012-05-29 1:16 ` Alex Shi 2012-05-30 10:40 ` Alex Shi 0 siblings, 1 reply; 15+ messages in thread From: Alex Shi @ 2012-05-29 1:16 UTC (permalink / raw) To: Srivatsa S. Bhat Cc: rusty, akpm, paul.gortmaker, kosaki.motohiro, linux-kernel >>> 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] cpumask: add a few comments of cpumask functions 2012-05-29 1:16 ` Alex Shi @ 2012-05-30 10:40 ` Alex Shi 2012-05-30 10:49 ` KOSAKI Motohiro 0 siblings, 1 reply; 15+ messages in thread From: Alex Shi @ 2012-05-30 10:40 UTC (permalink / raw) To: Srivatsa S. Bhat Cc: rusty, akpm, paul.gortmaker, kosaki.motohiro, linux-kernel 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) ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] cpumask: add a few comments of cpumask functions 2012-05-30 10:40 ` Alex Shi @ 2012-05-30 10:49 ` KOSAKI Motohiro 2012-05-31 8:15 ` Alex Shi 0 siblings, 1 reply; 15+ messages in thread From: KOSAKI Motohiro @ 2012-05-30 10:49 UTC (permalink / raw) To: Alex Shi; +Cc: Srivatsa S. Bhat, rusty, akpm, paul.gortmaker, linux-kernel 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> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] cpumask: add a few comments of cpumask functions 2012-05-30 10:49 ` KOSAKI Motohiro @ 2012-05-31 8:15 ` Alex Shi 2012-06-04 1:37 ` Rusty Russell 0 siblings, 1 reply; 15+ messages in thread From: Alex Shi @ 2012-05-31 8:15 UTC (permalink / raw) To: KOSAKI Motohiro Cc: Srivatsa S. Bhat, rusty, akpm, paul.gortmaker, linux-kernel 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? :) ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] cpumask: add a few comments of cpumask functions 2012-05-31 8:15 ` Alex Shi @ 2012-06-04 1:37 ` Rusty Russell 0 siblings, 0 replies; 15+ messages in thread From: Rusty Russell @ 2012-06-04 1:37 UTC (permalink / raw) To: Alex Shi, KOSAKI Motohiro Cc: Srivatsa S. Bhat, akpm, paul.gortmaker, linux-kernel 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] cpumask: add a few comments of cpumask functions 2012-05-28 13:49 ` Alex Shi 2012-05-28 14:23 ` Alex Shi 2012-05-28 15:25 ` Srivatsa S. Bhat @ 2012-06-04 1:36 ` Rusty Russell 2 siblings, 0 replies; 15+ messages in thread From: Rusty Russell @ 2012-06-04 1:36 UTC (permalink / raw) To: Alex Shi, Srivatsa S. Bhat Cc: akpm, paul.gortmaker, kosaki.motohiro, linux-kernel 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2012-07-16 8:01 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-05-28 9:02 [PATCH] cpumask: add a few comments of cpumask functions Alex Shi 2012-05-28 12:47 ` Srivatsa S. Bhat 2012-05-28 13:49 ` Alex Shi 2012-05-28 14:23 ` Alex Shi 2012-07-13 22:37 ` Andrew Morton 2012-07-16 2:34 ` Alex Shi 2012-07-16 7:35 ` Rusty Russell 2012-07-16 8:01 ` Alex Shi 2012-05-28 15:25 ` Srivatsa S. Bhat 2012-05-29 1:16 ` Alex Shi 2012-05-30 10:40 ` Alex Shi 2012-05-30 10:49 ` KOSAKI Motohiro 2012-05-31 8:15 ` Alex Shi 2012-06-04 1:37 ` Rusty Russell 2012-06-04 1:36 ` Rusty Russell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).