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

Re: [PATCH v6 03/13] vpci: move lock outside of struct vpci



On 07.02.2022 13:57, Oleksandr Andrushchenko wrote:
> 
> 
> On 07.02.22 14:34, Jan Beulich wrote:
>> On 07.02.2022 12:08, Oleksandr Andrushchenko wrote:
>>> 1. vpci_{read|write} are not protected with pcidevs_lock and can run in
>>> parallel with pci_remove_device which can remove pdev after 
>>> vpci_{read|write}
>>> acquired the pdev pointer. This may lead to a fail due to pdev dereference.
>>>
>>> So, to protect pdev dereference vpci_{read|write} must also use pdevs_lock.
>> I think this is not the only place where there is a theoretical race
>> against pci_remove_device().
> Not at all, that was just to demonstrate one of the possible sources of races.
>>   I would recommend to separate the
>> overall situation with pcidevs_lock from the issue here.
> Do you agree that there is already an issue with that? In the currently 
> existing code?
>>   I don't view
>> it as an option to acquire pcidevs_lock in vpci_{read,write}().
> Yes, that would hurt too much, I agree. But this needs to be solved
>>   If
>> anything, we need proper refcounting of PCI devices (at which point
>> likely a number of lock uses can go away).
> It seems so. Then not only pdev's need refcounting, but pdev->vpci as well
> 
> What's your view on how can we achieve both goals?
> pdev and pdev->vpci and locking/refcounting

I don't see why pdev->vpci might need refcounting. And just to state it
in different words: I'd like to suggest to leave aside the pdev locking
as long as it's _just_ to protect against hot remove of a device. That's
orthogonal to what you need for vPCI, where you need to protect
against the device disappearing from a guest (without at the same time
disappearing from the host).

Jan




 


Rackspace

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