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

Re: [RFC PATCH 00/10] Rework PCI locking



Hi Jan,

On 06/09/2022 11:32, Jan Beulich wrote:
On 31.08.2022 16:10, Volodymyr Babchuk wrote:
Hello,

This is yet another take to a PCI locking rework. This approach
was suggest by Jan Beulich who proposed to use a reference
counter to control lifetime of pci_dev objects.

When I started added reference counting it quickly became clear
that this approach can provide more granular locking insted of
huge pcidevs_lock() which is used right now. I studied how this
lock used and what it protects. And found the following:

0. Comment in pci.h states the following:

  153 /*
  154  * The pcidevs_lock protect alldevs_list, and the assignment for the
  155  * devices, it also sync the access to the msi capability that is not
  156  * interrupt handling related (the mask bit register).
  157  */

But in reality it does much more. Here is what I found:

1. Lifetime of pci_dev struct

2. Access to pseg->alldevs_list

3. Access to domain->pdev_list

4. Access to iommu->ats_list

5. Access to MSI capability

6. Some obsucure stuff in IOMMU drivers: there are places that
are guarded by pcidevs_lock() but it seems that nothing
PCI-related happens there.

Right - the lock being held was (ab)used in IOMMU code in a number of
places. This likely needs to change in the course of this re-work;
patch titles don't suggest this is currently part of the series.

7. Something that I probably overlooked

And this is the main risk here. The huge scope of the original lock
means that many things are serialized now but won't be anymore once
the lock is gone.

But yes - thanks for the work. To be honest I don't expect to be able
to look at this series in detail until after the Xen Summit. And even
then it may take a while ...

I was wondering if this is still in your list to review?

Cheers,

--
Julien Grall



 


Rackspace

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