Re: [Xen-devel] [PATCH v4 06/17] xen/arm: ITS: Add virtual ITS driver

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Fri, 2015-07-10 at 13:12 +0530, vijay.kilari@xxxxxxxxx wrote:
> +/* RB-tree helpers for vits_device attached to a domain */

In the rest of the series I found this used in three places:
      * On assignment, to insert the device into the tree
      * On deassignment, to remove it again
      * In vgic_vcpu_inject_lpi, where the device is looked up and then
        never used.

I don't see any other use and therefore I don't think this RB tree
serves any purpose, which is consistent with the design which doesn't
require this lookup anywhere. Please remove it.

If there is some use of it in some future series (e.g. perhaps the PCI
one) then please still remove it and add a patch to that series to
introduce it.



> +struct vits_device *vits_find_device(struct rb_root *root, uint32_t devid)
> +{
> +    struct rb_node *node = root->rb_node;
> +
> +    while ( node )
> +    {
> +        struct vits_device *dev;
> +
> +        dev = container_of(node, struct vits_device, node);
> +
> +        if ( devid < dev->vdevid )
> +            node = node->rb_left;
> +        else if ( devid > dev->vdevid )
> +            node = node->rb_right;
> +        else
> +            return dev;
> +    }
> +
> +    return NULL;
> +}
> +
> +int vits_insert_device(struct rb_root *root, struct vits_device *dev)
> +{
> +    struct rb_node **new, *parent;
> +
> +    new = &root->rb_node;
> +    parent = NULL;
> +    while ( *new )
> +    {
> +        struct vits_device *this;
> +
> +        this  = container_of(*new, struct vits_device, node);
> +
> +        parent = *new;
> +        if ( dev->vdevid < this->vdevid )
> +            new = &((*new)->rb_left);
> +        else if ( dev->vdevid > this->vdevid )
> +            new = &((*new)->rb_right);
> +        else
> +            return -EEXIST;
> +    }
> +
> +    rb_link_node(&dev->node, parent, new);
> +    rb_insert_color(&dev->node, root);
> +
> +    return 0;
> +}
> +
> +void vits_remove_device(struct rb_root *root, struct vits_device *dev)
> +{
> +    if ( dev )
> +        rb_erase(&dev->node, root);
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel