From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
by inbox.dpdk.org (Postfix) with ESMTP id 9174FA0C41;
Wed, 17 Nov 2021 08:50:23 +0100 (CET)
Received: from [217.70.189.124] (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id 5637741142;
Wed, 17 Nov 2021 08:50:23 +0100 (CET)
Received: from us-smtp-delivery-124.mimecast.com
(us-smtp-delivery-124.mimecast.com [170.10.133.124])
by mails.dpdk.org (Postfix) with ESMTP id 395B441141
for <dev@dpdk.org>; Wed, 17 Nov 2021 08:50:22 +0100 (CET)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
s=mimecast20190719; t=1637135421;
h=from:from:reply-to:subject:subject:date:date:message-id:message-id:
to:to:cc:cc:mime-version:mime-version:content-type:content-type:
in-reply-to:in-reply-to:references:references;
bh=e+sleGNwOKFtMlcWVEqqVoxZB+X8ZlKun5iSMoRJl/Q=;
b=WjIrZkHg5Nf412L4wC9NPlbrXGFyiZSvE+Q28CazV1KQuWHYDqjdvSGZ91wC931ZdTKqXS
kQPWj+CgNAGk9dRZrWCeNmL5I4layc0vi4fOAwW2JyiE9rEQurdif9GKKUiyzPpzfZ0+J7
z+wd1M6JIXwoAjs2DGatQ7/T1OuH1Qs=
Received: from mail-lf1-f72.google.com (mail-lf1-f72.google.com
[209.85.167.72]) by relay.mimecast.com with ESMTP with STARTTLS
(version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
us-mta-72-zQ3KwIncNomiomSFcj0L0w-1; Wed, 17 Nov 2021 02:50:19 -0500
X-MC-Unique: zQ3KwIncNomiomSFcj0L0w-1
Received: by mail-lf1-f72.google.com with SMTP id
i1-20020a056512318100b003ffd4e89eb9so922889lfe.19
for <dev@dpdk.org>; Tue, 16 Nov 2021 23:50:19 -0800 (PST)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20210112;
h=x-gm-message-state:mime-version:references:in-reply-to:from:date
:message-id:subject:to:cc;
bh=e+sleGNwOKFtMlcWVEqqVoxZB+X8ZlKun5iSMoRJl/Q=;
b=tlfoXmlnh/SByhWkPZbtm4aDkI8CK+o5F75CqAtCOPNI+7AN+h97mw6TQsGMI8d/tM
FkZ6BCo4j5lxIptUrTJ1pw78G3I5Y+Nb9bxoA7awWQg+kpfEczw5zOpzjzXOxQvtNdto
9u1LTrMD9PLAvkL3hnL23F8VWlgZpXD8S069IrAPEfWA/vVo3bq7YgGC1dWVmLVP639v
cbaGZvyxBQX7LgvZQG/p+YJUOfFUATr1kdbRNxCfPTRvhbmr6MMpCx/RWdbDr163Ud3c
UKQP1xgTfgISM2A1bPbHWOiL+Tek2iKQxSWJIL4rTPmHxdI5wJtmkpjOYhlJKSRUmihC
Xbjw==
X-Gm-Message-State: AOAM5301sCUlf+euWQCzg8zgv2YjelyGuxhEz30BBCjT61Hun8HlWfhu
GDwqxfw9MB0megjn+jBIxh3R7Lx9F4HUdMdjQgt7aBF2ghzjm1VUILt4JzKjkQzBXUCHAOsb8bD
QSf0UK+i3K7JTEGxVCQU=
X-Received: by 2002:ac2:42c6:: with SMTP id n6mr12790730lfl.553.1637135418276;
Tue, 16 Nov 2021 23:50:18 -0800 (PST)
X-Google-Smtp-Source: ABdhPJwMQ3xn2n0EiFIb7IXmoG7IbRGDSinRgG4u2DALnY+WPC9v64WmSmctU2c6YbDz/Q5Fj2MG7uGyZnNyrpPCX/I=
X-Received: by 2002:ac2:42c6:: with SMTP id n6mr12790707lfl.553.1637135417984;
Tue, 16 Nov 2021 23:50:17 -0800 (PST)
MIME-Version: 1.0
References: <20211115230328.20618-1-eagostini@nvidia.com>
In-Reply-To: <20211115230328.20618-1-eagostini@nvidia.com>
From: David Marchand <david.marchand@redhat.com>
Date: Wed, 17 Nov 2021 08:50:07 +0100
Message-ID: <CAJFAV8xE0T1Q1o3k97jVj04Wx+Sfr7P9Pko+HNJER6FtKpMkuw@mail.gmail.com>
Subject: Re: [PATCH v1] app/test-gpudev: improve tests' output
To: Elena Agostini <eagostini@nvidia.com>
Cc: dev <dev@dpdk.org>
Authentication-Results: relay.mimecast.com;
auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dmarchan@redhat.com
X-Mimecast-Spam-Score: 0
X-Mimecast-Originator: redhat.com
Content-Type: text/plain; charset="UTF-8"
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
On Mon, Nov 15, 2021 at 3:53 PM <eagostini@nvidia.com> wrote:
>
> From: Elena Agostini <eagostini@nvidia.com>
>
> Signed-off-by: Elena Agostini <eagostini@nvidia.com>
> ---
> app/test-gpudev/main.c | 90 ++++++++++++++++++++++++++----------------
> 1 file changed, 55 insertions(+), 35 deletions(-)
>
> diff --git a/app/test-gpudev/main.c b/app/test-gpudev/main.c
> index 1db3f69e10..e42133b3aa 100644
> --- a/app/test-gpudev/main.c
> +++ b/app/test-gpudev/main.c
> @@ -70,46 +70,51 @@ alloc_gpu_memory(uint16_t gpu_id)
> size_t buf_bytes = 1024;
> int ret;
>
> - printf("\n=======> TEST: Allocate GPU memory\n");
> + printf("\n=======> TEST: Allocate GPU memory\n\n");
>
> /* Alloc memory on GPU 0 */
> ptr_1 = rte_gpu_mem_alloc(gpu_id, buf_bytes);
> if (ptr_1 == NULL) {
> fprintf(stderr, "rte_gpu_mem_alloc GPU memory returned error\n");
> - return -1;
> + goto error;
> }
> - printf("GPU memory allocated at 0x%p %zdB\n", ptr_1, buf_bytes);
> + printf("GPU memory allocated at 0x%p size is %zd bytes\n", ptr_1, buf_bytes);
>
> ptr_2 = rte_gpu_mem_alloc(gpu_id, buf_bytes);
> if (ptr_2 == NULL) {
> fprintf(stderr, "rte_gpu_mem_alloc GPU memory returned error\n");
> - return -1;
> + goto error;
> }
> - printf("GPU memory allocated at 0x%p %zdB\n", ptr_2, buf_bytes);
> + printf("GPU memory allocated at 0x%p size is %zd bytes\n", ptr_2, buf_bytes);
>
> ret = rte_gpu_mem_free(gpu_id, (uint8_t *)(ptr_1)+0x700);
> if (ret < 0) {
> - printf("GPU memory 0x%p + 0x700 NOT freed because of memory address not recognized by driver\n", ptr_1);
> + printf("GPU memory 0x%p NOT freed: gpu driver didn't find this memory address internally.\n", (uint8_t *)(ptr_1)+0x700);
> } else {
> - fprintf(stderr, "rte_gpu_mem_free erroneously freed GPU memory 0x%p + 0x700\n", ptr_1);
> - return -1;
> + fprintf(stderr, "ERROR: rte_gpu_mem_free freed GPU memory 0x%p\n", (uint8_t *)(ptr_1)+0x700);
> + goto error;
> }
>
> ret = rte_gpu_mem_free(gpu_id, ptr_2);
> if (ret < 0) {
> fprintf(stderr, "rte_gpu_mem_free returned error %d\n", ret);
> - return -1;
> + goto error;
> }
> printf("GPU memory 0x%p freed\n", ptr_2);
>
> ret = rte_gpu_mem_free(gpu_id, ptr_1);
> if (ret < 0) {
> fprintf(stderr, "rte_gpu_mem_free returned error %d\n", ret);
> - return -1;
> + goto error;
> }
> printf("GPU memory 0x%p freed\n", ptr_1);
>
> + printf("\n=======> TEST: PASSED\n");
> return 0;
> +
> +error:
It was already the case before the patch, but this code leaks gpu
memory on error.
Other places in this file show the same type of leaks.
Please fix.
> + printf("\n=======> TEST: FAILED\n");
> + return -1;
> }
>
> static int
--
David Marchand
> + printf("\n=======> TEST: FAILED\n");
> + return -1;
> }
>
> static int
> @@ -162,25 +171,25 @@ create_update_comm_flag(uint16_t gpu_id)
> uint32_t set_val;
> uint32_t get_val;
>
> - printf("\n=======> TEST: Communication flag\n");
> + printf("\n=======> TEST: Communication flag\n\n");
>
> ret = rte_gpu_comm_create_flag(gpu_id, &devflag, RTE_GPU_COMM_FLAG_CPU);
> if (ret < 0) {
> fprintf(stderr, "rte_gpu_comm_create_flag returned error %d\n", ret);
> - return -1;
> + goto error;
> }
>
> set_val = 25;
> ret = rte_gpu_comm_set_flag(&devflag, set_val);
> if (ret < 0) {
> fprintf(stderr, "rte_gpu_comm_set_flag returned error %d\n", ret);
> - return -1;
> + goto error;
> }
>
> ret = rte_gpu_comm_get_flag_value(&devflag, &get_val);
> if (ret < 0) {
> fprintf(stderr, "rte_gpu_comm_get_flag_value returned error %d\n", ret);
> - return -1;
> + goto error;
> }
>
> printf("Communication flag value at 0x%p was set to %d and current value is %d\n", devflag.ptr, set_val, get_val);
> @@ -189,13 +198,13 @@ create_update_comm_flag(uint16_t gpu_id)
> ret = rte_gpu_comm_set_flag(&devflag, set_val);
> if (ret < 0) {
> fprintf(stderr, "rte_gpu_comm_set_flag returned error %d\n", ret);
> - return -1;
> + goto error;
> }
>
> ret = rte_gpu_comm_get_flag_value(&devflag, &get_val);
> if (ret < 0) {
> fprintf(stderr, "rte_gpu_comm_get_flag_value returned error %d\n", ret);
> - return -1;
> + goto error;
> }
>
> printf("Communication flag value at 0x%p was set to %d and current value is %d\n", devflag.ptr, set_val, get_val);
> @@ -203,10 +212,15 @@ create_update_comm_flag(uint16_t gpu_id)
> ret = rte_gpu_comm_destroy_flag(&devflag);
> if (ret < 0) {
> fprintf(stderr, "rte_gpu_comm_destroy_flags returned error %d\n", ret);
> - return -1;
> + goto error;
> }
>
> + printf("\n=======> TEST: PASSED\n");
> return 0;
> +
> +error:
> + printf("\n=======> TEST: FAILED\n");
> + return -1;
> }
>
> static int
> @@ -236,12 +250,12 @@ create_update_comm_list(uint16_t gpu_id)
> uint32_t num_comm_items = 1024;
> struct rte_mbuf *mbufs[10];
>
> - printf("\n=======> TEST: Communication list\n");
> + printf("\n=======> TEST: Communication list\n\n");
>
> comm_list = rte_gpu_comm_create_list(gpu_id, num_comm_items);
> if (comm_list == NULL) {
> fprintf(stderr, "rte_gpu_comm_create_list returned error %d\n", ret);
> - return -1;
> + goto error;
> }
>
> /**
> @@ -251,7 +265,7 @@ create_update_comm_list(uint16_t gpu_id)
> mbufs[i] = rte_zmalloc(NULL, sizeof(struct rte_mbuf), 0);
> if (mbufs[i] == NULL) {
> fprintf(stderr, "Failed to allocate fake mbufs in CPU memory.\n");
> - return -1;
> + goto error;
> }
>
> memset(mbufs[i], 0, sizeof(struct rte_mbuf));
> @@ -263,20 +277,21 @@ create_update_comm_list(uint16_t gpu_id)
> ret = rte_gpu_comm_populate_list_pkts(&(comm_list[0]), mbufs, 10);
> if (ret < 0) {
> fprintf(stderr, "rte_gpu_comm_populate_list_pkts returned error %d\n", ret);
> - return -1;
> + goto error;
> }
>
> ret = rte_gpu_comm_cleanup_list(&(comm_list[0]));
> if (ret == 0) {
> fprintf(stderr, "rte_gpu_comm_cleanup_list erroneously cleaned the list even if packets have not been consumed yet\n");
> - return -1;
> + goto error;
> }
> - fprintf(stderr, "rte_gpu_comm_cleanup_list correctly didn't clean up the packets because they have not been consumed yet\n");
> + printf("Communication list not cleaned because packets have not been consumed yet.\n");
>
> /**
> * Simulate a GPU tasks going through the packet list to consume
> * mbufs packets and release them
> */
> + printf("Consuming packets...\n");
> simulate_gpu_task(&(comm_list[0]), 10);
>
> /**
> @@ -286,20 +301,26 @@ create_update_comm_list(uint16_t gpu_id)
> ret = rte_gpu_comm_cleanup_list(&(comm_list[0]));
> if (ret < 0) {
> fprintf(stderr, "rte_gpu_comm_cleanup_list returned error %d\n", ret);
> - return -1;
> + goto error;
> }
>
> + printf("Communication list cleaned because packets have been consumed now.\n");
> +
> ret = rte_gpu_comm_destroy_list(comm_list, num_comm_items);
> if (ret < 0) {
> fprintf(stderr, "rte_gpu_comm_destroy_list returned error %d\n", ret);
> - return -1;
> + goto error;
> }
>
> for (i = 0; i < 10; i++)
> rte_free(mbufs[i]);
>
> - printf("\nCommunication list test passed!\n");
> + printf("\n=======> TEST: PASSED\n");
> return 0;
> +
> +error:
> + printf("\n=======> TEST: FAILED\n");
> + return -1;
> }
>
> int
> @@ -360,7 +381,6 @@ main(int argc, char **argv)
>
> /* clean up the EAL */
> rte_eal_cleanup();
> - printf("Bye...\n");
>
> return EXIT_SUCCESS;
> }
> --
> 2.17.1
>