diff mbox series

[v2,4/4] net/ixgbe: delete HW rings when releasing queues

Message ID 22811a29652fba4827277ddebc578596ee2e766d.1631953295.git.wangyunjian@huawei.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series delete HW rings when releasing queues for some drivers |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-broadcom-Functional fail Functional Testing issues
ci/iol-mellanox-Functional fail Functional Testing issues
ci/Intel-compilation success Compilation OK
ci/intel-Testing fail Testing issues
ci/iol-x86_64-compile-testing success Testing PASS

Commit Message

Yunjian Wang Sept. 18, 2021, 8:41 a.m. UTC 
  Normally when closing the device the queue memzone should be
freed. But the memzone will be not freed, when device setup
ops like:

rte_eth_bond_slave_remove
-->__eth_bond_slave_remove_lock_free
---->slave_remove
------>rte_eth_dev_internal_reset
-------->rte_eth_dev_rx_queue_config
---------->eth_dev_rx_queue_config
------------>ixgbe_dev_rx_queue_release
rte_eth_dev_close
-->ixgbe_dev_close
---->ixgbe_dev_free_queues
------>ixgbe_dev_rx_queue_release
      (not been called due to nb_rx_queues and nb_tx_queues are 0)

In order to free the memzone, we can release the memzone
when releasing queues.

Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
---
 drivers/net/ixgbe/ixgbe_rxtx.c | 6 ++++--
 drivers/net/ixgbe/ixgbe_rxtx.h | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

Comments

Wang, Haiyue Sept. 22, 2021, 6:22 a.m. UTC | #1 
> -----Original Message-----
> From: Yunjian Wang <wangyunjian@huawei.com>
> Sent: Saturday, September 18, 2021 16:42
> To: dev@dpdk.org
> Cc: Wang, Haiyue <haiyue.wang@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; dingxiaoxiong@huawei.com; Yunjian Wang
> <wangyunjian@huawei.com>
> Subject: [dpdk-dev] [PATCH v2 4/4] net/ixgbe: delete HW rings when releasing queues
> 
> Normally when closing the device the queue memzone should be
> freed. But the memzone will be not freed, when device setup
> ops like:
> 
> rte_eth_bond_slave_remove
> -->__eth_bond_slave_remove_lock_free
> ---->slave_remove
> ------>rte_eth_dev_internal_reset
> -------->rte_eth_dev_rx_queue_config
> ---------->eth_dev_rx_queue_config
> ------------>ixgbe_dev_rx_queue_release
> rte_eth_dev_close
> -->ixgbe_dev_close
> ---->ixgbe_dev_free_queues
> ------>ixgbe_dev_rx_queue_release
>       (not been called due to nb_rx_queues and nb_tx_queues are 0)
> 
> In order to free the memzone, we can release the memzone
> when releasing queues.
> 

After re-check the eth dev API, I think we can simplify the commit
message to such as:

Fix memzone leak when re-configure the RX/TX queues.

Please see 'rte_eth_dev_configure', when queue number is changed to
small size, the BIG memzone queue index will be lost. This will make
it is a MUST fix. ;-)

And add the Fixes tag and CC to stable.

What do you think ?

> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> ---
>  drivers/net/ixgbe/ixgbe_rxtx.c | 6 ++++--
>  drivers/net/ixgbe/ixgbe_rxtx.h | 2 ++
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index bfdfd5e755..1b6e0489f4 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -2482,6 +2482,7 @@ ixgbe_tx_queue_release(struct ixgbe_tx_queue *txq)
>  	if (txq != NULL && txq->ops != NULL) {
>  		txq->ops->release_mbufs(txq);
>  		txq->ops->free_swring(txq);
> +		rte_memzone_free(txq->mz);
>  		rte_free(txq);
>  	}
>  }
> @@ -2763,6 +2764,7 @@ ixgbe_dev_tx_queue_setup(struct rte_eth_dev *dev,
>  		return -ENOMEM;
>  	}
> 
> +	txq->mz = tz;
>  	txq->nb_tx_desc = nb_desc;
>  	txq->tx_rs_thresh = tx_rs_thresh;
>  	txq->tx_free_thresh = tx_free_thresh;
> @@ -2887,6 +2889,7 @@ ixgbe_rx_queue_release(struct ixgbe_rx_queue *rxq)
>  		ixgbe_rx_queue_release_mbufs(rxq);
>  		rte_free(rxq->sw_ring);
>  		rte_free(rxq->sw_sc_ring);
> +		rte_memzone_free(rxq->mz);
>  		rte_free(rxq);
>  	}
>  }
> @@ -3162,6 +3165,7 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
>  		return -ENOMEM;
>  	}
> 
> +	rxq->mz = rz;
>  	/*
>  	 * Zero init all the descriptors in the ring.
>  	 */
> @@ -3433,14 +3437,12 @@ ixgbe_dev_free_queues(struct rte_eth_dev *dev)
>  	for (i = 0; i < dev->data->nb_rx_queues; i++) {
>  		ixgbe_dev_rx_queue_release(dev->data->rx_queues[i]);
>  		dev->data->rx_queues[i] = NULL;
> -		rte_eth_dma_zone_free(dev, "rx_ring", i);
>  	}
>  	dev->data->nb_rx_queues = 0;
> 
>  	for (i = 0; i < dev->data->nb_tx_queues; i++) {
>  		ixgbe_dev_tx_queue_release(dev->data->tx_queues[i]);
>  		dev->data->tx_queues[i] = NULL;
> -		rte_eth_dma_zone_free(dev, "tx_ring", i);
>  	}
>  	dev->data->nb_tx_queues = 0;
>  }
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
> index 476ef62cfd..a1764f2b08 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.h
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.h
> @@ -138,6 +138,7 @@ struct ixgbe_rx_queue {
>  	struct rte_mbuf fake_mbuf;
>  	/** hold packets to return to application */
>  	struct rte_mbuf *rx_stage[RTE_PMD_IXGBE_RX_MAX_BURST*2];
> +	const struct rte_memzone *mz;
>  };
> 
>  /**
> @@ -236,6 +237,7 @@ struct ixgbe_tx_queue {
>  	uint8_t		    using_ipsec;
>  	/**< indicates that IPsec TX feature is in use */
>  #endif
> +	const struct rte_memzone *mz;
>  };
> 
>  struct ixgbe_txq_ops {
> --
> 2.23.0
Yunjian Wang Sept. 22, 2021, 6:58 a.m. UTC | #2 
> -----Original Message-----
> From: Wang, Haiyue [mailto:haiyue.wang@intel.com]
> Sent: Wednesday, September 22, 2021 2:23 PM
> To: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; dingxiaoxiong
> <dingxiaoxiong@huawei.com>
> Subject: RE: [dpdk-dev] [PATCH v2 4/4] net/ixgbe: delete HW rings when
> releasing queues
> 
> > -----Original Message-----
> > From: Yunjian Wang <wangyunjian@huawei.com>
> > Sent: Saturday, September 18, 2021 16:42
> > To: dev@dpdk.org
> > Cc: Wang, Haiyue <haiyue.wang@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Zhang,
> > Qi Z <qi.z.zhang@intel.com>; dingxiaoxiong@huawei.com; Yunjian Wang
> > <wangyunjian@huawei.com>
> > Subject: [dpdk-dev] [PATCH v2 4/4] net/ixgbe: delete HW rings when
> > releasing queues
> >
> > Normally when closing the device the queue memzone should be freed.
> > But the memzone will be not freed, when device setup ops like:
> >
> > rte_eth_bond_slave_remove
> > -->__eth_bond_slave_remove_lock_free
> > ---->slave_remove
> > ------>rte_eth_dev_internal_reset
> > -------->rte_eth_dev_rx_queue_config
> > ---------->eth_dev_rx_queue_config
> > ------------>ixgbe_dev_rx_queue_release
> > rte_eth_dev_close
> > -->ixgbe_dev_close
> > ---->ixgbe_dev_free_queues
> > ------>ixgbe_dev_rx_queue_release
> >       (not been called due to nb_rx_queues and nb_tx_queues are 0)
> >
> > In order to free the memzone, we can release the memzone when
> > releasing queues.
> >
> 
> After re-check the eth dev API, I think we can simplify the commit message to
> such as:
> 
> Fix memzone leak when re-configure the RX/TX queues.
> 
> Please see 'rte_eth_dev_configure', when queue number is changed to small
> size, the BIG memzone queue index will be lost. This will make it is a MUST
> fix. ;-)
> 
> And add the Fixes tag and CC to stable.
> 
> What do you think ?

OK, thanks for your suggestion, will include them in next version.

