mm/nodemask: Correctly describe nodemask operation return types

[PATCH] mm/nodemask: Correctly describe nodemask operation return types

Posted by Joshua Hahn 1 week, 2 days ago

Commit 0dfe54071d7c8 ("nodemask: Fix return values to be unsigned")
changed a number of nodemask operations that used to return int
to returning a bool instead. However, it did not update the comment
block that described these functions, leaving the documentation
incorrect.

Fix the comment block to accurately describe the functions.
Also fix a typo (unsigend --> unsigned), and fix a callsite in
mempolicy.c that did not get updated during the conversion.

No functional changes intended; changes are purely cosmetic.

Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
---
 include/linux/nodemask.h | 18 +++++++++---------
 mm/mempolicy.c           |  2 +-
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/linux/nodemask.h b/include/linux/nodemask.h
index 204c92462f3c0..b842aa5255464 100644
--- a/include/linux/nodemask.h
+++ b/include/linux/nodemask.h
@@ -24,23 +24,23 @@
  * void nodes_setall(mask)		set all bits
  * void nodes_clear(mask)		clear all bits
  * int node_isset(node, mask)		true iff bit 'node' set in mask
- * int node_test_and_set(node, mask)	test and set bit 'node' in mask
+ * bool node_test_and_set(node, mask)	test and set bit 'node' in mask
  *
- * void nodes_and(dst, src1, src2)	dst = src1 & src2  [intersection]
+ * bool nodes_and(dst, src1, src2)	dst = src1 & src2  [intersection]
  * void nodes_or(dst, src1, src2)	dst = src1 | src2  [union]
  * void nodes_xor(dst, src1, src2)	dst = src1 ^ src2
- * void nodes_andnot(dst, src1, src2)	dst = src1 & ~src2
+ * bool nodes_andnot(dst, src1, src2)	dst = src1 & ~src2
  * void nodes_complement(dst, src)	dst = ~src
  *
- * int nodes_equal(mask1, mask2)	Does mask1 == mask2?
- * int nodes_intersects(mask1, mask2)	Do mask1 and mask2 intersect?
- * int nodes_subset(mask1, mask2)	Is mask1 a subset of mask2?
- * int nodes_empty(mask)		Is mask empty (no bits sets)?
- * int nodes_full(mask)			Is mask full (all bits sets)?
+ * bool nodes_equal(mask1, mask2)	Does mask1 == mask2?
+ * bool nodes_intersects(mask1, mask2)	Do mask1 and mask2 intersect?
+ * bool nodes_subset(mask1, mask2)	Is mask1 a subset of mask2?
+ * bool nodes_empty(mask)		Is mask empty (no bits sets)?
+ * bool nodes_full(mask)		Is mask full (all bits sets)?
  * int nodes_weight(mask)		Hamming weight - number of set bits
  *
  * unsigned int first_node(mask)	Number lowest set bit, or MAX_NUMNODES
- * unsigend int next_node(node, mask)	Next node past 'node', or MAX_NUMNODES
+ * unsigned int next_node(node, mask)	Next node past 'node', or MAX_NUMNODES
  * unsigned int next_node_in(node, mask) Next node past 'node', or wrap to first,
  *					or MAX_NUMNODES
  * unsigned int first_unset_node(mask)	First node not set in mask, or
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 4e4421b22b59f..36699fabd3c22 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -2865,7 +2865,7 @@ bool __mpol_equal(struct mempolicy *a, struct mempolicy *b)
 	case MPOL_PREFERRED:
 	case MPOL_PREFERRED_MANY:
 	case MPOL_WEIGHTED_INTERLEAVE:
-		return !!nodes_equal(a->nodes, b->nodes);
+		return nodes_equal(a->nodes, b->nodes);
 	case MPOL_LOCAL:
 		return true;
 	default:
-- 
2.53.0-Meta

Re: [PATCH] mm/nodemask: Correctly describe nodemask operation return types

Posted by SeongJae Park 1 week, 2 days ago

On Fri, 29 May 2026 13:27:54 -0700 Joshua Hahn <joshua.hahnjy@gmail.com> wrote:

> Commit 0dfe54071d7c8 ("nodemask: Fix return values to be unsigned")
> changed a number of nodemask operations that used to return int
> to returning a bool instead. However, it did not update the comment
> block that described these functions, leaving the documentation
> incorrect.
> 
> Fix the comment block to accurately describe the functions.
> Also fix a typo (unsigend --> unsigned), and fix a callsite in
> mempolicy.c that did not get updated during the conversion.

Nice catch, thank you fixing this!

> 
> No functional changes intended; changes are purely cosmetic.
> 
> Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>

Reviewed-by: SeongJae Park <sj@kernel.org>

