Re: [PATCH v3 2/6] xen: pci: introduce reference counting for pdev

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.04.2023 13:00, Volodymyr Babchuk wrote:
> 
> Hello Roger,
> 
> Roger Pau Monné <roger.pau@xxxxxxxxxx> writes:
> 
>> On Mon, Apr 17, 2023 at 12:34:31PM +0200, Jan Beulich wrote:
>>> On 17.04.2023 12:17, Roger Pau Monné wrote:
>>>> On Fri, Apr 14, 2023 at 01:30:39AM +0000, Volodymyr Babchuk wrote:
>>>>> Above I have proposed another view on this. I hope, it will work for
>>>>> you. Just to reiterate, idea is to allow "harmless" refcounts to be left
>>>>> after returning from pci_remove_device(). By "harmless" I mean that
>>>>> owners of those refcounts will not try to access the physical PCI
>>>>> device if pci_remove_device() is already finished.
>>>>
>>>> I'm not strictly a maintainer of this piece code, albeit I have an
>>>> opinion.  I will like to also hear Jans opinion, since he is the
>>>> maintainer.
>>>
>>> I'm afraid I can't really appreciate the term "harmless refcounts". Whoever
>>> holds a ref is entitled to access the device. As stated before, I see only
>>> two ways of getting things consistent: Either pci_remove_device() is
>>> invoked upon dropping of the last ref,
>>
>> With this approach, what would be the implementation of
>> PHYSDEVOP_manage_pci_remove?  Would it just check whether the pdev
>> exist and either return 0 or -EBUSY?
>>
> 
> Okay, I am preparing patches with the behavior you proposed. To test it,
> I artificially set refcount to 2 and indeed PHYSDEVOP_manage_pci_remove
> returned -EBUSY, which propagated to the linux driver. Problem is that
> Linux driver can't do anything with this. It just displayed an error
> message and removed device anyways. This is because Linux sends
> PHYSDEVOP_manage_pci_remove in device_remove() call path and there is no
> way to prevent the device removal. So, admin is not capable to try this
> again.

So maybe Linux'es issuing of the call needs moving elsewhere? Or we need
a new sub-op, such that PHYSDEVOP_manage_pci_remove can remain purely a
last-moment notification?

> As I workaround, I can create hypercall continuation in case if
> pci_remove_device() returns -EBUSY. What is your opinion?

How would that help? You'd then spin perhaps for hours or days ...

Jan