On Tue, Aug 23, 2022 at 8:18 AM Jason Wang <jasow...@redhat.com> wrote: > > > 在 2022/8/20 00:53, Eugenio Pérez 写道: > > It's convenient to call iova_tree_remove from a map returned from > > iova_tree_find or iova_tree_find_iova. > > > The looks like a hint of the defect of current API. > > > > With the current code this is not > > possible, since we will free it, and then we will try to search for it > > again. > > > > Fix it making a copy of the argument. Not applying a fixes tag, since > > there is no use like that at the moment. > > > > Signed-off-by: Eugenio Pérez <epere...@redhat.com> > > --- > > util/iova-tree.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/util/iova-tree.c b/util/iova-tree.c > > index fee530a579..713e3edd7b 100644 > > --- a/util/iova-tree.c > > +++ b/util/iova-tree.c > > @@ -166,9 +166,11 @@ void iova_tree_foreach(IOVATree *tree, > > iova_tree_iterator iterator) > > > > void iova_tree_remove(IOVATree *tree, const DMAMap *map) > > { > > + /* Just in case caller is calling iova_tree_remove from a result of > > find */ > > + const DMAMap needle = *map; > > > Then let's simply make iova_tree_remove() accept const DMAMap instead of > the pointer to it. >
Do you mean to accept DMAMap by value, isn't it? (no need for const it then, isn't it?). > > > const DMAMap *overlap; > > > > - while ((overlap = iova_tree_find(tree, map))) { > > + while ((overlap = iova_tree_find(tree, &needle))) { > > g_tree_remove(tree->tree, overlap); > > } > > > So we had following in iova_tree_insert(): > > /* We don't allow to insert range that overlaps with existings */ > if (iova_tree_find(tree, map)) { > return IOVA_ERR_OVERLAP; > } > > I wonder how overlap can happen? > I don't get this part. The problem is that iova_tree_find returns a pointer to an internal structure, there is no need to insert multiple times overlapping maps. Thanks!