From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jianbo Liu <jianbo.liu@linaro.org>
Subject: Re: [PATCH] mbuf: extend rte_mbuf_prefetch_part* to
 support more prefetching methods
Date: Wed, 1 Jun 2016 11:29:47 +0800
Message-ID: <CAP4Qi3_PGM1kW4zGZVgitafZwKeyPP-60VrzGFAL4EyQEdjAtw@mail.gmail.com>
References: <1464663966-8122-1-git-send-email-jianbo.liu@linaro.org>
 <574DE5D6.2040001@6wind.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Cc: Jerin Jacob <jerin.jacob@caviumnetworks.com>, dev@dpdk.org
To: Olivier MATZ <olivier.matz@6wind.com>
Return-path: <dev-bounces@dpdk.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)
In-Reply-To: <574DE5D6.2040001@6wind.com>
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>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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