Re: [PATCH v5 09/11] PCI: liveupdate: Freeze preservation status during shutdown

[David Matlack <dmatlack@google.com>]

On 2026-05-14 03:14 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Preservation rollback fails if the device is concurrently frozen, breaking bridge hierarchy invariants.
> - [Low] Syntax error in the !CONFIG_PCI_LIVEUPDATE stub for pci_liveupdate_freeze().
> --
> 
> commit f49768e50767ced554fd83ab7e5016423c3d87a2
> Author: David Matlack <dmatlack@google.com>
> 
> PCI: liveupdate: Freeze preservation status during shutdown
> 
> Freeze a device's outgoing preservation status (preserved or not preserved)
> during shutdown. This enables the PCI core and drivers to safely make
> decisions based on the device's preservation status during shutdown.
> 
> > diff --git a/drivers/pci/liveupdate.c b/drivers/pci/liveupdate.c
> > index 6ab03bd548b37..825166a579130 100644
> > --- a/drivers/pci/liveupdate.c
> > +++ b/drivers/pci/liveupdate.c
> > @@ -334,6 +334,11 @@ static int pci_liveupdate_unpreserve_device(struct pci_ser *ser, struct pci_dev
> >  
> >  	guard(write_lock)(&dev->liveupdate.lock);
> >  
> > +	if (dev->liveupdate.frozen) {
> > +		pci_WARN(dev, 1, "Cannot unpreserve device after it is frozen!\n");
> > +		return -EINVAL;
> > +	}
> 
> During preservation rollback, can this check leave a child device preserved
> while its parent is not?
> 
> If pci_liveupdate_preserve_path() successfully preserves a child device,
> but fails to preserve the parent bridge, it attempts to roll back by calling
> pci_liveupdate_unpreserve_device() on the child.
> 
> If a concurrent system shutdown calls pci_liveupdate_freeze() on the child
> device before this rollback occurs, this check will return -EINVAL and
> fail the rollback.
> 
> Does this result in a broken topology being passed to the incoming kernel?

Hm yes that race does look possible. Will fix in v6.

> 
> [ ... ]
> 
> > diff --git a/drivers/pci/liveupdate.h b/drivers/pci/liveupdate.h
> > index fd7693c7ddd21..30deaa673efed 100644
> > --- a/drivers/pci/liveupdate.h
> > +++ b/drivers/pci/liveupdate.h
> [ ... ]
> > @@ -26,6 +27,10 @@ static inline void pci_liveupdate_cleanup_device(struct pci_dev *dev)
> >  {
> >  }
> >  
> > +static inline void pci_liveupdate_freeze(struct pci_dev *dev);
> > +{
> > +}
> 
> Does this stray semicolon after the function signature cause a compilation
> error for kernels built without CONFIG_PCI_LIVEUPDATE?

Yup, Mike Rapoport and kernel-test-robot also flagged this off-list.
Will fix in v6.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260512184846.119396-1-dmatlack@google.com?part=9