Date: Wed, 5 Jun 2024 01:00:24 +0100 From: Jessica Clarke <jrtc27@freebsd.org> To: John Baldwin <jhb@FreeBSD.org> Cc: "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@FreeBSD.org> Subject: Re: git: 871b33ad65ba - main - pci: Consistently use pci_vf_* for suballocated VF memory resources Message-ID: <587A0604-BF30-4A90-9B04-FC56C53BBF2A@freebsd.org> In-Reply-To: <202406042352.454NqFVj061328@gitrepo.freebsd.org> References: <202406042352.454NqFVj061328@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 5 Jun 2024, at 00:52, John Baldwin <jhb@FreeBSD.org> wrote: > > The branch main has been updated by jhb: > > URL: https://cgit.FreeBSD.org/src/commit/?id=871b33ad65baf07c92cce120a4fc1978c2ed7b3b > > commit 871b33ad65baf07c92cce120a4fc1978c2ed7b3b > Author: John Baldwin <jhb@FreeBSD.org> > AuthorDate: 2024-06-04 23:51:37 +0000 > Commit: John Baldwin <jhb@FreeBSD.org> > CommitDate: 2024-06-04 23:51:37 +0000 > > pci: Consistently use pci_vf_* for suballocated VF memory resources > > Some of the bus resource methods were passing these up to the parent > which triggered rman mismatch assertions in INVARIANTS kernels. > > Reported by: kp > Reviewed by: imp > Tested by: kp (earlier version) > Differential Revision: https://reviews.freebsd.org/D45406 > --- > sys/dev/pci/pci.c | 118 ++++++++++++++++++++++++++++++++++-- > sys/dev/pci/pci_iov.c | 151 ++++++++++++++++++++++++++++++++++++++++++++++ > sys/dev/pci/pci_private.h | 19 ++++++ > 3 files changed, 284 insertions(+), 4 deletions(-) > > diff --git a/sys/dev/pci/pci.c b/sys/dev/pci/pci.c > index 2cb8b4ce9fcc..2093d6a8b5ef 100644 > --- a/sys/dev/pci/pci.c > +++ b/sys/dev/pci/pci.c > @@ -164,10 +164,18 @@ static device_method_t pci_methods[] = { > DEVMETHOD(bus_get_resource, bus_generic_rl_get_resource), > DEVMETHOD(bus_delete_resource, pci_delete_resource), > DEVMETHOD(bus_alloc_resource, pci_alloc_resource), > +#ifdef PCI_IOV > + DEVMETHOD(bus_adjust_resource, pci_adjust_resource), > +#else > DEVMETHOD(bus_adjust_resource, bus_generic_adjust_resource), > +#endif > DEVMETHOD(bus_release_resource, pci_release_resource), > DEVMETHOD(bus_activate_resource, pci_activate_resource), > DEVMETHOD(bus_deactivate_resource, pci_deactivate_resource), > +#ifdef PCI_IOV > + DEVMETHOD(bus_map_resource, pci_map_resource), > + DEVMETHOD(bus_unmap_resource, pci_unmap_resource), > +#endif > DEVMETHOD(bus_child_deleted, pci_child_deleted), > DEVMETHOD(bus_child_detached, pci_child_detached), > DEVMETHOD(bus_child_pnpinfo, pci_child_pnpinfo_method), Would it make sense to instead #ifdef parts of the body of pci_adjust_resource rather than switching which function you’re using in the first place? I feel that is generally easier to understand, as there’s less conditionality, and it’s more easily extensible if any other special cases come along. Jess
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?587A0604-BF30-4A90-9B04-FC56C53BBF2A>