18/09/2021 05:24, Huisong Li: > 在 2021/9/17 20:50, Thomas Monjalon 写道: > > 17/09/2021 04:13, Huisong Li: > >> 在 2021/9/16 18:36, Thomas Monjalon 写道: > >>> 16/09/2021 10:01, Huisong Li: > >>>> 在 2021/9/8 15:20, Thomas Monjalon 写道: > >>>>> 08/09/2021 04:01, Huisong Li: > >>>>>> 在 2021/9/7 16:53, Thomas Monjalon 写道: > >>>>>>> 07/09/2021 05:41, Huisong Li: > >>>>>>>> Calling rte_eth_dev_close() will release resources of eth device and > >>>>>>>> close > >>>>>>>> it. But rte_pci_device struct isn't released when app exit, which > >>>>>>>> will lead > >>>>>>>> to memory leak. > >>>>>>> That's a PMD issue. > >>>>>>> When the last port of a PCI device is closed, the device should be > >>>>>>> freed. > >>>>>> Why is this a PMD problem? I don't understand. > >>>>> In the PMD close function, freeing of PCI device must be managed, > >>>>> so the app doesn't have to bother. > >>>> I know what you mean. Currently, there are two ways to close PMD device > >>>> (rte_eth_dev_close() and rte_dev_remove()). > >>>> > >>>> For rte_dev_remove(), eth device can be closed and rte_pci_device also > >>>> can be freed, so it can make app not care about that. > >>>> > >>>> But dev_close() is only used to close eth device, and nothing about > >>>> rte_pci_device is involved in the framework layer > >>>> > >>>> call stack of dev_close(). The rte_pci_device is allocated and > >>>> initialized when the rte_pci_bus scans "/sys/bus/pci/devices" directory. > >>>> > >>>> Generally, the PMD of eth devices operates on the basis of eth devices, > >>>> and rarely on rte_pci_device. > >>> No. The PMD is doing the relation between the PCI device and the ethdev > >>> port. > >> It seems that the ethdev layer can create eth devices based on > >> rte_pci_device, but does not release rte_pci_device. > > No, the ethdev layer does not manage any bus. > > Only the PMD does that. > > I don't mean that the ethdev layer manages the bus. > > I mean, it neither allocate rte_pci_device nor free it. > > >>>> And the rte_pci_device corresponding to the eth devices managed and > >>>> processed by rte_pci_bus. > >>>> > >>>> So, PMD is closed only based on the port ID of the eth device, whilch > >>>> only shuts down eth devices, not frees rte_pci_device > >>>> and remove it from rte_pci_bus. > >>> Not really. > >> I do not see any PMD driver releasing rte_pci_device in dev_close(). > > Maybe not but we should. > > I'm sure. > > As far as I know, the PMD does not free rte_pci_device for devices under > the PCI bus, whether ethdev or dmadev. > > > > >>> If there is no port using the PCI device, it should be released. > >> Yes. > >>>>>> As far as I know, most apps or examples in the DPDK project have only > >>>>>> one port for a pci device. > >>>>> The number of ports per PCI device is driver-specific. > >>>>> > >>>>>> When the port is closed, the rte_pci_device should be freed. But none > >>>>>> of > >>>>>> the apps seem to do this. > >>>>> That's because from the app point of view, only ports should be managed. > >>>>> The hardware device is managed by the PMD. > >>>>> Only drivers (PMDs) have to do the relation between class ports > >>>>> and hardware devices. > >>>> Yes. But the current app only closes the port to disable the PMD, and > >>>> the rte_pci_device cannot be freed. > >>> Why not? > >> Because most apps in DPDK call dev_close() to close the eth device > >> corresponding to a port. > > You don't say why the underlying PCI device could not be freed. > From the current implementation, rte_eth_dev_close() in ethdev layer > and dev_close() in PMD both do not free it. > > > >>>> Because rte_pci_device cannot be released in dev_close() of PMD, and is > >>>> managed by framework layer. > >>> No > >>> > >>>> Btw. Excluding rte_dev_probe() and rte_dev_remove(), it seems that the > >>>> DPDK framework only automatically > >>>> scans PCI devices, but does not automatically release PCI devices when > >>>> the process exits. > >>> Indeed, because such freeing is the responsibility of the PMD. > >> Do you mean to free rte_pci_device in the dev_close() API? > > I mean free the PCI device in the PMD implementation of dev_close. > > I don't think it's reasonable.
What is not reasonable, is to not free a device which is closed. > In the normal process, the rte_pci_device is allocated rte_eal_init() > when pci bus scan "/sys/bus/pci/devices" > > by calling rte_bus_scan() and insert to rte_pci_bus.device_list. > > Then, calling rte_bus_probe() in rte_eal_init to match rte_pci_device > and rte_pci_driver registered under rte_pci_bus > > to generate an eth device. > > From this point of view, the rte_pci_device should be managed and > released by the rte_pci_bus. > > Generally, the uninstallation operation should be reversed. Release the > eth device first and then release the rte_pci_device. Same for mbuf in mempool: allocation is done by the app, free is done by the PMD. Not everything is symmetrical. > Therefore the rte_pci_device does not be freed in the PMD > implementation of dev_close. > > > > >> How should PMD free it? What should we do? Any good suggestions? > > Check that there is no other port sharing the same PCI device, > > then call the PMD callback for rte_pci_remove_t. > > For primary and secondary processes, their rte_pci_device is independent. Yes it requires to free on both primary and secondary. > Is this for a scenario where there are multiple representor ports under > the same PCI address in the same processe? A PCI device can have multiple physical or representor ports. > >> Would it be more appropriate to do this in rte_eal_cleanup() if it > >> cann't be done in the API above? > > rte_eal_cleanup is a last cleanup for what was not done earlier. > > We could do that but first we should properly free devices when closed. > > > Totally, it is appropriate that rte_eal_cleanup is responsible for > releasing devices under the pci bus. Yes, but if a device is closed while the rest of the app keep running, we should not wait to free it.
