[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v13 03/10] virtio-iommu: Implement attach/detach command

From: Peter Xu
Subject: Re: [PATCH v13 03/10] virtio-iommu: Implement attach/detach command
Date: Mon, 3 Feb 2020 10:19:17 -0500

On Mon, Feb 03, 2020 at 03:59:00PM +0100, Auger Eric wrote:


> >> +static void virtio_iommu_detach_endpoint_from_domain(VirtIOIOMMUEndpoint 
> >> *ep)
> >> +{
> >> +    QLIST_REMOVE(ep, next);
> >> +    g_tree_unref(ep->domain->mappings);
> > 
> > Here domain->mapping is unreferenced for each endpoint, while at [1]
> > below you only reference the domain->mappings if it's the first
> > endpoint.  Is that problematic?
> in [1] I take a ref to the domain->mappings if it is *not* the 1st
> endpoint. This aims at deleting the mappings gtree when the last EP is
> detached from the domain.
> This fixes the issue reported by Jean in:
> https://patchwork.kernel.org/patch/11258267/#23046313

Ah OK. :)

However this is tricky.  How about do explicit g_tree_destroy() in
virtio_iommu_detach() when it's the last endpoint?  I see that you

     * when the last EP is detached, simply remove the domain for
     * the domain list and destroy it. Note its mappings were already
     * freed by the ref count mechanism. Next operation involving
     * the same domain id will re-create one domain object.
    if (QLIST_EMPTY(&domain->endpoint_list)) {
        g_tree_remove(s->domains, GUINT_TO_POINTER(domain->id));

Then it becomes:

    if (QLIST_EMPTY(&domain->endpoint_list)) {
        g_tree_remove(s->domains, GUINT_TO_POINTER(domain->id));

And also remove the trick in attach() so you take the domain ref
unconditionally.  Would that work?


Peter Xu

reply via email to

[Prev in Thread] Current Thread [Next in Thread]