Re: [PATCH v7 03/12] PCI: liveupdate: Track incoming preserved PCI devices

[David Matlack]

On Mon, Jul 20, 2026 at 3:44 PM Pasha Tatashin
<pasha.tatashin@xxxxxxxxxx> wrote:
>
> On 2026-07-20 14:54:51-07:00, David Matlack wrote:
> > On Fri, Jul 17, 2026 at 2:47 PM Pasha Tatashin
> > <pasha.tatashin@xxxxxxxxxx> wrote:
> >
> > > On Fri, 10 Jul 2026 21:26:06 +0000, David Matlack <dmatlack@xxxxxxxxxx> wrote:
> > >
> > > I am confused by this. Each preserved PCI device must have an associated
> > > FD preserved with it via LUO. I.e., vfiofd would need to be preserved. If
> > > the vfiofd was not reclaimed, and finish is not possible, that vfiofd
> > > would still be owned by LUO, and therefore PCI FLB refcount would stay
> > > positive.
> > >
> > > However, if finish is possible, and this is the last vfiofd that is
> > > finished, FLB will be freed as soon as the reference count reaches zero,
> > > which I would think is the expected behavior.
> > >
> > > What is the point of holding a reference here, instead of only for the
> > > duration of FLB access, i.e. to make sure we are accessing a valid data?
> >
> > The duration of the access is from here until pci_liveupdate_finish()
> > because that it when the pointer (dev->liveupdate.incoming) is
> > cleared. So that is why the PCI core holds the reference from here
> > until pci_liveupdate_finish().
> >
> > We could avoid this by deleteing dev->liveupdate.incoming and,
> > instead, fetching the incoming FLB and doing the xarray lookup every
> > atime the PCI core needs to access the device's incoming ser struct,
> > but that would be inefficient.
>
> Thanks for the explanation. As I understand, the most straightforward
> way to avoid holding the permanent reference is indeed to delete
> dev->liveupdate.incoming entirely and perform an xarray lookup on every
> access, like this:
>
> bool pci_liveupdate_is_incoming(struct pci_dev *dev)
> {
>         ...
>         incoming = pci_liveupdate_flb_get_incoming();
>         ...
>         dev_ser = xa_load(&incoming->xa, key);
>         ...
>         pci_liveupdate_flb_put_incoming();
>         return dev_ser && dev_ser->refcount > 0;
> }
>
> However, as you note, this is inefficient because it affects every
> single device and adds lookup overhead to every access (not sure about
> the actual cost though, xarray access is pretty fast!).
>
> We can, however, still avoid tinkering with the lifecycle of the FLB,
> and instead treat dev->liveupdate.incoming as a "hint" that we validate
> on access with a fast, liveness check:

Can you tell me more about your concern about FLB lifetime?

The lifetime of the FLB will not be affected by this reference unless
there is a bug in the driver where it fails to call
pci_liveupdate_finish() during it's file handler finish callback.

>
> 1. At Setup: In pci_liveupdate_setup_device(), we do the xarray lookup
> once, cache the pointer in dev->liveupdate.incoming, and immediately
> call pci_liveupdate_flb_put_incoming(). We do not hold a permanent
> reference.
>
> 2. On Access: When an accessor runs, instead of doing a full xarray
> lookup, it just validates the cached pointer's liveness by temporarily
> securing the FLB:
>
> static struct pci_flb_incoming *pci_liveupdate_get_incoming(struct pci_dev *dev)
> {
>         struct pci_flb_incoming *incoming;
>
>         incoming = pci_liveupdate_flb_get_incoming();
>         if (!incoming)
>                 return NULL;
>
>         if (dev->liveupdate.incoming)
>                 return incoming;
>
>         pci_liveupdate_flb_put_incoming();
>         return NULL;
> }
>
> * If get_incoming() returns NULL (the FLB has already finished/freed),
>   the hint is invalid and the device is no longer incoming.

This avoids the xarray lookup but still requires taking the incoming
FLB mutex twice (once for get and once for put) on every access. And
if there's no incoming PCI FLB, the LUO will iterate over all incoming
FLBs under the mutex to find it.

>
> * If it returns a valid pointer, the FLB is guaranteed to be alive, so
>   we can immediately use the cached dev->liveupdate.incoming pointer
>   without xarray lookups. The accessor then puts the transient
>   reference when done.
>
> 3. In Finish/Cleanup: We can simply clear dev->liveupdate.incoming,
> and decrement ser->nr_devices.
>