From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id E7FBDA04B5; Wed, 28 Oct 2020 21:34:07 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9CD2F4C9B; Wed, 28 Oct 2020 21:34:05 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by dpdk.org (Postfix) with ESMTP id C5619378B for ; Wed, 28 Oct 2020 21:34:04 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1603917243; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Opt4FvCCWAVpONUug/vDFUxRFcq7sP9eSOCUlKW5nq4=; b=HShj2yy+SuN06bSh6epHqdWnBwCo/44tG6Lf2iYqcTFiZCnn//kq1eP+Dhbb8auiM5cSss aJYi238SSwWJEU4cZduahKSUMz/FN5I2INsM+QDqlzLxEet4xQzoXdM3Lov7BjYpOI/2mY ZUjftaOXAKip0zQx9OoDHZcj7ImMdt0= Received: from mail-oo1-f71.google.com (mail-oo1-f71.google.com [209.85.161.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-4-yrhD2DipMnGSmQ-V-bo-1w-1; Wed, 28 Oct 2020 16:34:00 -0400 X-MC-Unique: yrhD2DipMnGSmQ-V-bo-1w-1 Received: by mail-oo1-f71.google.com with SMTP id w3so186701oov.6 for ; Wed, 28 Oct 2020 13:34:00 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding :content-language; bh=Opt4FvCCWAVpONUug/vDFUxRFcq7sP9eSOCUlKW5nq4=; b=Iw/kZl/Trh42fBsTUYDw18wiobR9nMBhoBGeQ/fUrA6q8Om3jh7W2/l9ouskR/NQVj ne28dKFR/Znt8SvQqTfQAtBYHiv/OSQp2FuJ2T86lUbUVe+S1bwmkFpA0Hdm6aIj2lsH fscRj/nW4S3++dOxJrjRhE2NS7z94HI83GI2jco9xced8COuwokfX2J7JseT0BhTZVtw IHYFCcarY2+g/ejFSsb0BYJ6nGCHBwdeUezicZ9OPERmpN3erjdxSWMAcOGaC1uG50MU hGlyly8SfxvLjY7ntsAR5WWMy2zSxUQde9CtJSIliTpqtse5gGSY+YCSRykC7y8QmKCy nYOA== X-Gm-Message-State: AOAM530RSz2d8pAaKaKCSIOuClP/k7Mp0gtNmB2u9xMSe197Frg8Xj6X RwQeZsxYeq1jnug714XxrWELPdLiNdRzIZo98R02Trf1B2lUYOLyTi0WG5jM8wYxRft61MJvaCo 8R/Y= X-Received: by 2002:a9d:5e14:: with SMTP id d20mr733852oti.107.1603917239800; Wed, 28 Oct 2020 13:33:59 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwKyM2y0tIyKnuYLHKfHZ7xWIA0ysA9XEjW1RC4o6m19wCO7ixQNv0uVKtRM1A9XC3YXaGOAQ== X-Received: by 2002:a9d:5e14:: with SMTP id d20mr733836oti.107.1603917239496; Wed, 28 Oct 2020 13:33:59 -0700 (PDT) Received: from trix.remote.csb (075-142-250-213.res.spectrum.com. [75.142.250.213]) by smtp.gmail.com with ESMTPSA id w16sm165673oth.8.2020.10.28.13.33.58 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 28 Oct 2020 13:33:58 -0700 (PDT) To: "Chautru, Nicolas" , "dev@dpdk.org" , "akhil.goyal@nxp.com" Cc: "david.marchand@redhat.com" References: <1603496581-35966-1-git-send-email-nicolas.chautru@intel.com> <1603496581-35966-4-git-send-email-nicolas.chautru@intel.com> <6863b163-cab4-7c60-750b-d96c7d59172c@redhat.com> From: Tom Rix Message-ID: Date: Wed, 28 Oct 2020 13:33:57 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=trix@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: en-US Subject: Re: [dpdk-dev] [PATCH v5 3/7] app/bbdev: include explicit HARQ preloading X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 10/26/20 9:50 AM, Chautru, Nicolas wrote: >> -----Original Message----- >> From: Tom Rix >> Sent: Monday, October 26, 2020 6:32 AM >> To: Chautru, Nicolas ; dev@dpdk.org; >> akhil.goyal@nxp.com >> Cc: david.marchand@redhat.com >> Subject: Re: [PATCH v5 3/7] app/bbdev: include explicit HARQ preloading >> >> >> On 10/23/20 4:42 PM, Nicolas Chautru wrote: >>> Run preloading explicitly for unit tests. Load each code block by >>> reusing existing input op then restore for the actual test. >>> >>> Signed-off-by: Nicolas Chautru >>> Acked-by: Liu Tianjiao >>> --- >>> app/test-bbdev/main.h | 1 + >>> app/test-bbdev/test_bbdev_perf.c | 51 >>> +++++++++++++++++++++------------------- >>> 2 files changed, 28 insertions(+), 24 deletions(-) >>> >>> diff --git a/app/test-bbdev/main.h b/app/test-bbdev/main.h index >>> fb3dec8..dc10a50 100644 >>> --- a/app/test-bbdev/main.h >>> +++ b/app/test-bbdev/main.h >>> @@ -17,6 +17,7 @@ >>> #define TEST_SKIPPED 1 >>> >>> #define MAX_BURST 512U >>> +#define MAX_OPS 1024U >> This #define is not consistently used. >> >> ex/ see retrieve_harq_ddr, the old 1024 is still being used. > Thanks I missed it. I will change this now. > >>> #define DEFAULT_BURST 32U >>> #define DEFAULT_OPS 64U >>> #define DEFAULT_ITER 6U >>> diff --git a/app/test-bbdev/test_bbdev_perf.c >>> b/app/test-bbdev/test_bbdev_perf.c >>> index b62848e..f30cbdb 100644 >>> --- a/app/test-bbdev/test_bbdev_perf.c >>> +++ b/app/test-bbdev/test_bbdev_perf.c >>> @@ -2513,20 +2513,20 @@ typedef int (test_case_function)(struct >> active_device *ad, >>> bool preload) >>> { >>> uint16_t j; >>> - int ret; >>> - uint32_t harq_offset = (uint32_t) queue_id * HARQ_INCR * 1024; >>> - struct rte_bbdev_op_data save_hc_in, save_hc_out; >>> - struct rte_bbdev_dec_op *ops_deq[MAX_BURST]; >>> + int deq; >>> + uint32_t harq_offset = (uint32_t) queue_id * HARQ_INCR * >> MAX_OPS; >>> + struct rte_bbdev_op_data save_hc_in[MAX_OPS], >> save_hc_out[MAX_OPS]; >>> + struct rte_bbdev_dec_op *ops_deq[MAX_OPS]; >>> uint32_t flags = ops[0]->ldpc_dec.op_flags; >>> bool mem_in = flags & >> RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_IN_ENABLE; >>> bool hc_in = flags & RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE; >>> bool mem_out = flags & >> RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_OUT_ENABLE; >>> bool hc_out = flags & >> RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE; >>> bool h_comp = flags & >> RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION; >>> - for (j = 0; j < n; ++j) { >>> - if ((mem_in || hc_in) && preload) { >>> - save_hc_in = ops[j]- >>> ldpc_dec.harq_combined_input; >>> - save_hc_out = ops[j]- >>> ldpc_dec.harq_combined_output; >>> + if ((mem_in || hc_in) && preload) { >>> + for (j = 0; j < n; ++j) { >>> + save_hc_in[j] = ops[j]- >>> ldpc_dec.harq_combined_input; >>> + save_hc_out[j] = ops[j]- >>> ldpc_dec.harq_combined_output; >>> ops[j]->ldpc_dec.op_flags = >>> >> RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_LOOPBACK + >> RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_OUT_ENABLE; >> >> flags are usually handled with bit operators, not arithmetic. >> >> this seems to be a general issue. > This is keeping same coding style as rest of file. So keeping as is. Ugh. Add to a cleanup list. > >>> @@ -2536,16 +2536,23 @@ typedef int (test_case_function)(struct >> active_device *ad, >>> ops[j]->ldpc_dec.harq_combined_output.offset = >>> harq_offset; >>> ops[j]->ldpc_dec.harq_combined_input.offset = 0; >>> - rte_bbdev_enqueue_ldpc_dec_ops(dev_id, queue_id, >>> - &ops[j], 1); >>> - ret = 0; >>> - while (ret == 0) >>> - ret = rte_bbdev_dequeue_ldpc_dec_ops( >>> - dev_id, queue_id, &ops_deq[j], 1); >>> + harq_offset += HARQ_INCR; >>> + } >>> + rte_bbdev_enqueue_ldpc_dec_ops(dev_id, queue_id, >> &ops[0], n); >> Add check the return is 'n' >>> + deq = 0; >>> + while (deq != n) >>> + deq += rte_bbdev_dequeue_ldpc_dec_ops( >>> + dev_id, queue_id, &ops_deq[deq], >>> + n - deq); >> Add check the return >= 0 > This cannot be <0. uint16_t ok Tom > >> Tom >> >>> + /* Restore the operations */ >>> + for (j = 0; j < n; ++j) { >>> ops[j]->ldpc_dec.op_flags = flags; >>> - ops[j]->ldpc_dec.harq_combined_input = >> save_hc_in; >>> - ops[j]->ldpc_dec.harq_combined_output = >> save_hc_out; >>> + ops[j]->ldpc_dec.harq_combined_input = >> save_hc_in[j]; >>> + ops[j]->ldpc_dec.harq_combined_output = >> save_hc_out[j]; >>> } >>> + } >>> + harq_offset = (uint32_t) queue_id * HARQ_INCR * MAX_OPS; >>> + for (j = 0; j < n; ++j) { >>> /* Adjust HARQ offset when we reach external DDR */ >>> if (mem_in || hc_in) >>> ops[j]->ldpc_dec.harq_combined_input.offset >>> @@ -3231,11 +3238,9 @@ typedef int (test_case_function)(struct >> active_device *ad, >>> mbuf_reset( >>> ops_enq[j]- >>> ldpc_dec.harq_combined_output.data); >>> } >>> - if (extDdr) { >>> - bool preload = i == (TEST_REPETITIONS - 1); >>> + if (extDdr) >>> preload_harq_ddr(tp->dev_id, queue_id, ops_enq, >>> - num_ops, preload); >>> - } >>> + num_ops, true); >>> start_time = rte_rdtsc_precise(); >>> >>> for (enq = 0, deq = 0; enq < num_ops;) { @@ -3362,11 >> +3367,9 @@ >>> typedef int (test_case_function)(struct active_device *ad, >>> mbuf_reset( >>> ops_enq[j]- >>> ldpc_dec.harq_combined_output.data); >>> } >>> - if (extDdr) { >>> - bool preload = i == (TEST_REPETITIONS - 1); >>> + if (extDdr) >>> preload_harq_ddr(tp->dev_id, queue_id, ops_enq, >>> - num_ops, preload); >>> - } >>> + num_ops, true); >>> start_time = rte_rdtsc_precise(); >>> >>> for (enq = 0, deq = 0; enq < num_ops;) {