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

[Pasha Tatashin]

On 2026-07-20 16:07:47-07:00, David Matlack wrote:
> 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:
> >
> > 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.

>From a design perspective, liveupdate_flb_get/put_incoming() is a 
logical read-lock/unlock pair on the FLB data. We use a refcount for 
optimization and sharing, but holding a get over a long asynchronous gap 
(from boot-time device setup to driver probe) is essentially holding an 
unbound lock.

Unbound locks make it difficult to trace refcount leaks or debug 
lifecycle issues.

> 
> > 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.

Can we do a fast-path check first?

static struct pci_flb_incoming *pci_liveupdate_get_incoming(struct pci_dev *dev)
{
	struct pci_flb_incoming *incoming;

	/* Fast-path to avoid unnecessary FLB querying */
	if (!dev->liveupdate.incoming)
		return NULL;

	incoming = pci_liveupdate_flb_get_incoming();
	if (!incoming)
		return NULL;

	/* Check again, now that FLB is acquired */
	if (dev->liveupdate.incoming)
		return incoming;

	pci_liveupdate_flb_put_incoming();
	return NULL;
}

This seems to gives us the best of both worlds: robust refcount hygiene 
and a sane fast path.

What do you think?