> 
> > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> > ---
> >  drivers/net/ixgbe/ixgbe_rxtx.c | 6 ++++--
> > drivers/net/ixgbe/ixgbe_rxtx.h | 2 ++
> >  2 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c
> > b/drivers/net/ixgbe/ixgbe_rxtx.c index bfdfd5e755..1b6e0489f4 100644
> > --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> > +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> > @@ -2482,6 +2482,7 @@ ixgbe_tx_queue_release(struct ixgbe_tx_queue
> *txq)
> >  	if (txq != NULL && txq->ops != NULL) {
> >  		txq->ops->release_mbufs(txq);
> >  		txq->ops->free_swring(txq);
> > +		rte_memzone_free(txq->mz);
> >  		rte_free(txq);
> >  	}
> >  }
> > @@ -2763,6 +2764,7 @@ ixgbe_dev_tx_queue_setup(struct rte_eth_dev
> *dev,
> >  		return -ENOMEM;
> >  	}
> >
> > +	txq->mz = tz;
> >  	txq->nb_tx_desc = nb_desc;
> >  	txq->tx_rs_thresh = tx_rs_thresh;
> >  	txq->tx_free_thresh = tx_free_thresh; @@ -2887,6 +2889,7 @@
> > ixgbe_rx_queue_release(struct ixgbe_rx_queue *rxq)
> >  		ixgbe_rx_queue_release_mbufs(rxq);
> >  		rte_free(rxq->sw_ring);
> >  		rte_free(rxq->sw_sc_ring);
> > +		rte_memzone_free(rxq->mz);
> >  		rte_free(rxq);
> >  	}
> >  }
> > @@ -3162,6 +3165,7 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev
> *dev,
> >  		return -ENOMEM;
> >  	}
> >
> > +	rxq->mz = rz;
> >  	/*
> >  	 * Zero init all the descriptors in the ring.
> >  	 */
> > @@ -3433,14 +3437,12 @@ ixgbe_dev_free_queues(struct rte_eth_dev *dev)
> >  	for (i = 0; i < dev->data->nb_rx_queues; i++) {
> >  		ixgbe_dev_rx_queue_release(dev->data->rx_queues[i]);
> >  		dev->data->rx_queues[i] = NULL;
> > -		rte_eth_dma_zone_free(dev, "rx_ring", i);
> >  	}
> >  	dev->data->nb_rx_queues = 0;
> >
> >  	for (i = 0; i < dev->data->nb_tx_queues; i++) {
> >  		ixgbe_dev_tx_queue_release(dev->data->tx_queues[i]);
> >  		dev->data->tx_queues[i] = NULL;
> > -		rte_eth_dma_zone_free(dev, "tx_ring", i);
> >  	}
> >  	dev->data->nb_tx_queues = 0;
> >  }
> > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h
> > b/drivers/net/ixgbe/ixgbe_rxtx.h index 476ef62cfd..a1764f2b08 100644
> > --- a/drivers/net/ixgbe/ixgbe_rxtx.h
> > +++ b/drivers/net/ixgbe/ixgbe_rxtx.h
> > @@ -138,6 +138,7 @@ struct ixgbe_rx_queue {
> >  	struct rte_mbuf fake_mbuf;
> >  	/** hold packets to return to application */
> >  	struct rte_mbuf *rx_stage[RTE_PMD_IXGBE_RX_MAX_BURST*2];
> > +	const struct rte_memzone *mz;
> >  };
> >
> >  /**
> > @@ -236,6 +237,7 @@ struct ixgbe_tx_queue {
> >  	uint8_t		    using_ipsec;
> >  	/**< indicates that IPsec TX feature is in use */  #endif
> > +	const struct rte_memzone *mz;
> >  };
> >
> >  struct ixgbe_txq_ops {
> > --
> > 2.23.0
Wang, Haiyue Sept. 22, 2021, 7:04 a.m. UTC | #3 
> -----Original Message-----
> From: wangyunjian <wangyunjian@huawei.com>
> Sent: Wednesday, September 22, 2021 14:58
> To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; dingxiaoxiong <dingxiaoxiong@huawei.com>
> Subject: RE: [dpdk-dev] [PATCH v2 4/4] net/ixgbe: delete HW rings when releasing queues
> 
> > -----Original Message-----
> > From: Wang, Haiyue [mailto:haiyue.wang@intel.com]
> > Sent: Wednesday, September 22, 2021 2:23 PM
> > To: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org
> > Cc: Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming
> > <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; dingxiaoxiong
> > <dingxiaoxiong@huawei.com>
> > Subject: RE: [dpdk-dev] [PATCH v2 4/4] net/ixgbe: delete HW rings when
> > releasing queues
> >
> > > -----Original Message-----
> > > From: Yunjian Wang <wangyunjian@huawei.com>
> > > Sent: Saturday, September 18, 2021 16:42
> > > To: dev@dpdk.org
> > > Cc: Wang, Haiyue <haiyue.wang@intel.com>; Xing, Beilei
> > > <beilei.xing@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Zhang,
> > > Qi Z <qi.z.zhang@intel.com>; dingxiaoxiong@huawei.com; Yunjian Wang
> > > <wangyunjian@huawei.com>
> > > Subject: [dpdk-dev] [PATCH v2 4/4] net/ixgbe: delete HW rings when
> > > releasing queues
> > >
> > > Normally when closing the device the queue memzone should be freed.
> > > But the memzone will be not freed, when device setup ops like:
> > >
> > > rte_eth_bond_slave_remove
> > > -->__eth_bond_slave_remove_lock_free
> > > ---->slave_remove
> > > ------>rte_eth_dev_internal_reset
> > > -------->rte_eth_dev_rx_queue_config
> > > ---------->eth_dev_rx_queue_config
> > > ------------>ixgbe_dev_rx_queue_release
> > > rte_eth_dev_close
> > > -->ixgbe_dev_close
> > > ---->ixgbe_dev_free_queues
> > > ------>ixgbe_dev_rx_queue_release
> > >       (not been called due to nb_rx_queues and nb_tx_queues are 0)
> > >
> > > In order to free the memzone, we can release the memzone when
> > > releasing queues.
> > >
> >
> > After re-check the eth dev API, I think we can simplify the commit message to
> > such as:
> >
> > Fix memzone leak when re-configure the RX/TX queues.
> >
> > Please see 'rte_eth_dev_configure', when queue number is changed to small
> > size, the BIG memzone queue index will be lost. This will make it is a MUST
> > fix. ;-)
> >
> > And add the Fixes tag and CC to stable.
> >
> > What do you think ?
> 
> OK, thanks for your suggestion, will include them in next version.
> 

