From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pd0-f177.google.com (mail-pd0-f177.google.com [209.85.192.177]) by dpdk.org (Postfix) with ESMTP id 22918FFA for ; Tue, 7 Jul 2015 20:04:18 +0200 (CEST) Received: by pddu5 with SMTP id u5so42303531pdd.3 for ; Tue, 07 Jul 2015 11:04:17 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-type:content-transfer-encoding; bh=55K4Yf2fq7IahAcia/zwqKQII69X95TAWe/YzDNUgnQ=; b=gs8kcTbKOHOWwx4dVsI1MDgiBWZ1d+AAGcDnViLYexjVueBKMnF4bm0JxRx7QZaEvy djHDpI1g4z9r7jYaFewmeLPPsH/fZUMl9tb7doMN4UHUkHCL1JuvkpV/rbreDTbtI+pw ENClPlA16eaY6Q38zcJtv/kIdxSl+SkhBqxJASJiJA6r+zy7Dnw+/XgzW9l2oZyw0r0c UHVCFXQirangko4Cqg1IC6Oy4Q8tV6S3ULNoia173syMfbzp+mc/60fA17Vtdw2VvdUr ll/DDJD7ufrkuv5NPNF7eRn+Ioh8RzVd8zpT2ev6YjKZO6Tz6ItH4rgHB1Coq8FV8RTm 68AQ== X-Gm-Message-State: ALoCoQkaChvE07WS/k6VKvO1odg0emPawZiSaQcFra50XoMXK85qs0yd1QG8LcOiXv5xY9UXQDs3 X-Received: by 10.70.24.33 with SMTP id r1mr11563557pdf.74.1436292257353; Tue, 07 Jul 2015 11:04:17 -0700 (PDT) Received: from urahara (static-50-53-82-155.bvtn.or.frontiernet.net. [50.53.82.155]) by mx.google.com with ESMTPSA id ia3sm22547488pbc.31.2015.07.07.11.04.16 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 07 Jul 2015 11:04:16 -0700 (PDT) Date: Tue, 7 Jul 2015 11:04:26 -0700 From: Stephen Hemminger To: Bernard Iremonger Message-ID: <20150707110426.3529b9b6@urahara> In-Reply-To: <1436260687-28549-4-git-send-email-bernard.iremonger@intel.com> References: <1436260687-28549-1-git-send-email-bernard.iremonger@intel.com> <1436260687-28549-4-git-send-email-bernard.iremonger@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v4 3/4] virtio: free queue memory in virtio_dev_close() X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 07 Jul 2015 18:04:18 -0000 On Tue, 7 Jul 2015 10:18:06 +0100 Bernard Iremonger wrote: > Add function virtio_free_queues() and call from virtio_dev_close() > > Signed-off-by: Bernard Iremonger > --- > drivers/net/virtio/virtio_ethdev.c | 19 +++++++++++++++++++ > 1 files changed, 19 insertions(+), 0 deletions(-) > > diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c > index b88f297..ed99f33 100644 > --- a/drivers/net/virtio/virtio_ethdev.c > +++ b/drivers/net/virtio/virtio_ethdev.c > @@ -428,6 +428,24 @@ virtio_dev_cq_queue_setup(struct rte_eth_dev *dev, uint16_t vtpci_queue_idx, > } > > static void > +virtio_free_queues(struct rte_eth_dev *dev) > +{ > + unsigned int i; > + > + for (i = 0; i < dev->data->nb_rx_queues; i++) { > + rte_free(dev->data->rx_queues[i]); > + dev->data->rx_queues[i] = NULL; > + } > + dev->data->nb_rx_queues = 0; > + > + for (i = 0; i < dev->data->nb_tx_queues; i++) { > + rte_free(dev->data->tx_queues[i]); > + dev->data->tx_queues[i] = NULL; > + } > + dev->data->nb_tx_queues = 0; > +} > + > +static void > virtio_dev_close(struct rte_eth_dev *dev) > { > struct virtio_hw *hw = dev->data->dev_private; > @@ -441,6 +459,7 @@ virtio_dev_close(struct rte_eth_dev *dev) > vtpci_reset(hw); > hw->started = 0; > virtio_dev_free_mbufs(dev); > + virtio_free_queues(dev); > } > > static void I think virtio should implement regular queue free: Subject: virtio: add proper queue release When rx or tx queue is released, the virtio driver leaks memory because it never frees the associated queue structure. Signed-off-by: Stephen Hemminger --- a/lib/librte_pmd_virtio/virtio_ethdev.c +++ b/lib/librte_pmd_virtio/virtio_ethdev.c @@ -78,9 +78,6 @@ static int virtio_dev_link_update(struct static void virtio_set_hwaddr(struct virtio_hw *hw); static void virtio_get_hwaddr(struct virtio_hw *hw); -static void virtio_dev_rx_queue_release(__rte_unused void *rxq); -static void virtio_dev_tx_queue_release(__rte_unused void *txq); - static void virtio_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats); static void virtio_dev_stats_reset(struct rte_eth_dev *dev); static void virtio_dev_free_mbufs(struct rte_eth_dev *dev); @@ -389,6 +386,19 @@ int virtio_dev_queue_setup(struct rte_et return 0; } + +void +virtio_dev_queue_release(struct virtqueue *vq) +{ + struct virtio_hw *hw = vq->hw; + + /* Select and deactivate the queue */ + VIRTIO_WRITE_REG_2(hw, VIRTIO_PCI_QUEUE_SEL, vq->queue_id); + VIRTIO_WRITE_REG_4(hw, VIRTIO_PCI_QUEUE_PFN, 0); + + rte_free(vq); +} + static int virtio_dev_cq_queue_setup(struct rte_eth_dev *dev, uint16_t vtpci_queue_idx, uint32_t socket_id) @@ -536,10 +546,8 @@ static struct eth_dev_ops virtio_eth_dev .stats_reset = virtio_dev_stats_reset, .link_update = virtio_dev_link_update, .rx_queue_setup = virtio_dev_rx_queue_setup, - /* meaningfull only to multiple queue */ .rx_queue_release = virtio_dev_rx_queue_release, .tx_queue_setup = virtio_dev_tx_queue_setup, - /* meaningfull only to multiple queue */ .tx_queue_release = virtio_dev_tx_queue_release, /* collect stats per queue */ .queue_stats_mapping_set = virtio_dev_queue_stats_mapping_set, @@ -1278,19 +1286,6 @@ rte_virtio_pmd_init(const char *name __r } /* - * Only 1 queue is supported, no queue release related operation - */ -static void -virtio_dev_rx_queue_release(__rte_unused void *rxq) -{ -} - -static void -virtio_dev_tx_queue_release(__rte_unused void *txq) -{ -} - -/* * Configure virtio device * It returns 0 on success. */ --- a/lib/librte_pmd_virtio/virtio_ethdev.h +++ b/lib/librte_pmd_virtio/virtio_ethdev.h @@ -84,15 +84,21 @@ int virtio_dev_queue_setup(struct rte_et unsigned int socket_id, struct virtqueue **pvq); +void virtio_dev_queue_release(struct virtqueue *vq); + int virtio_dev_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id, uint16_t nb_rx_desc, unsigned int socket_id, const struct rte_eth_rxconf *rx_conf, struct rte_mempool *mb_pool); +void virtio_dev_rx_queue_release(void *rxq); + int virtio_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id, uint16_t nb_tx_desc, unsigned int socket_id, const struct rte_eth_txconf *tx_conf); +void virtio_dev_tx_queue_release(void *txq); + uint16_t virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts); --- a/lib/librte_pmd_virtio/virtio_rxtx.c +++ b/lib/librte_pmd_virtio/virtio_rxtx.c @@ -401,6 +401,13 @@ virtio_dev_rx_queue_setup(struct rte_eth return 0; } +void +virtio_dev_rx_queue_release(void *rxq) +{ + if (rxq) + virtio_dev_queue_release(rxq); +} + /* * struct rte_eth_dev *dev: Used to update dev * uint16_t nb_desc: Defaults to values read from config space @@ -455,6 +462,13 @@ virtio_dev_tx_queue_setup(struct rte_eth return 0; } +void +virtio_dev_tx_queue_release(void *txq) +{ + if (txq) + virtio_dev_queue_release(txq); +} + static void virtio_discard_rxbuf(struct virtqueue *vq, struct rte_mbuf *m) {