Re: [PATCH v4 01/11] PCI: liveupdate: Set up FLB handler for the PCI core

[David Matlack <dmatlack@google.com>]

On 2026-04-28 12:45 PM, Vipin Sharma wrote:
> On Thu, Apr 23, 2026 at 09:23:05PM +0000, David Matlack wrote:
> > +	pr_debug("Preserving struct pci_ser with room for %u devices\n",
> > +		 max_nr_devices);
> > +
> > +	ser = kho_alloc_preserve(size);
> > +	if (IS_ERR(ser))
> > +		return PTR_ERR(ser);
> 
> Should there be a similar pr_debug() in case of failure to denote that above
> "Preserving ..." message didn't finish, or, maybe just print one
> pr_debug() after the error check above?

Hm... I guess there could always be more pr_debug()s but I don't want to
instrument every error path. I could move it to the success path but I
don't see how that makes it any better.

> 
> > +/**
> > + * struct pci_dev_ser - Serialized state about a single PCI device.
> > + *
> > + * @domain: The device's PCI domain number (segment).
> > + * @bdf: The device's PCI bus, device, and function number.
> > + * @reserved: Reserved (to naturally align struct pci_dev_ser).
> > + */
> > +struct pci_dev_ser {
> > +	u32 domain;
> > +	u16 bdf;
> > +	u16 reserved;
> 
> Should this be renamed to 'u8 __padding[2];' instead? This will allow to
> just change the array length based on the need (0, 1, 2, 3).

Sorry I'm not following what you mean here. What is the reason to rename
this field and change it to an array?

> > +} __packed;
> > +
> > +/**
> > + * struct pci_ser - PCI Subsystem Live Update State
> > + *
> > + * This struct tracks state about all devices that are being preserved across
> > + * a Live Update for the next kernel.
> > + *
> > + * @max_nr_devices: The length of the devices[] flexible array.
> > + * @nr_devices: The number of devices that were preserved.
> > + * @devices: Flexible array of pci_dev_ser structs for each device.
> > + */
> > +struct pci_ser {
> > +	u32 max_nr_devices;
> > +	u32 nr_devices;
> > +	struct pci_dev_ser devices[];
> > +} __packed;
> > +
> > +/* Ensure all elements of devices[] are naturally aligned. */
> > +static_assert(offsetof(struct pci_ser, devices) % sizeof(unsigned long) == 0);
> > +static_assert(sizeof(struct pci_dev_ser) % sizeof(unsigned long) == 0);
> 
> Nit: Maybe move this assert to be near to the definition of this struct,
> easier to find it when editing the struct vs finding it later during
> build.

The combination of these 2 asserts is what guarantees that every element
of the devices[] array are naturally aligned, that's why I put them
together here.

I can move it up though if you think it's better.