Hi, > -----Original Message----- > From: Hannes Frederic Sowa [mailto:han...@stressinduktion.org] > Sent: Tuesday, August 29, 2017 3:14 PM > To: Chris Mi <chr...@mellanox.com> > Cc: netdev@vger.kernel.org; j...@mojatatu.com; > xiyou.wangc...@gmail.com; j...@resnulli.us; da...@davemloft.net; > mawil...@microsoft.com > Subject: Re: [patch net-next 1/3] idr: Add new APIs to support unsigned long > > Hello, > > Chris Mi <chr...@mellanox.com> writes: > > > The following new APIs are added: > > > > int idr_alloc_ext(struct idr *idr, void *ptr, unsigned long *index, > > unsigned long start, unsigned long end, gfp_t gfp); > > static inline void *idr_remove_ext(struct idr *idr, unsigned long id); > > static inline void *idr_find_ext(const struct idr *idr, unsigned long > > id); void *idr_replace_ext(struct idr *idr, void *ptr, unsigned long > > id); void *idr_get_next_ext(struct idr *idr, unsigned long *nextid); > > > > Signed-off-by: Chris Mi <chr...@mellanox.com> > > Signed-off-by: Jiri Pirko <j...@mellanox.com> > > --- > > include/linux/idr.h | 16 ++++++++++ > > include/linux/radix-tree.h | 3 ++ > > lib/idr.c | 56 +++++++++++++++++++++++++++++++++++ > > lib/radix-tree.c | 73 > ++++++++++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 148 insertions(+) > > > > [...] > > > +int idr_alloc_ext(struct idr *idr, void *ptr, unsigned long *index, > > + unsigned long start, unsigned long end, gfp_t gfp) { > > + void __rcu **slot; > > + struct radix_tree_iter iter; > > + > > + if (WARN_ON_ONCE(radix_tree_is_internal_node(ptr))) > > + return -EINVAL; > > + > > + radix_tree_iter_init(&iter, start); > > + slot = idr_get_free_ext(&idr->idr_rt, &iter, gfp, end); > > + if (IS_ERR(slot)) > > + return PTR_ERR(slot); > > + > > + radix_tree_iter_replace(&idr->idr_rt, &iter, slot, ptr); > > + radix_tree_iter_tag_clear(&idr->idr_rt, &iter, IDR_FREE); > > + > > + if (index) > > + *index = iter.index; > > + return 0; > > +} > > +EXPORT_SYMBOL_GPL(idr_alloc_ext); > > Can you express idr_alloc in terms of idr_alloc_ext? Same for most of the > other functions (it seems that signed int was used as return value to indicate > error cases, thus it should be easy to map those). In idr_alloc(), we have the following check:
if (WARN_ON_ONCE(start < 0)) return -EINVAL; But in idr_alloc_ext(), since we are using unsigned long, we needn't such check. In order to reuse several lines of code, I think it is not worth to express idr_alloc() In terms of idr_alloc_ext. Thanks, Chris > > [...] > > Thanks, > Hannes