[PATCH] maple_tree: document that "last" in mtree_insert_range() is inclusive

Steven Rostedt posted 1 patch 1 month, 1 week ago
There is a newer version of this series
lib/maple_tree.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
[PATCH] maple_tree: document that "last" in mtree_insert_range() is inclusive
Posted by Steven Rostedt 1 month, 1 week ago
From: Steven Rostedt <rostedt@goodmis.org>

The kernel doc of mtree_insert_range() does not state if the address
represented by the "last" parameter is inclusive or exclusive. This can
lead to bugs by code that assumes it is exclusive. Explicitly state that
the parameter is inclusive, and add '[' and ']' around the word "end" to
also stress this point.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 lib/maple_tree.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index 60ae5e6fc1ee..dc9591218446 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -5730,10 +5730,12 @@ EXPORT_SYMBOL(mtree_store);
  * mtree_insert_range() - Insert an entry at a given range if there is no value.
  * @mt: The maple tree
  * @first: The start of the range
- * @last: The end of the range
+ * @last: The [end] of the range
  * @entry: The entry to store
  * @gfp: The GFP_FLAGS to use for allocations.
  *
+ * Note that @last is inclusive. That is, @last = @first + length - 1;
+ *
  * Return: 0 on success, -EEXISTS if the range is occupied, -EINVAL on invalid
  * request, -ENOMEM if memory could not be allocated.
  */
-- 
2.53.0
Re: [PATCH] maple_tree: document that "last" in mtree_insert_range() is inclusive
Posted by Alice Ryhl 1 month, 1 week ago
On Wed, May 06, 2026 at 10:52:18AM -0400, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
> 
> The kernel doc of mtree_insert_range() does not state if the address
> represented by the "last" parameter is inclusive or exclusive. This can
> lead to bugs by code that assumes it is exclusive. Explicitly state that
> the parameter is inclusive, and add '[' and ']' around the word "end" to
> also stress this point.
> 
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

>  lib/maple_tree.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> index 60ae5e6fc1ee..dc9591218446 100644
> --- a/lib/maple_tree.c
> +++ b/lib/maple_tree.c
> @@ -5730,10 +5730,12 @@ EXPORT_SYMBOL(mtree_store);
>   * mtree_insert_range() - Insert an entry at a given range if there is no value.
>   * @mt: The maple tree
>   * @first: The start of the range
> - * @last: The end of the range
> + * @last: The [end] of the range
>   * @entry: The entry to store
>   * @gfp: The GFP_FLAGS to use for allocations.
>   *
> + * Note that @last is inclusive. That is, @last = @first + length - 1;

How about writing it like this?

* @first: The start of the range
* @last: The end of the range (inclusive)

Alice
Re: [PATCH] maple_tree: document that "last" in mtree_insert_range() is inclusive
Posted by Liam R. Howlett 1 month ago
On 26/05/07 06:44AM, Alice Ryhl wrote:
> On Wed, May 06, 2026 at 10:52:18AM -0400, Steven Rostedt wrote:
> > From: Steven Rostedt <rostedt@goodmis.org>
> > 
> > The kernel doc of mtree_insert_range() does not state if the address
> > represented by the "last" parameter is inclusive or exclusive. This can
> > lead to bugs by code that assumes it is exclusive. Explicitly state that
> > the parameter is inclusive, and add '[' and ']' around the word "end" to
> > also stress this point.
> > 
> > Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> 
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> 
> >  lib/maple_tree.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> > index 60ae5e6fc1ee..dc9591218446 100644
> > --- a/lib/maple_tree.c
> > +++ b/lib/maple_tree.c
> > @@ -5730,10 +5730,12 @@ EXPORT_SYMBOL(mtree_store);
> >   * mtree_insert_range() - Insert an entry at a given range if there is no value.
> >   * @mt: The maple tree
> >   * @first: The start of the range
> > - * @last: The end of the range
> > + * @last: The [end] of the range
> >   * @entry: The entry to store
> >   * @gfp: The GFP_FLAGS to use for allocations.
> >   *
> > + * Note that @last is inclusive. That is, @last = @first + length - 1;
> 
> How about writing it like this?
> 
> * @first: The start of the range
> * @last: The end of the range (inclusive)


I like this.

