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

On 2026-07-21 20:25:18+00:00, David Matlack wrote:
> On 2026-07-21 05:55 PM, Pasha Tatashin wrote:
> 
> > On 2026-07-20 16:07:47-07:00, David Matlack wrote:
> > 
> > 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.
> 
> It's very standard for refcounts to be held for a long time, e.g. file
> refcounts and device refcounts. LUO FLBs themselves already have
> long-lived refcounts taken by each file that depends on themm, which
> aren't released until that file handler's finish callback runs. The PCI
> core is just doing the same thing.

That is right, but that refcount is tracked internally by LUO and 
follows the design of File-Lifecycle-Bound objects, not the lifecycle of 
the FLB's users.

It really seems backward to me to force FLB objects to have a different 
lifecycle. Let's start with a concise lifecycle without setting a 
precedent where clients of FLB try to control the duration for which 
objects exist.

> 
> > 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?
> 
> It still seems like an overall worse approach to me.
> 
>  * From a performance perspective, the PCI core would still have to
>    acquire the FLB mutex twice every time it needs to use the device's
>    serialized state (once to acquire a new reference and once to release
>    it).

I suspect, acquiring/releasing the FLB mutex in this case is trivial and 
won't be noticeable in any  real-world scenario.

> 
>  * From a locking perspective, the refcount doesn't actually protect
>    against the device itself going through finish. So we still need the
>    pci_liveupdate.rwsem.
> 
>  * From a hygiene perspective, each reference is held for less time yes,
>    but there will more places in the code that need to acquire and
>    release a refcount.  That is more room for bugs to leak a refcount.
>    So I'm not sure taking more small refcounts is an improvement.

IMO, short-scoped acquire/release helpers around actual usage are much 
easier to audit than holding a refcount open across an arbitrary, 
asynchronous gap waiting for driver probe.