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 6F7FDA0524;
Tue, 13 Apr 2021 14:37:11 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id 567ED160F18;
Tue, 13 Apr 2021 14:37:11 +0200 (CEST)
Received: from mga09.intel.com (mga09.intel.com [134.134.136.24])
by mails.dpdk.org (Postfix) with ESMTP id 1195B160F11;
Tue, 13 Apr 2021 14:37:08 +0200 (CEST)
IronPort-SDR: 9fTVWlBTaMaqFYK4/7lqIj0aIy5Y7VSK01Iv3vH6M8gpRunr1eYu2+gMzLCLAI2iB8u7uMkIzz
nk9suG8NHWKQ==
X-IronPort-AV: E=McAfee;i="6200,9189,9952"; a="194515437"
X-IronPort-AV: E=Sophos;i="5.82,219,1613462400"; d="scan'208";a="194515437"
Received: from orsmga001.jf.intel.com ([10.7.209.18])
by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
13 Apr 2021 05:37:02 -0700
IronPort-SDR: QZIE9wvYf0PJm3typHQ4peR5ZCNfrV1jedd7w/47+fn6RNYj6LWe5ohWrBW/x6VOrb1rNNZPMs
VfXm2GAhSeqA==
X-IronPort-AV: E=Sophos;i="5.82,219,1613462400"; d="scan'208";a="460567721"
Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.204.251])
([10.213.204.251])
by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
13 Apr 2021 05:37:01 -0700
To: Wenzhuo Lu <wenzhuo.lu@intel.com>, dev@dpdk.org,
Bruce Richardson <bruce.richardson@intel.com>
Cc: stable@dpdk.org
References: <1617074128-50681-1-git-send-email-wenzhuo.lu@intel.com>
<1617937317-130223-1-git-send-email-wenzhuo.lu@intel.com>
<1617937317-130223-2-git-send-email-wenzhuo.lu@intel.com>
From: Ferruh Yigit <ferruh.yigit@intel.com>
X-User: ferruhy
Message-ID: <a12a973a-2e8a-7048-4f8b-4e7bc233ea08@intel.com>
Date: Tue, 13 Apr 2021 13:37:00 +0100
MIME-Version: 1.0
In-Reply-To: <1617937317-130223-2-git-send-email-wenzhuo.lu@intel.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Subject: Re: [dpdk-dev] [PATCH v4 1/3] net/iavf: fix segment fault in AVX512
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
Sender: "dev" <dev-bounces@dpdk.org>
On 4/9/2021 4:01 AM, Wenzhuo Lu wrote:
> Fix segment fault when failing to get the memory from the pool.
>
Can be good to clarify there is no change in the moved code, it is not possible
to recognize this from patch without using a compare app.
> Fixes: 31737f2b66fb ("net/iavf: enable AVX512 for legacy Rx")
> Cc: stable@dpdk.org
>
> Reported-by: David Coyle <David.Coyle@intel.com>
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
> drivers/net/iavf/iavf_rxtx_vec_avx2.c | 120 +------------------
> drivers/net/iavf/iavf_rxtx_vec_avx512.c | 5 +-
> drivers/net/iavf/iavf_rxtx_vec_common.h | 203 ++++++++++++++++++++++++++++++++
The common vector code seems moved to 'iavf_rxtx_vec_common.h' but that header
is included by 'iavf_rxtx_vec_sse.c' too, and the moved function has AVX2 code
in it.
Won't this fail to build if the AVX2 is not enabled?
Bruce, is there an easy way to test this, meson detects the AVX2 support even I
provide c_args march that doesn't have AVX2 support.
<...>
> index 46a1873..57b4381 100644
> --- a/drivers/net/iavf/iavf_rxtx_vec_common.h
> +++ b/drivers/net/iavf/iavf_rxtx_vec_common.h
> @@ -11,6 +11,10 @@
> #include "iavf.h"
> #include "iavf_rxtx.h"
>
> +#ifndef __INTEL_COMPILER
> +#pragma GCC diagnostic ignored "-Wcast-qual"
> +#endif
> +
Is this pragma needed or carried here to be sure?
> static inline uint16_t
> reassemble_packets(struct iavf_rx_queue *rxq, struct rte_mbuf **rx_bufs,
> uint16_t nb_bufs, uint8_t *split_flags)
> @@ -276,4 +280,203 @@
> return 0;
> }
>
> +#ifdef RTE_ARCH_X86
> +static __rte_always_inline void
> +iavf_rxq_rearm_cmn(struct iavf_rx_queue *rxq, __rte_unused bool avx512)
What do you think expand 'cmn' to full 'common', it is clear from this patch
what it stands for but later if you just look this function it is not that clear
if it is an abbreviation for something else or common.