I think the range should be identified in the initial statement.
Something like this:

mtree_insert_range() - Insert an entry from [first, last] if there isn't
an entry within that range.

> 
> Alice
> 
> -- 
> maple-tree mailing list
> maple-tree@lists.infradead.org
> https://lists.infradead.org/mailman/listinfo/maple-tree
Re: [PATCH] maple_tree: document that "last" in mtree_insert_range() is inclusive
Posted by SeongJae Park 1 month ago
On Fri, 8 May 2026 22:51:51 +0200 "Liam R. Howlett" <liam@infradead.org> wrote:

> On 26/05/07 06:44AM, Alice Ryhl wrote:
> > On Wed, May 06, 2026 at 10:52:18AM -0400, Steven Rostedt wrote:
> > > From: Steven Rostedt <rostedt@goodmis.org>
> > > 
> > > The kernel doc of mtree_insert_range() does not state if the address
> > > represented by the "last" parameter is inclusive or exclusive. This can
> > > lead to bugs by code that assumes it is exclusive. Explicitly state that
> > > the parameter is inclusive, and add '[' and ']' around the word "end" to
> > > also stress this point.
> > > 
> > > Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> > 
> > Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> > 
> > >  lib/maple_tree.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> > > index 60ae5e6fc1ee..dc9591218446 100644
> > > --- a/lib/maple_tree.c
> > > +++ b/lib/maple_tree.c
> > > @@ -5730,10 +5730,12 @@ EXPORT_SYMBOL(mtree_store);
> > >   * mtree_insert_range() - Insert an entry at a given range if there is no value.
> > >   * @mt: The maple tree
> > >   * @first: The start of the range
> > > - * @last: The end of the range
> > > + * @last: The [end] of the range
> > >   * @entry: The entry to store
> > >   * @gfp: The GFP_FLAGS to use for allocations.
> > >   *
> > > + * Note that @last is inclusive. That is, @last = @first + length - 1;
> > 
> > How about writing it like this?
> > 
> > * @first: The start of the range
> > * @last: The end of the range (inclusive)
> 
> 
> I like this.

+1.  I'm also wondering if it make sense to add '(inclusive)' for 'first', too.

> 
> I think the range should be identified in the initial statement.
> Something like this:
> 
> mtree_insert_range() - Insert an entry from [first, last] if there isn't
> an entry within that range.

+1.


Thanks,
SJ

[...]
Re: [PATCH] maple_tree: document that "last" in mtree_insert_range() is inclusive
Posted by Matthew Wilcox 1 month ago
On Fri, May 08, 2026 at 05:59:46PM -0700, SeongJae Park wrote:
> > > * @first: The start of the range
> > > * @last: The end of the range (inclusive)
> > 
> > 
> > I like this.
> 
> +1.  I'm also wondering if it make sense to add '(inclusive)' for 'first', too.

I can't think of a situation in computing where we use an
exclusive-first.  Pure mathematics, yes, we might want to express a
range as (1,2) to exclude both 1 and 2 but include 1+epsilon for all
epsilon > 0.  Maybe I don't work with floating point numbers enough,
but I've never seen a kernel programmer make an off-by-one with the
start of a range.  End-of-the-range is all too common.
Re: [PATCH] maple_tree: document that "last" in mtree_insert_range() is inclusive
Posted by SeongJae Park 1 month ago
On Sat, 9 May 2026 09:31:13 +0100 Matthew Wilcox <willy@infradead.org> wrote:

> On Fri, May 08, 2026 at 05:59:46PM -0700, SeongJae Park wrote:
> > > > * @first: The start of the range
> > > > * @last: The end of the range (inclusive)
> > > 
> > > 
> > > I like this.
> > 
> > +1.  I'm also wondering if it make sense to add '(inclusive)' for 'first', too.
> 
> I can't think of a situation in computing where we use an
> exclusive-first.  Pure mathematics, yes, we might want to express a
> range as (1,2) to exclude both 1 and 2 but include 1+epsilon for all
> epsilon > 0.  Maybe I don't work with floating point numbers enough,
> but I've never seen a kernel programmer make an off-by-one with the
> start of a range.  End-of-the-range is all too common.

Makes sense, thank you for sharing your thought for my silly question!


Thanks,
SJ