Re: [PATCH v2 31/36] maple_tree: Add mas_prev_range() and mas_find_range_rev interface

[Liam R. Howlett]

* Vernon Yang <vernon2gm@xxxxxxxxx> [230508 09:27]:
> On Fri, May 05, 2023 at 01:41:59PM -0400, Liam R. Howlett wrote:
> > Some users of the maple tree may want to move to the previous range
> > regardless of the value stored there.  Add this interface as well as the
> > 'find' variant to support walking to the first value, then iterating
> > over the previous ranges.
> >
> > Signed-off-by: Liam R. Howlett <Liam.Howlett@xxxxxxxxxx>
> > ---
> >  include/linux/maple_tree.h |   1 +
> >  lib/maple_tree.c           | 160 +++++++++++++++++++++++++++----------
> >  2 files changed, 121 insertions(+), 40 deletions(-)
> >
> > diff --git a/include/linux/maple_tree.h b/include/linux/maple_tree.h
> > index a4cd8f891a090..542b09118a09f 100644
> > --- a/include/linux/maple_tree.h
> > +++ b/include/linux/maple_tree.h
> > @@ -467,6 +467,7 @@ void mas_destroy(struct ma_state *mas);
> >  int mas_expected_entries(struct ma_state *mas, unsigned long nr_entries);
> >
> >  void *mas_prev(struct ma_state *mas, unsigned long min);
> > +void *mas_prev_range(struct ma_state *mas, unsigned long max);
> >  void *mas_next(struct ma_state *mas, unsigned long max);
> >  void *mas_next_range(struct ma_state *mas, unsigned long max);
> >
> > diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> > index e050fd1f7cce8..f060c71965c0d 100644
> > --- a/lib/maple_tree.c
> > +++ b/lib/maple_tree.c
> > @@ -5924,18 +5924,8 @@ void *mt_next(struct maple_tree *mt, unsigned long index, unsigned long max)
> >  }
> >  EXPORT_SYMBOL_GPL(mt_next);
> >
> > -/**
> > - * mas_prev() - Get the previous entry
> > - * @mas: The maple state
> > - * @min: The minimum value to check.
> > - *
> > - * Must hold rcu_read_lock or the write lock.
> > - * Will reset mas to MAS_START if the node is MAS_NONE.  Will stop on not
> > - * searchable nodes.
> > - *
> > - * Return: the previous value or %NULL.
> > - */
> > -void *mas_prev(struct ma_state *mas, unsigned long min)
> > +static inline bool mas_prev_setup(struct ma_state *mas, unsigned long min,
> > +		void **entry)
> >  {
> >  	if (mas->index <= min)
> >  		goto none;
> > @@ -5953,7 +5943,8 @@ void *mas_prev(struct ma_state *mas, unsigned long min)
> >  		if (!mas->index)
> >  			goto none;
> >  		mas->index = mas->last = 0;
> > -		return mas_root(mas);
> > +		*entry = mas_root(mas);
> > +		return true;
> >  	}
> >
> >  	if (mas_is_none(mas)) {
> > @@ -5961,18 +5952,64 @@ void *mas_prev(struct ma_state *mas, unsigned long min)
> >  			/* Walked to out-of-range pointer? */
> >  			mas->index = mas->last = 0;
> >  			mas->node = MAS_ROOT;
> > -			return mas_root(mas);
> > +			*entry = mas_root(mas);
> > +			return true;
> >  		}
> > -		return NULL;
> > +		return true;
> >  	}
> > -	return mas_prev_slot(mas, min, false);
> > +
> > +	return false;
> >
> >  none:
> >  	mas->node = MAS_NONE;
> > -	return NULL;
> > +	return true;
> > +}
> > +
> > +/**
> > + * mas_prev() - Get the previous entry
> > + * @mas: The maple state
> > + * @min: The minimum value to check.
> > + *
> > + * Must hold rcu_read_lock or the write lock.
> > + * Will reset mas to MAS_START if the node is MAS_NONE.  Will stop on not
> > + * searchable nodes.
> > + *
> > + * Return: the previous value or %NULL.
> > + */
> > +void *mas_prev(struct ma_state *mas, unsigned long min)
> > +{
> > +	void *entry = NULL;
> > +
> > +	if (mas_prev_setup(mas, min, &entry))
> > +		return entry;
> > +
> > +	return mas_prev_slot(mas, min, false);
> >  }
> >  EXPORT_SYMBOL_GPL(mas_prev);
> >
> > +/**
> > + * mas_prev_range() - Advance to the previous range
> > + * @mas: The maple state
> > + * @min: The minimum value to check.
> > + *
> > + * Sets @mas->index and @mas->last to the range.
> > + * Must hold rcu_read_lock or the write lock.
> > + * Will reset mas to MAS_START if the node is MAS_NONE.  Will stop on not
> > + * searchable nodes.
> > + *
> > + * Return: the previous value or %NULL.
> > + */
> > +void *mas_prev_range(struct ma_state *mas, unsigned long min)
> > +{
> > +	void *entry = NULL;
> > +
> > +	if (mas_prev_setup(mas, min, &entry))
> > +		return entry;
> > +
> > +	return mas_prev_slot(mas, min, true);
> > +}
> > +EXPORT_SYMBOL_GPL(mas_prev_slot);
> 
> Hi Liam,
> 
> I guess you want to export mas_prev_range symbol instead of mas_prev_slot.

Yes.. and it mas_prev_slot should be static.

Thanks for catching this.  I noticed this only after the bot complained
about non-static functions in this series.


Regards,
Liam