> ---
>  include/linux/nodemask.h | 18 +++++++++---------
>  mm/mempolicy.c           |  2 +-
>  2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/include/linux/nodemask.h b/include/linux/nodemask.h
> index 204c92462f3c0..b842aa5255464 100644
> --- a/include/linux/nodemask.h
> +++ b/include/linux/nodemask.h
> @@ -24,23 +24,23 @@
>   * void nodes_setall(mask)		set all bits
>   * void nodes_clear(mask)		clear all bits
>   * int node_isset(node, mask)		true iff bit 'node' set in mask
> - * int node_test_and_set(node, mask)	test and set bit 'node' in mask
> + * bool node_test_and_set(node, mask)	test and set bit 'node' in mask
>   *
> - * void nodes_and(dst, src1, src2)	dst = src1 & src2  [intersection]
> + * bool nodes_and(dst, src1, src2)	dst = src1 & src2  [intersection]
>   * void nodes_or(dst, src1, src2)	dst = src1 | src2  [union]
>   * void nodes_xor(dst, src1, src2)	dst = src1 ^ src2
> - * void nodes_andnot(dst, src1, src2)	dst = src1 & ~src2
> + * bool nodes_andnot(dst, src1, src2)	dst = src1 & ~src2
>   * void nodes_complement(dst, src)	dst = ~src
>   *
> - * int nodes_equal(mask1, mask2)	Does mask1 == mask2?
> - * int nodes_intersects(mask1, mask2)	Do mask1 and mask2 intersect?
> - * int nodes_subset(mask1, mask2)	Is mask1 a subset of mask2?
> - * int nodes_empty(mask)		Is mask empty (no bits sets)?
> - * int nodes_full(mask)			Is mask full (all bits sets)?
> + * bool nodes_equal(mask1, mask2)	Does mask1 == mask2?
> + * bool nodes_intersects(mask1, mask2)	Do mask1 and mask2 intersect?
> + * bool nodes_subset(mask1, mask2)	Is mask1 a subset of mask2?
> + * bool nodes_empty(mask)		Is mask empty (no bits sets)?
> + * bool nodes_full(mask)		Is mask full (all bits sets)?
>   * int nodes_weight(mask)		Hamming weight - number of set bits
>   *
>   * unsigned int first_node(mask)	Number lowest set bit, or MAX_NUMNODES
> - * unsigend int next_node(node, mask)	Next node past 'node', or MAX_NUMNODES
> + * unsigned int next_node(node, mask)	Next node past 'node', or MAX_NUMNODES
>   * unsigned int next_node_in(node, mask) Next node past 'node', or wrap to first,
>   *					or MAX_NUMNODES
>   * unsigned int first_unset_node(mask)	First node not set in mask, or

I feel like this comment block might be easy to be outdated in future.  This
comment block is introduced before the git era.  I think this was the best and
great way at that time.  But now we have modern documentation tools like
kernel-doc.  I'm wondering if it makes sense to convert this comment block to
kernel-doc comments in long term, after this patch is merged.  If we sure these
APIs are quite stable and expect no many changes in future, such conversion may
not make sense, though.


Thanks,
SJ

[...]

Re: [PATCH] mm/nodemask: Correctly describe nodemask operation return types

Posted by Joshua Hahn 6 days, 18 hours ago

> On Fri, 29 May 2026 13:27:54 -0700 Joshua Hahn <joshua.hahnjy@gmail.com> wrote:
> 
> > Commit 0dfe54071d7c8 ("nodemask: Fix return values to be unsigned")
> > changed a number of nodemask operations that used to return int
> > to returning a bool instead. However, it did not update the comment
> > block that described these functions, leaving the documentation
> > incorrect.
> > 
> > Fix the comment block to accurately describe the functions.
> > Also fix a typo (unsigend --> unsigned), and fix a callsite in
> > mempolicy.c that did not get updated during the conversion.
> 
> Nice catch, thank you fixing this!
> 
> > 
> > No functional changes intended; changes are purely cosmetic.
> > 
> > Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> 
> Reviewed-by: SeongJae Park <sj@kernel.org>

Thank you, SJ! 

> >   * unsigned int first_node(mask)	Number lowest set bit, or MAX_NUMNODES
> > - * unsigend int next_node(node, mask)	Next node past 'node', or MAX_NUMNODES
> > + * unsigned int next_node(node, mask)	Next node past 'node', or MAX_NUMNODES
> >   * unsigned int next_node_in(node, mask) Next node past 'node', or wrap to first,
> >   *					or MAX_NUMNODES
> >   * unsigned int first_unset_node(mask)	First node not set in mask, or
> 
> I feel like this comment block might be easy to be outdated in future.  This
> comment block is introduced before the git era.  I think this was the best and
> great way at that time.  But now we have modern documentation tools like
> kernel-doc.  I'm wondering if it makes sense to convert this comment block to
> kernel-doc comments in long term, after this patch is merged.  If we sure these
> APIs are quite stable and expect no many changes in future, such conversion may
> not make sense, though.

I agree with this. I'm not entirely too sure if we expect too many changes
in this section, but I could always be surprised : -) I would say that even
in 2022 (when the nodemask function changes happened) not many people would
have expected changes to this otherwise stable set of functions.

So maybe the next changes to the function should also do the migration!

Thanks again, SJ! I hope you have a great day,
Joshua