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

Re: [RFC PATCH] arm/vgic-v3: provide custom callbacks for pend_lpi_tree radix tree





On 11/02/2022 15:45, Luca Fancellu wrote:


On 11 Feb 2022, at 15:26, Julien Grall <julien@xxxxxxx> wrote:

Hi Luca,

On 11/02/2022 15:00, Luca Fancellu wrote:
pend_lpi_tree is a radix tree used to store pending irqs, the tree is
protected by a lock for read/write operations.
Currently the radix tree default function to free items uses the
RCU mechanism, calling call_rcu and deferring the operation.
However every access to the structure is protected by the lock so we
can avoid using the default free function that, by using RCU,
increases memory usage and impacts the predictability of the system.

I understand goal but looking at the implementation of vgic_v3_lpi_to_pending() 
(Copied below for convenience). We would release the lock as soon as the 
look-up finish, yet the element is returned.

static struct pending_irq *vgic_v3_lpi_to_pending(struct domain *d,
                                                  unsigned int lpi)
{
    struct pending_irq *pirq;

    read_lock(&d->arch.vgic.pend_lpi_tree_lock);
    pirq = radix_tree_lookup(&d->arch.vgic.pend_lpi_tree, lpi);
    read_unlock(&d->arch.vgic.pend_lpi_tree_lock);

    return pirq;
}

So the lock will not protect us against removal. If you want to drop the RCU, 
you will need to ensure the structure pending_irq is suitably protected. I 
haven't check whether there are other locks that may suit us here.


Hi Julien,

Yes you are right! I missed that, sorry for the noise.

Actually,... I think I am wrong :/.

I thought the lock pend_lpi_tre_lock would protect pending_irq, but it only protects the radix tree element (not the value).

The use in its_discard_event() seems to confirm that because the
pending_irq is re-initialized as soon as it gets destroyed.

I would like a second opinion though.

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.