On Fri, Sep 17, 2021 at 03:42:18PM +0000, Conor Walsh wrote:
> When a suitable device is found during the PCI probe, create a dmadev
> instance for each channel. Internal structures and HW definitions required
> for device creation are also included.
>
> Signed-off-by: Conor Walsh <conor.wa...@intel.com>
> Reviewed-by: Kevin Laatz <kevin.la...@intel.com>
> ---
> drivers/dma/ioat/ioat_dmadev.c | 119 ++++++++++++++++++++++++++++++-
> drivers/dma/ioat/ioat_hw_defs.h | 45 ++++++++++++
> drivers/dma/ioat/ioat_internal.h | 24 +++++++
> 3 files changed, 186 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/ioat/ioat_dmadev.c b/drivers/dma/ioat/ioat_dmadev.c
> index f3491d45b1..b815d30bcf 100644
> --- a/drivers/dma/ioat/ioat_dmadev.c
> +++ b/drivers/dma/ioat/ioat_dmadev.c
> @@ -4,6 +4,7 @@
>
<snip>
> +/* Destroy a DMA device. */
> +static int
> +ioat_dmadev_destroy(const char *name)
> +{
> + struct rte_dma_dev *dev;
> + struct ioat_dmadev *ioat;
> + int ret;
> +
> + if (!name) {
> + IOAT_PMD_ERR("Invalid device name");
> + return -EINVAL;
> + }
> +
> + dev = &rte_dma_devices[rte_dma_get_dev_id(name)];
> + if (!dev) {
> + IOAT_PMD_ERR("Invalid device name (%s)", name);
> + return -EINVAL;
> + }
> +
I think you need to independently check the return value from
rte_dma_get_dev_id, rather than assuming when it returns an error value the
resultant index location will hold a null pointer.
> + ioat = dev->dev_private;
> + if (!ioat) {
> + IOAT_PMD_ERR("Error getting dev_private");
> + return -EINVAL;
> + }
> +
> + dev->dev_private = NULL;
> + rte_free(ioat->desc_ring);
> +
> + ret = rte_dma_pmd_release(name);
The rte_dma_pmd_allocate function reserves memory for the private data, so
the release function should free that memory too. However, you have
assigned private_data to NULL just above, so that probably won't work.
> + if (ret)
> + IOAT_PMD_DEBUG("Device cleanup failed");
> +
> + return 0;
> +}
> +
<snip>
