On 6/26/2015 7:02 PM, Iremonger, Bernard wrote: >> -----Original Message----- >> From: Qiu, Michael >> Sent: Friday, June 26, 2015 9:30 AM >> To: dev at dpdk.org >> Cc: Chen, Jing D; He, Shaopeng; Iremonger, Bernard; Qiu, Michael >> Subject: [PATCH 1/2 v4] fm10k: Free queues when close port >> >> When close a port, lots of memory should be released, such as software >> rings, queues, etc. >> >> Signed-off-by: Michael Qiu <michael.qiu at intel.com> >> --- > Hi Michael, > > There are 2 comments inline > >> drivers/net/fm10k/fm10k_ethdev.c | 37 >> +++++++++++++++++++++++++++++++++---- >> 1 file changed, 33 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/fm10k/fm10k_ethdev.c >> b/drivers/net/fm10k/fm10k_ethdev.c >> index 406c350..eba7228 100644 >> --- a/drivers/net/fm10k/fm10k_ethdev.c >> +++ b/drivers/net/fm10k/fm10k_ethdev.c >> @@ -65,6 +65,8 @@ static void >> fm10k_MAC_filter_set(struct rte_eth_dev *dev, const u8 *mac, bool add); >> static void fm10k_MACVLAN_remove_all(struct rte_eth_dev *dev); >> +static void fm10k_tx_queue_release(void *queue); static void >> +fm10k_rx_queue_release(void *queue); >> >> static void >> fm10k_mbx_initlock(struct fm10k_hw *hw) @@ -809,11 +811,37 @@ >> fm10k_dev_stop(struct rte_eth_dev *dev) >> >> PMD_INIT_FUNC_TRACE(); >> >> - for (i = 0; i < dev->data->nb_tx_queues; i++) >> - fm10k_dev_tx_queue_stop(dev, i); >> + if (dev->data->tx_queues) >> + for (i = 0; i < dev->data->nb_tx_queues; i++) >> + fm10k_dev_tx_queue_stop(dev, i); >> >> - for (i = 0; i < dev->data->nb_rx_queues; i++) >> - fm10k_dev_rx_queue_stop(dev, i); >> + if (dev->data->rx_queues) >> + for (i = 0; i < dev->data->nb_rx_queues; i++) >> + fm10k_dev_rx_queue_stop(dev, i); >> +} >> + >> +static void >> +fm10k_dev_queue_release(struct rte_eth_dev *dev) { >> + int i; >> + >> + PMD_INIT_FUNC_TRACE(); >> + >> + if (dev->data->tx_queues) { >> + for (i = 0; i < dev->data->nb_tx_queues; i++) >> + fm10k_tx_queue_release(dev->data- >>> tx_queues[i]); >> + rte_free(dev->data->tx_queues); >> + dev->data->tx_queues = NULL; > The memory for dev->data->tx_queues is not allocated in the fm10k PMD, > so it should probably not be released here. > I have submitted a patch today for rte_eth_dev.c to do this. > /dev/patchwork/patch/5829/ > >> + dev->data->nb_tx_queues = 0; >> + } >> + >> + if (dev->data->rx_queues) { >> + for (i = 0; i < dev->data->nb_rx_queues; i++) >> + fm10k_rx_queue_release(dev->data- >>> rx_queues[i]); >> + rte_free(dev->data->rx_queues); >> + dev->data->rx_queues = NULL; > The memory for dev->data->rx_queues is not allocated in the fm10k PMD, > so it should probably not be released here. > I have submitted a patch today for rte_eth_dev.c to do this. > /dev/patchwork/patch/5829/
Is it a good idea? What about to close the port for twice at a time? I think it is better to do it in rte_eth_dev_close(), I will give the comments to you. Thanks, Michael > Regards, > > Bernard. > > >> + dev->data->nb_rx_queues = 0; >> + } >> } >> >> static void >> @@ -828,6 +856,7 @@ fm10k_dev_close(struct rte_eth_dev *dev) >> /* Stop mailbox service first */ >> fm10k_close_mbx_service(hw); >> fm10k_dev_stop(dev); >> + fm10k_dev_queue_release(dev); >> fm10k_stop_hw(hw); >> } >> >> -- >> 1.9.3 >