Also, please fix the patch format warnings. You can get this
from patchwork CI.

FYI:

https://doc.dpdk.org/guides/contributing/patches.html

6.9. Checking the Patches

> >
> > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> > > ---
> > >  drivers/net/ixgbe/ixgbe_rxtx.c | 6 ++++--
> > > drivers/net/ixgbe/ixgbe_rxtx.h | 2 ++
> > >  2 files changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c
> > > b/drivers/net/ixgbe/ixgbe_rxtx.c index bfdfd5e755..1b6e0489f4 100644
> > > --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> > > +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> > > @@ -2482,6 +2482,7 @@ ixgbe_tx_queue_release(struct ixgbe_tx_queue
> > *txq)
> > >  	if (txq != NULL && txq->ops != NULL) {
> > >  		txq->ops->release_mbufs(txq);
> > >  		txq->ops->free_swring(txq);
> > > +		rte_memzone_free(txq->mz);
> > >  		rte_free(txq);
> > >  	}
> > >  }
> > > @@ -2763,6 +2764,7 @@ ixgbe_dev_tx_queue_setup(struct rte_eth_dev
> > *dev,
> > >  		return -ENOMEM;
> > >  	}
> > >
> > > +	txq->mz = tz;
> > >  	txq->nb_tx_desc = nb_desc;
> > >  	txq->tx_rs_thresh = tx_rs_thresh;
> > >  	txq->tx_free_thresh = tx_free_thresh; @@ -2887,6 +2889,7 @@
> > > ixgbe_rx_queue_release(struct ixgbe_rx_queue *rxq)
> > >  		ixgbe_rx_queue_release_mbufs(rxq);
> > >  		rte_free(rxq->sw_ring);
> > >  		rte_free(rxq->sw_sc_ring);
> > > +		rte_memzone_free(rxq->mz);
> > >  		rte_free(rxq);
> > >  	}
> > >  }
> > > @@ -3162,6 +3165,7 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev
> > *dev,
> > >  		return -ENOMEM;
> > >  	}
> > >
> > > +	rxq->mz = rz;
> > >  	/*
> > >  	 * Zero init all the descriptors in the ring.
> > >  	 */
> > > @@ -3433,14 +3437,12 @@ ixgbe_dev_free_queues(struct rte_eth_dev *dev)
> > >  	for (i = 0; i < dev->data->nb_rx_queues; i++) {
> > >  		ixgbe_dev_rx_queue_release(dev->data->rx_queues[i]);
> > >  		dev->data->rx_queues[i] = NULL;
> > > -		rte_eth_dma_zone_free(dev, "rx_ring", i);
> > >  	}
> > >  	dev->data->nb_rx_queues = 0;
> > >
> > >  	for (i = 0; i < dev->data->nb_tx_queues; i++) {
> > >  		ixgbe_dev_tx_queue_release(dev->data->tx_queues[i]);
> > >  		dev->data->tx_queues[i] = NULL;
> > > -		rte_eth_dma_zone_free(dev, "tx_ring", i);
> > >  	}
> > >  	dev->data->nb_tx_queues = 0;
> > >  }
> > > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h
> > > b/drivers/net/ixgbe/ixgbe_rxtx.h index 476ef62cfd..a1764f2b08 100644
> > > --- a/drivers/net/ixgbe/ixgbe_rxtx.h
> > > +++ b/drivers/net/ixgbe/ixgbe_rxtx.h
> > > @@ -138,6 +138,7 @@ struct ixgbe_rx_queue {
> > >  	struct rte_mbuf fake_mbuf;
> > >  	/** hold packets to return to application */
> > >  	struct rte_mbuf *rx_stage[RTE_PMD_IXGBE_RX_MAX_BURST*2];
> > > +	const struct rte_memzone *mz;
> > >  };
> > >
> > >  /**
> > > @@ -236,6 +237,7 @@ struct ixgbe_tx_queue {
> > >  	uint8_t		    using_ipsec;
> > >  	/**< indicates that IPsec TX feature is in use */  #endif
> > > +	const struct rte_memzone *mz;
> > >  };
> > >
> > >  struct ixgbe_txq_ops {
> > > --
> > > 2.23.0
Yunjian Wang Sept. 22, 2021, 11:13 a.m. UTC | #4 
> -----Original Message-----
> From: Wang, Haiyue [mailto:haiyue.wang@intel.com]
> Sent: Wednesday, September 22, 2021 3:05 PM
> To: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; dingxiaoxiong
> <dingxiaoxiong@huawei.com>
> Subject: RE: [dpdk-dev] [PATCH v2 4/4] net/ixgbe: delete HW rings when
> releasing queues
> 
> > -----Original Message-----
> > From: wangyunjian <wangyunjian@huawei.com>
> > Sent: Wednesday, September 22, 2021 14:58
> > To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org
> > Cc: Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming
> > <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> > dingxiaoxiong <dingxiaoxiong@huawei.com>
> > Subject: RE: [dpdk-dev] [PATCH v2 4/4] net/ixgbe: delete HW rings when
> > releasing queues
> >
> > > -----Original Message-----
> > > From: Wang, Haiyue [mailto:haiyue.wang@intel.com]
> > > Sent: Wednesday, September 22, 2021 2:23 PM
> > > To: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org
> > > Cc: Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming
> > > <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> > > dingxiaoxiong <dingxiaoxiong@huawei.com>
> > > Subject: RE: [dpdk-dev] [PATCH v2 4/4] net/ixgbe: delete HW rings
> > > when releasing queues
> > >
> > > > -----Original Message-----
> > > > From: Yunjian Wang <wangyunjian@huawei.com>
> > > > Sent: Saturday, September 18, 2021 16:42
> > > > To: dev@dpdk.org
> > > > Cc: Wang, Haiyue <haiyue.wang@intel.com>; Xing, Beilei
> > > > <beilei.xing@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> > > > Zhang, Qi Z <qi.z.zhang@intel.com>; dingxiaoxiong@huawei.com;
> > > > Yunjian Wang <wangyunjian@huawei.com>
> > > > Subject: [dpdk-dev] [PATCH v2 4/4] net/ixgbe: delete HW rings when
> > > > releasing queues
> > > >
> > > > Normally when closing the device the queue memzone should be freed.
> > > > But the memzone will be not freed, when device setup ops like:
> > > >
> > > > rte_eth_bond_slave_remove
> > > > -->__eth_bond_slave_remove_lock_free
> > > > ---->slave_remove
> > > > ------>rte_eth_dev_internal_reset
> > > > -------->rte_eth_dev_rx_queue_config
> > > > ---------->eth_dev_rx_queue_config
> > > > ------------>ixgbe_dev_rx_queue_release
> > > > rte_eth_dev_close
> > > > -->ixgbe_dev_close
> > > > ---->ixgbe_dev_free_queues
> > > > ------>ixgbe_dev_rx_queue_release
> > > >       (not been called due to nb_rx_queues and nb_tx_queues are 0)
> > > >
> > > > In order to free the memzone, we can release the memzone when
> > > > releasing queues.
> > > >
> > >
> > > After re-check the eth dev API, I think we can simplify the commit
> > > message to such as:
> > >
> > > Fix memzone leak when re-configure the RX/TX queues.
> > >
> > > Please see 'rte_eth_dev_configure', when queue number is changed to
> > > small size, the BIG memzone queue index will be lost. This will make
> > > it is a MUST fix. ;-)
> > >
> > > And add the Fixes tag and CC to stable.
> > >
> > > What do you think ?
> >
> > OK, thanks for your suggestion, will include them in next version.
> >
> 
> Also, please fix the patch format warnings. You can get this from patchwork CI.
> 
> FYI:
> 
> https://doc.dpdk.org/guides/contributing/patches.html
> 
> 6.9. Checking the Patches

