Re: [PATCH 7/8] pci: Add pci_for_each_device_all()

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Oct 25, 2021 at 08:57:36PM +0800, Peter Xu wrote:
> On Fri, Oct 22, 2021 at 04:43:43AM -0400, Michael S. Tsirkin wrote:
> > On Fri, Oct 22, 2021 at 10:33:15AM +0800, Peter Xu wrote:
> > > Hi, Michael,
> > > 
> > > On Thu, Oct 21, 2021 at 06:54:59AM -0400, Michael S. Tsirkin wrote:
> > > > > +typedef struct {
> > > > > +    pci_bus_dev_fn fn;
> > > > > +    void *opaque;
> > > > > +} pci_bus_dev_args;
> > > > 
> > > > code style violation. CamelCase for structs pls.
> > > 
> > > OK.
> > > 
> > > > > +/* Call 'fn' for each pci device on the system */
> > > > > +void pci_for_each_device_all(pci_bus_dev_fn fn, void *opaque);
> > > > 
> > > > Instead of hacking pci making initialization o(N^2),
> > > 
> > > Why it's O(N^2)?  One vIOMMU walks O(N), and we only have one vIOMMU, or am I
> > > wrong?
> > 
> > What I meant is this is O(N) and if called M times will be O(N * M)
> > yes your patches only call once so O(N), still we can do better.
> 
> I see.
> 
> > 
> > > > can't we add a variant of object_resolve_path_type ?
> > > 
> > > Could you elaborate?  Here what we want to do is to make sure there're no
> > > specific PCI devices registered, and potentially it can be more than one type
> > > of device in the future.
> > > 
> > > Thanks,
> > 
> > All you seem to care about is checking there's no VFIO
> > (why - should really be documented in a code comment much more clearly).
> 
> Right, Alex asked the same question.  I'll make sure to mention that in the
> commit message in the next version.
> 
> > Looks like object_resolve_path_type does that with O(1) complexity.
> > If we need a variant that checks for multiple types we can add that.
> 
> It's still O(N), or am I wrong?  I mean for example there's the loop in
> object_resolve_partial_path().

Only if there's a hash collision.

> But yeah I can use that too if you prefer, it's just that when we want to
> detect more types of pci classes it could be slower iiuc, because we'll need to
> call object_resolve_path_type() once for each type.  For pci bus scan it's
> always one round because we only have at most one x86 vIOMMU for each guest.
> 
> At the meantime, IMHO patch 1-6 are cleanups that should be good even without
> patch 7/8. If we prefer object_resolve_path_type() I'd still think it would be
> good to propose patch 1-6 separately (with some patch properly squashed as
> suggested by reviewers)?
> 
> Thanks,

OK let's handle that separately.

> -- 
> Peter Xu