From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <jianbo.liu@linaro.org>
Received: from mail-yw0-f177.google.com (mail-yw0-f177.google.com
 [209.85.161.177]) by dpdk.org (Postfix) with ESMTP id D2B89137C
 for <dev@dpdk.org>; Wed,  1 Jun 2016 05:29:47 +0200 (CEST)
Received: by mail-yw0-f177.google.com with SMTP id h19so7039051ywc.0
 for <dev@dpdk.org>; Tue, 31 May 2016 20:29:47 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;
 h=mime-version:in-reply-to:references:date:message-id:subject:from:to
 :cc; bh=oOHsneVyJ7xT5gzZqcN/FJPb5Q9e0Puu43S2qfZoy7c=;
 b=F+WI/cTkZxXnhaDUHGfOZ2hM8hIV1Rgv99zXYHZBloVFwx5GrQRNY/2DwWBCEUgmwH
 KibKf0oQlM07GKkBqws6UL3Xq9xkF+c+pJ8eaXwfEqBkHmszsqOZMbvfZxY6vuqp0c7h
 Y93PXuad8rekEuCSQ6fwPsNBQnz/Nl7PmVgu8=
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20130820;
 h=x-gm-message-state:mime-version:in-reply-to:references:date
 :message-id:subject:from:to:cc;
 bh=oOHsneVyJ7xT5gzZqcN/FJPb5Q9e0Puu43S2qfZoy7c=;
 b=d70flZTrWvI53oxwRpuVGblA/8vtfEAMDTGYG2i7kxxD3Su1052w1+BlMCdPfCzzef
 or0M8r4kzOyQk1JpEXO+eypnUwvJukTSB5Fh9juoa2OM1TsxEjB1m6K+f/fZt7X9aGZH
 6G65CIgkzUpcdDeCNoVJluVQ7hb7Jh0rWIiyKv8HWpXnKgSOYIfV2Jtwfdld217rtkmU
 XHzIxM+zgKzpHH3i1NytpBA9Sfx/YGGd23ge9pW7oIWBFt16JUfCRpbXba73U/jJmv9U
 zlQovDZ4hl6pmxZro1GsdmUOyxBGhMqt+pjKHcs9LAagZtv6ljgHCsWMXUSC8P7ar/Fg
 dxNA==
X-Gm-Message-State: ALyK8tJ5nl7cV1+UqStAiumNDe/bhHR8jSUxFy14amKATwm3ALdMy3DicvMfzpTGdF3eU8nEQP5xT4BLzam2xJxP
MIME-Version: 1.0
X-Received: by 10.37.195.133 with SMTP id t127mr791101ybf.149.1464751787261;
 Tue, 31 May 2016 20:29:47 -0700 (PDT)
Received: by 10.37.212.146 with HTTP; Tue, 31 May 2016 20:29:47 -0700 (PDT)
In-Reply-To: <574DE5D6.2040001@6wind.com>
References: <1464663966-8122-1-git-send-email-jianbo.liu@linaro.org>
 <574DE5D6.2040001@6wind.com>
Date: Wed, 1 Jun 2016 11:29:47 +0800
Message-ID: <CAP4Qi3_PGM1kW4zGZVgitafZwKeyPP-60VrzGFAL4EyQEdjAtw@mail.gmail.com>
From: Jianbo Liu <jianbo.liu@linaro.org>
To: Olivier MATZ <olivier.matz@6wind.com>
Cc: Jerin Jacob <jerin.jacob@caviumnetworks.com>, dev@dpdk.org
Content-Type: text/plain; charset=UTF-8
Subject: Re: [dpdk-dev] [PATCH] mbuf: extend rte_mbuf_prefetch_part* to
 support more prefetching methods
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 01 Jun 2016 03:29:48 -0000

On 1 June 2016 at 03:28, Olivier MATZ <olivier.matz@6wind.com> wrote:
> Hi Jianbo,
>
> On 05/31/2016 05:06 AM, Jianbo Liu wrote:
>> Change the inline function to macro with parameters
>>
>> Signed-off-by: Jianbo Liu <jianbo.liu@linaro.org>
>>
>> [...]
>> --- a/lib/librte_mbuf/rte_mbuf.h
>> +++ b/lib/librte_mbuf/rte_mbuf.h
>> @@ -849,14 +849,15 @@ struct rte_mbuf {
>>   * in the receive path. If the cache line of the architecture is higher than
>>   * 64B, the second part will also be prefetched.
>>   *
>> + * @param method
>> + *   The prefetch method: prefetch0, prefetch1, prefetch2 or
>> + *                        prefetch_non_temporal.
>> + *
>>   * @param m
>>   *   The pointer to the mbuf.
>>   */
>> -static inline void
>> -rte_mbuf_prefetch_part1(struct rte_mbuf *m)
>> -{
>> -     rte_prefetch0(&m->cacheline0);
>> -}
>> +#define RTE_MBUF_PREFETCH_PART1(method, m)   \
>> +     rte_##method(&(m)->cacheline0)
>
> I'm not very fan of this macro, because it allows to
> really do everything):
>
>   RTE_MBUF_PREFETCH_PART1(pktmbuf_free, m)
>
> would expand as:
>
>   rte_pktmbuf_free(m)
>
>
> I'd prefer to have a switch case like this, almost similar
> to what Keith proposed in the initial discussion for my
> patch:
>
> enum rte_mbuf_prefetch_type {
>         PREFETCH0,
>         PREFETCH1,
> ...
> };
>
> static inline void
> rte_mbuf_prefetch_part1(enum rte_mbuf_prefetch_type type,
>         struct rte_mbuf *m)
> {
>         switch (type) {
>         case PREFETCH0:
>                 rte_prefetch0(&m->cacheline0);
>                 break;
>         case PREFETCH1:
>                 rte_prefetch1(&m->cacheline0);
>                 break;
>         ...
> }
>
How about adding these to forbid the illegal use of this macro?
enum rte_mbuf_prefetch_type {
         ENUM_prefetch0,
         ENUM_prefetch1,
 ...
};

#define RTE_MBUF_PREFETCH_PART1(type, m) \
    if (ENUM_##type == ENUM_prefretch0) \
        rte_prefetch0(&(m)->cacheline0);   \
    else if (ENUM_##type == ENUM_prefetch1) \
        rte_prefetch1(&(m)->cacheline0); \
    ....

>
> Some questions: could you give some details about the use
> of non-temporal prefetch in ixgbe_vec_neon? What are the
> pros and cons, and would it be useful in other drivers?
> Currently all drivers are doing prefetch0 when they prefetch
> the mbuf structure. Some drivers use prefetch1 for data.
>
It's for performance consideration, and only on armv8a platform.

>
> By the way, I did not try to apply the patch, but it looks
> it's on top of dpdk-next-net/rel_16_07, right?
>
Yes