Thanks, I've seen it. It should be a report incorrectly.

http://mails.dpdk.org/archives/test-report/2021-September/220735.html

Test-Label: checkpatch
Test-Status: WARNING
http://dpdk.org/patch/99273

_coding style issues_


WARNING:TYPO_SPELLING: 'slave' may be misspelled - perhaps 'secondary'?
#66: 
rte_eth_bond_slave_remove

WARNING:TYPO_SPELLING: 'slave' may be misspelled - perhaps 'secondary'?
#67: 
-->__eth_bond_slave_remove_lock_free

WARNING:TYPO_SPELLING: 'slave' may be misspelled - perhaps 'secondary'?
#68: 
---->slave_remove

total: 0 errors, 3 warnings, 0 checks, 56 lines checked

> 
> > >
> > > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> > > > ---
> > > >  drivers/net/ixgbe/ixgbe_rxtx.c | 6 ++++--
> > > > drivers/net/ixgbe/ixgbe_rxtx.h | 2 ++
> > > >  2 files changed, 6 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c
> > > > b/drivers/net/ixgbe/ixgbe_rxtx.c index bfdfd5e755..1b6e0489f4
> > > > 100644
> > > > --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> > > > +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> > > > @@ -2482,6 +2482,7 @@ ixgbe_tx_queue_release(struct ixgbe_tx_queue
> > > *txq)
> > > >  	if (txq != NULL && txq->ops != NULL) {
> > > >  		txq->ops->release_mbufs(txq);
> > > >  		txq->ops->free_swring(txq);
> > > > +		rte_memzone_free(txq->mz);
> > > >  		rte_free(txq);
> > > >  	}
> > > >  }
> > > > @@ -2763,6 +2764,7 @@ ixgbe_dev_tx_queue_setup(struct rte_eth_dev
> > > *dev,
> > > >  		return -ENOMEM;
> > > >  	}
> > > >
> > > > +	txq->mz = tz;
> > > >  	txq->nb_tx_desc = nb_desc;
> > > >  	txq->tx_rs_thresh = tx_rs_thresh;
> > > >  	txq->tx_free_thresh = tx_free_thresh; @@ -2887,6 +2889,7 @@
> > > > ixgbe_rx_queue_release(struct ixgbe_rx_queue *rxq)
> > > >  		ixgbe_rx_queue_release_mbufs(rxq);
> > > >  		rte_free(rxq->sw_ring);
> > > >  		rte_free(rxq->sw_sc_ring);
> > > > +		rte_memzone_free(rxq->mz);
> > > >  		rte_free(rxq);
> > > >  	}
> > > >  }
> > > > @@ -3162,6 +3165,7 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev
> > > *dev,
> > > >  		return -ENOMEM;
> > > >  	}
> > > >
> > > > +	rxq->mz = rz;
> > > >  	/*
> > > >  	 * Zero init all the descriptors in the ring.
> > > >  	 */
> > > > @@ -3433,14 +3437,12 @@ ixgbe_dev_free_queues(struct rte_eth_dev
> *dev)
> > > >  	for (i = 0; i < dev->data->nb_rx_queues; i++) {
> > > >  		ixgbe_dev_rx_queue_release(dev->data->rx_queues[i]);
> > > >  		dev->data->rx_queues[i] = NULL;
> > > > -		rte_eth_dma_zone_free(dev, "rx_ring", i);
> > > >  	}
> > > >  	dev->data->nb_rx_queues = 0;
> > > >
> > > >  	for (i = 0; i < dev->data->nb_tx_queues; i++) {
> > > >  		ixgbe_dev_tx_queue_release(dev->data->tx_queues[i]);
> > > >  		dev->data->tx_queues[i] = NULL;
> > > > -		rte_eth_dma_zone_free(dev, "tx_ring", i);
> > > >  	}
> > > >  	dev->data->nb_tx_queues = 0;
> > > >  }
> > > > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h
> > > > b/drivers/net/ixgbe/ixgbe_rxtx.h index 476ef62cfd..a1764f2b08
> > > > 100644
> > > > --- a/drivers/net/ixgbe/ixgbe_rxtx.h
> > > > +++ b/drivers/net/ixgbe/ixgbe_rxtx.h
> > > > @@ -138,6 +138,7 @@ struct ixgbe_rx_queue {
> > > >  	struct rte_mbuf fake_mbuf;
> > > >  	/** hold packets to return to application */
> > > >  	struct rte_mbuf *rx_stage[RTE_PMD_IXGBE_RX_MAX_BURST*2];
> > > > +	const struct rte_memzone *mz;
> > > >  };
> > > >
> > > >  /**
> > > > @@ -236,6 +237,7 @@ struct ixgbe_tx_queue {
> > > >  	uint8_t		    using_ipsec;
> > > >  	/**< indicates that IPsec TX feature is in use */  #endif
> > > > +	const struct rte_memzone *mz;
> > > >  };
> > > >
> > > >  struct ixgbe_txq_ops {
> > > > --
> > > > 2.23.0
Wang, Haiyue Sept. 22, 2021, 11:43 a.m. UTC | #5 
> -----Original Message-----
> From: wangyunjian <wangyunjian@huawei.com>
> Sent: Wednesday, September 22, 2021 19:14
> To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; dingxiaoxiong <dingxiaoxiong@huawei.com>
> Subject: RE: [dpdk-dev] [PATCH v2 4/4] net/ixgbe: delete HW rings when releasing queues
> 
> 
> 
> > -----Original Message-----
> > From: Wang, Haiyue [mailto:haiyue.wang@intel.com]
> > Sent: Wednesday, September 22, 2021 3:05 PM
> > To: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org
> > Cc: Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming
> > <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; dingxiaoxiong
> > <dingxiaoxiong@huawei.com>
> > Subject: RE: [dpdk-dev] [PATCH v2 4/4] net/ixgbe: delete HW rings when
> > releasing queues
> >
> > > -----Original Message-----
> > > From: wangyunjian <wangyunjian@huawei.com>
> > > Sent: Wednesday, September 22, 2021 14:58
> > > To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org
> > > Cc: Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming
> > > <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> > > dingxiaoxiong <dingxiaoxiong@huawei.com>
> > > Subject: RE: [dpdk-dev] [PATCH v2 4/4] net/ixgbe: delete HW rings when
> > > releasing queues
> > >
> > > > -----Original Message-----
> > > > From: Wang, Haiyue [mailto:haiyue.wang@intel.com]
> > > > Sent: Wednesday, September 22, 2021 2:23 PM
> > > > To: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org
> > > > Cc: Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming
> > > > <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> > > > dingxiaoxiong <dingxiaoxiong@huawei.com>
> > > > Subject: RE: [dpdk-dev] [PATCH v2 4/4] net/ixgbe: delete HW rings
> > > > when releasing queues
> > > >
> > > > > -----Original Message-----
> > > > > From: Yunjian Wang <wangyunjian@huawei.com>
> > > > > Sent: Saturday, September 18, 2021 16:42
> > > > > To: dev@dpdk.org
> > > > > Cc: Wang, Haiyue <haiyue.wang@intel.com>; Xing, Beilei
> > > > > <beilei.xing@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> > > > > Zhang, Qi Z <qi.z.zhang@intel.com>; dingxiaoxiong@huawei.com;
> > > > > Yunjian Wang <wangyunjian@huawei.com>
> > > > > Subject: [dpdk-dev] [PATCH v2 4/4] net/ixgbe: delete HW rings when
> > > > > releasing queues
> > > > >
> > > > > Normally when closing the device the queue memzone should be freed.
> > > > > But the memzone will be not freed, when device setup ops like:
> > > > >
> > > > > rte_eth_bond_slave_remove
> > > > > -->__eth_bond_slave_remove_lock_free
> > > > > ---->slave_remove
> > > > > ------>rte_eth_dev_internal_reset
> > > > > -------->rte_eth_dev_rx_queue_config
> > > > > ---------->eth_dev_rx_queue_config
> > > > > ------------>ixgbe_dev_rx_queue_release
> > > > > rte_eth_dev_close
> > > > > -->ixgbe_dev_close
> > > > > ---->ixgbe_dev_free_queues
> > > > > ------>ixgbe_dev_rx_queue_release
> > > > >       (not been called due to nb_rx_queues and nb_tx_queues are 0)
> > > > >
> > > > > In order to free the memzone, we can release the memzone when
> > > > > releasing queues.
> > > > >
> > > >
> > > > After re-check the eth dev API, I think we can simplify the commit
> > > > message to such as:
> > > >
> > > > Fix memzone leak when re-configure the RX/TX queues.
> > > >
> > > > Please see 'rte_eth_dev_configure', when queue number is changed to
> > > > small size, the BIG memzone queue index will be lost. This will make
> > > > it is a MUST fix. ;-)
> > > >
> > > > And add the Fixes tag and CC to stable.
> > > >
> > > > What do you think ?
> > >
> > > OK, thanks for your suggestion, will include them in next version.
> > >
> >
> > Also, please fix the patch format warnings. You can get this from patchwork CI.
> >
> > FYI:
> >
> > https://doc.dpdk.org/guides/contributing/patches.html
> >
> > 6.9. Checking the Patches
> 
> Thanks, I've seen it. It should be a report incorrectly.
> 
> http://mails.dpdk.org/archives/test-report/2021-September/220735.html
> 
> Test-Label: checkpatch
> Test-Status: WARNING
> http://dpdk.org/patch/99273
> 
> _coding style issues_
> 
> 
> WARNING:TYPO_SPELLING: 'slave' may be misspelled - perhaps 'secondary'?
> #66:
> rte_eth_bond_slave_remove
> 
> WARNING:TYPO_SPELLING: 'slave' may be misspelled - perhaps 'secondary'?
> #67:
> -->__eth_bond_slave_remove_lock_free
> 
> WARNING:TYPO_SPELLING: 'slave' may be misspelled - perhaps 'secondary'?
> #68:
> ---->slave_remove
> 
> total: 0 errors, 3 warnings, 0 checks, 56 lines checked
> 

