Re: [PATCH v2 1/4] ida: Add ida_get_lowest()

[Jason Gunthorpe <jgg@nvidia.com>]

On Fri, Apr 19, 2024 at 07:55:04AM -0600, Alex Williamson wrote:
> On Fri, 19 Apr 2024 21:43:17 +0800
> Yi Liu <yi.l.liu@intel.com> wrote:
> 
> > On 2024/4/19 01:12, Jason Gunthorpe wrote:
> > > On Thu, Apr 18, 2024 at 10:23:14AM -0600, Alex Williamson wrote:  
> > >>> yep. maybe we can start with the below code, no need for ida_for_each()
> > >>> today.
> > >>>
> > >>>
> > >>>    	int id = 0;
> > >>>
> > >>>    	while (!ida_is_empty(&pasid_ida)) {
> > >>>    		id = ida_find_first_range(pasid_ida, id, INT_MAX);  
> > >>
> > >> You've actually already justified the _min function here:
> > >>
> > >> static inline int ida_find_first_min(struct ida *ida, unsigned int min)
> > >> {
> > >> 	return ida_find_first_range(ida, min, ~0);
> > >> }  
> > > 
> > > It should also always start from 0..  
> > 
> > any special reason to always start from 0? Here we want to loop all the
> > IDs, and remove them. In this usage, it should be more efficient if we
> > start from the last found ID.
> 
> In the above version, there's a possibility of an infinite loop, in the
> below there's not.  I don't think the infinite loop is actually
> reachable, but given the xarray backend to ida I'm not sure you're
> gaining much to restart after the previously found id either.  Thanks,

Right, there is no performance win on xarray and it only risks an
infinite loop compared to:

> > > while ((id = ida_find_first(pasid_ida)) != EMPTY_IDA) {
> > >    ida_remove(id);
> > > }  

Which does not by construction

Jason