Re: [dpdk-dev] [PATCH] bus/pci: optimize pci device probe

[Jerin Jacob <jerinjacobk@gmail.com>]

On Tue, Apr 28, 2020 at 3:04 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
>
> On Tue, Apr 28, 2020 at 2:21 PM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > On Sun, Apr 26, 2020 at 10:06 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > >
> > > 26/04/2020 20:41, Jerin Jacob:
> > > > On Sun, Apr 26, 2020 at 11:38 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > > > >
> > > > > 26/04/2020 19:38, jerinj@marvell.com:
> > > > > > From: Jerin Jacob <jerinj@marvell.com>
> > > > > >
> > > > > > If the PCI device is not attached to any driver then there is no
> > > > > > point in probing it. As an optimization, skip the PCI device probe if
> > > > > > the PCI device driver of type RTE_KDRV_NONE.
> > > > > >
> > > > > > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > > > > > ---
> > > > > > Notes:
> > > > > > ------
> > > > > > - virtio drivers does special treatment based on RTE_KDRV_UNKNOWN, That is
> > > > > > the reason allowing RTE_KDRV_UNKNOWN in this patch.
> > > > > > - virio devices uses RTE_KDRV_UNKNOWN for some special meaning, IMO, if it would
> > > > > >   be better, if
> > > > > > a) Introduce the KDRV for virio
> > > > > > b) If the PCIe device of driver type NONE or UNKNOWN then not even add in pci
> > > > > > list
> > > > > > in the scan, It will improve the boot time by avoiding operation on
> > > > > > unwanted device like sorting the PCI devices, scanning it, probe it, managing
> > > > > > it etc.
> > > > >
> > > > > mlx4/mlx4 uses RTE_KDRV_UNKNOWN.
> > > >
> > > > OK.
> > > >
> > > > > > - Initial problem reported at http://patches.dpdk.org/patch/64999/ as
> > > > > > boot time printf clutter on octeontx2 devices with a lot PCI devices which are
> > > > > > of type RTE_KDRV_NONE.
> > > > >
> > > > > Add a logtype for PCI driver and adjust log level accordingly
> > > > > to your preferences.
> > > > >
> > > > > > @@ -271,6 +271,8 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
> > > > > >       FOREACH_DRIVER_ON_PCIBUS(dr) {
> > > > > > +             if (dev->kdrv == RTE_KDRV_NONE)
> > > > > > +                     continue;
> > > > > >               rc = rte_pci_probe_one_driver(dr, dev);
> > > > >
> > > > > Nack
> > > >
> > > > I understand mlx4/mlx5 is using RTE_KDRV_UNKNOWN, Here we are skipping
> > > > the RTE_KDRV_NONE,
> > > > What is the use case for probing the devices with RTE_KDRV_NONE?
> > >
> > > Maybe you are right. I don't remember the use case.
> > > I think I remember these virtio and vmxnet3 PMD were not using UIO:
> > >         http://git.dpdk.org/old/virtio-net-pmd/tree/virtio_user.c
> > >         http://git.dpdk.org/old/vmxnet3-usermap/tree/pmd/vmxnet3.c
> > >
> > > We need to know which case is using following code:
> > >
> > >     case RTE_KDRV_NONE:
> > > #if defined(RTE_ARCH_X86)
> > >         ret = pci_ioport_map(dev, bar, p);
> > > #endif
> > >         break;
> > >
> > > David, please could you refresh our memory?
> >
> > The in-tree virtio-net driver directly calls rte_pci_map_device /
> > rte_pci_ioport_map depending on virtio legacy/modern modes.
> > This is why the virtio pci driver does not ask for
> > RTE_PCI_DRV_NEED_MAPPING to the pci bus.
> >
> >
> > In ioport mode, there were two options for historical reasons because
> > of the virtio driver you mention.
> > This driver did not rely on uio, and this behavior was later merged to
> > the current in-tree driver.
> > It ended up (not clean) in the pci bus driver because virtio was the
> > only user of this code.
> >
> >
> > Removing this special case could break x86 applications running with
> > legacy virtio.
> >
> >
> > On the plus side, we have been announcing for some time in virtio:
> > RTE_PMD_REGISTER_KMOD_DEP(net_virtio, "* igb_uio | uio_pci_generic | vfio-pci");
>
> What is to conclude?
> # The In-tree virtio driver uses ""* igb_uio | uio_pci_generic |
> vfio-pci"" driver as backend and it does not need RTE_KDRV_NONE?
> OR
> # The in-tree, legacy virtio(const struct virtio_pci_ops legacy_op)
> can work without any kernel driver in the backend. So RTE_KDRV_NONE
> need?

Ping. What is the conclusion? If it is former then this patch is valid.



>
>
>
>
> >
> >
> > --
> > David Marchand
> >