Yes, report incorrectly. After changed to new commit message, this will
be gone. Thanks!

> >
> > > >
> > > > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> > > > > ---
> > > > >  drivers/net/ixgbe/ixgbe_rxtx.c | 6 ++++--
> > > > > drivers/net/ixgbe/ixgbe_rxtx.h | 2 ++
> > > > >  2 files changed, 6 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c
> > > > > b/drivers/net/ixgbe/ixgbe_rxtx.c index bfdfd5e755..1b6e0489f4
> > > > > 100644
> > > > > --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> > > > > +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> > > > > @@ -2482,6 +2482,7 @@ ixgbe_tx_queue_release(struct ixgbe_tx_queue
> > > > *txq)
> > > > >  	if (txq != NULL && txq->ops != NULL) {
> > > > >  		txq->ops->release_mbufs(txq);
> > > > >  		txq->ops->free_swring(txq);
> > > > > +		rte_memzone_free(txq->mz);
> > > > >  		rte_free(txq);
> > > > >  	}
> > > > >  }
> > > > > @@ -2763,6 +2764,7 @@ ixgbe_dev_tx_queue_setup(struct rte_eth_dev
> > > > *dev,
> > > > >  		return -ENOMEM;
> > > > >  	}
> > > > >
> > > > > +	txq->mz = tz;
> > > > >  	txq->nb_tx_desc = nb_desc;
> > > > >  	txq->tx_rs_thresh = tx_rs_thresh;
> > > > >  	txq->tx_free_thresh = tx_free_thresh; @@ -2887,6 +2889,7 @@
> > > > > ixgbe_rx_queue_release(struct ixgbe_rx_queue *rxq)
> > > > >  		ixgbe_rx_queue_release_mbufs(rxq);
> > > > >  		rte_free(rxq->sw_ring);
> > > > >  		rte_free(rxq->sw_sc_ring);
> > > > > +		rte_memzone_free(rxq->mz);
> > > > >  		rte_free(rxq);
> > > > >  	}
> > > > >  }
> > > > > @@ -3162,6 +3165,7 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev
> > > > *dev,
> > > > >  		return -ENOMEM;
> > > > >  	}
> > > > >
> > > > > +	rxq->mz = rz;
> > > > >  	/*
> > > > >  	 * Zero init all the descriptors in the ring.
> > > > >  	 */
> > > > > @@ -3433,14 +3437,12 @@ ixgbe_dev_free_queues(struct rte_eth_dev
> > *dev)
> > > > >  	for (i = 0; i < dev->data->nb_rx_queues; i++) {
> > > > >  		ixgbe_dev_rx_queue_release(dev->data->rx_queues[i]);
> > > > >  		dev->data->rx_queues[i] = NULL;
> > > > > -		rte_eth_dma_zone_free(dev, "rx_ring", i);
> > > > >  	}
> > > > >  	dev->data->nb_rx_queues = 0;
> > > > >
> > > > >  	for (i = 0; i < dev->data->nb_tx_queues; i++) {
> > > > >  		ixgbe_dev_tx_queue_release(dev->data->tx_queues[i]);
> > > > >  		dev->data->tx_queues[i] = NULL;
> > > > > -		rte_eth_dma_zone_free(dev, "tx_ring", i);
> > > > >  	}
> > > > >  	dev->data->nb_tx_queues = 0;
> > > > >  }
> > > > > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h
> > > > > b/drivers/net/ixgbe/ixgbe_rxtx.h index 476ef62cfd..a1764f2b08
> > > > > 100644
> > > > > --- a/drivers/net/ixgbe/ixgbe_rxtx.h
> > > > > +++ b/drivers/net/ixgbe/ixgbe_rxtx.h
> > > > > @@ -138,6 +138,7 @@ struct ixgbe_rx_queue {
> > > > >  	struct rte_mbuf fake_mbuf;
> > > > >  	/** hold packets to return to application */
> > > > >  	struct rte_mbuf *rx_stage[RTE_PMD_IXGBE_RX_MAX_BURST*2];
> > > > > +	const struct rte_memzone *mz;
> > > > >  };
> > > > >
> > > > >  /**
> > > > > @@ -236,6 +237,7 @@ struct ixgbe_tx_queue {
> > > > >  	uint8_t		    using_ipsec;
> > > > >  	/**< indicates that IPsec TX feature is in use */  #endif
> > > > > +	const struct rte_memzone *mz;
> > > > >  };
> > > > >
> > > > >  struct ixgbe_txq_ops {
> > > > > --
> > > > > 2.23.0
diff mbox series

Patch

diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index bfdfd5e755..1b6e0489f4 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -2482,6 +2482,7 @@  ixgbe_tx_queue_release(struct ixgbe_tx_queue *txq)
 	if (txq != NULL && txq->ops != NULL) {
 		txq->ops->release_mbufs(txq);
 		txq->ops->free_swring(txq);
+		rte_memzone_free(txq->mz);
 		rte_free(txq);
 	}
 }
