Re: [Xen-devel] [PATCH v3 08/15] argo: implement the unregister op

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.01.19 at 08:42, <christopher.w.clark@xxxxxxxxx> wrote:
> @@ -666,6 +667,105 @@ ring_find_info(const struct domain *d, const struct 
> argo_ring_id *id)
>      return NULL;
>  }
>  
> +static struct argo_send_info *
> +send_find_info(const struct domain *d, const struct argo_ring_id *id)

As per the comment on patch 7, perhaps find_send_info()?

> +{
> +    struct hlist_node *node;

const?

> +static long
> +unregister_ring(struct domain *currd,
> +                XEN_GUEST_HANDLE_PARAM(xen_argo_unregister_ring_t) unreg_hnd)
> +{
> +    xen_argo_unregister_ring_t unreg;
> +    struct argo_ring_id ring_id;
> +    struct argo_ring_info *ring_info;
> +    struct argo_send_info *send_info;
> +    struct domain *dst_d = NULL;
> +    int ret;
> +
> +    ret = copy_from_guest(&unreg, unreg_hnd, 1) ? -EFAULT : 0;
> +    if ( ret )
> +        goto out;
> +
> +    ret = unreg.pad ? -EINVAL : 0;
> +    if ( ret )
> +        goto out;
> +
> +    ring_id.partner_id = unreg.partner_id;
> +    ring_id.port = unreg.port;
> +    ring_id.domain_id = currd->domain_id;
> +
> +    read_lock(&argo_lock);
> +
> +    if ( !currd->argo )
> +    {
> +        ret = -ENODEV;
> +        goto out_unlock;
> +    }
> +
> +    write_lock(&currd->argo->lock);
> +
> +    ring_info = ring_find_info(currd, &ring_id);
> +    if ( ring_info )
> +    {
> +        ring_remove_info(currd, ring_info);
> +        currd->argo->ring_count--;
> +    }
> +
> +    dst_d = get_domain_by_id(ring_id.partner_id);
> +    if ( dst_d )
> +    {
> +        if ( dst_d->argo )
> +        {
> +            spin_lock(&dst_d->argo->send_lock);
> +
> +            send_info = send_find_info(dst_d, &ring_id);
> +            if ( send_info )
> +            {
> +                hlist_del(&send_info->node);
> +                xfree(send_info);
> +            }
> +
> +            spin_unlock(&dst_d->argo->send_lock);

As per the comment to an earlier patch, if at all possible call
allocation (and hence also freeing) functions with as little
locks held as possible. Pulling it out of the innermost lock
here looks straightforward at least.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel