Re: [PATCH v4 02/11] PCI: liveupdate: Track outgoing preserved PCI devices

David Matlack
2026-05-01T18:25:54Z

On Fri, May 1, 2026 at 11:17 AM Samiullah Khawaja <skhawaja@google.com> wrote:
>
> On Thu, Apr 30, 2026 at 09:15:14PM +0000, David Matlack wrote:
> >On 2026-04-28 05:24 PM, Samiullah Khawaja wrote:
> >> On Thu, Apr 23, 2026 at 09:23:06PM +0000, David Matlack wrote:
> >
> >> > + for (i = 0; i < ser->max_nr_devices; i++) {
> >> > + /*
> >> > + * Start searching at index ser->nr_devices. This should result
> >> > + * in a constant time search under expected conditions (devices
> >> > + * are not getting unpreserved).
> >> > + */
> >> > + int index = (ser->nr_devices + i) % ser->max_nr_devices;
> >> > + struct pci_dev_ser *dev_ser = &ser->devices[index];
> >>
> >> nit: Maybe we can move this logic in a separate function as down the road
> >> when we expand this to add VFs and Hotpluggable devices, this might
> >> change significantly? It's good if it is self-contained.
> >
> >Did you mean to leave this comment on pci_flb_preserve() where it
> >decides how many devices to allocate room for?
>
> I was talking about this one, as I think depending on the scheme we take
> this might change significantly. Just a nit, you can ignore it.

It would depend on if userspace is doing a lot of unpreserving as
well... I'll see what I can do in v5.

> >
> >> > +static inline struct pci_dev_ser *pci_liveupdate_outgoing(struct pci_dev *dev)
> >> > +{
> >> > + return dev->liveupdate_outgoing;
> >> > +}
> >>
> >> Is this expected to be called under the outgoing lock?
> >
> >For now this API is only used during shutdown, at which point userspace
> >should have already been stopped so drivers should not be changing the
> >preservation status of an outgoing device. So I don't think this needs
> >to be under the outgoing lock, but it would be nice to have some more
> >explicit synchronization.
>
> Ok that makes sense. I have similar cases in my series, but maybe we can
> add kdoc regarding these stating in which context this is expected to be
> used?

Yes will do. I'll add some documentation and also restrict it to avoid
misuse (move to internal pci.h, return a bool instead of pointer, and
use READ_ONCE()).