@@ -2763,6 +2764,7 @@  ixgbe_dev_tx_queue_setup(struct rte_eth_dev *dev,
 		return -ENOMEM;
 	}
 
+	txq->mz = tz;
 	txq->nb_tx_desc = nb_desc;
 	txq->tx_rs_thresh = tx_rs_thresh;
 	txq->tx_free_thresh = tx_free_thresh;
@@ -2887,6 +2889,7 @@  ixgbe_rx_queue_release(struct ixgbe_rx_queue *rxq)
 		ixgbe_rx_queue_release_mbufs(rxq);
 		rte_free(rxq->sw_ring);
 		rte_free(rxq->sw_sc_ring);
+		rte_memzone_free(rxq->mz);
 		rte_free(rxq);
 	}
 }
@@ -3162,6 +3165,7 @@  ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
 		return -ENOMEM;
 	}
 
+	rxq->mz = rz;
 	/*
 	 * Zero init all the descriptors in the ring.
 	 */
@@ -3433,14 +3437,12 @@  ixgbe_dev_free_queues(struct rte_eth_dev *dev)
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		ixgbe_dev_rx_queue_release(dev->data->rx_queues[i]);
 		dev->data->rx_queues[i] = NULL;
-		rte_eth_dma_zone_free(dev, "rx_ring", i);
 	}
 	dev->data->nb_rx_queues = 0;
 
 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		ixgbe_dev_tx_queue_release(dev->data->tx_queues[i]);
 		dev->data->tx_queues[i] = NULL;
-		rte_eth_dma_zone_free(dev, "tx_ring", i);
 	}
 	dev->data->nb_tx_queues = 0;
 }
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
index 476ef62cfd..a1764f2b08 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.h
+++ b/drivers/net/ixgbe/ixgbe_rxtx.h
@@ -138,6 +138,7 @@  struct ixgbe_rx_queue {
 	struct rte_mbuf fake_mbuf;
 	/** hold packets to return to application */
 	struct rte_mbuf *rx_stage[RTE_PMD_IXGBE_RX_MAX_BURST*2];
+	const struct rte_memzone *mz;
 };
 
 /**
@@ -236,6 +237,7 @@  struct ixgbe_tx_queue {
 	uint8_t		    using_ipsec;
 	/**< indicates that IPsec TX feature is in use */
 #endif
+	const struct rte_memzone *mz;
 };
 
 struct ixgbe_txq_ops {