From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <rk@semihalf.com>
Received: from mail-lj1-f194.google.com (mail-lj1-f194.google.com
 [209.85.208.194]) by dpdk.org (Postfix) with ESMTP id D7E138D4D
 for <dev@dpdk.org>; Fri, 29 Jun 2018 10:35:05 +0200 (CEST)
Received: by mail-lj1-f194.google.com with SMTP id l12-v6so6671046lja.1
 for <dev@dpdk.org>; Fri, 29 Jun 2018 01:35:05 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=semihalf-com.20150623.gappssmtp.com; s=20150623;
 h=mime-version:in-reply-to:references:from:date:message-id:subject:to
 :cc; bh=r5IE+2py6+Lg1GCWXXxg3Th5Tj9MN7TwZmNviEByWy8=;
 b=Q2XQpH04lUgnOaz/sP6QiQU+5eS5O907hOiCLbYt65btW3PdgRRNCaq2yzdQAfR/rU
 jGXtpq1x0IVwIgI6Fb9M1Uv1fKVSnB+pZzi12E/unx+cFGVXBMnKGmvaUaalrYtNcQZr
 BHUVTlBfjshKP3fI66TPZRaMcNLs0niA6qDOvIHVnPJl3vKA+6MOT2XJbgmG9k9z0qTg
 PaSyaZU7rTYJiK6CJvx/SqCvH7G61kHf8GGi6BWEE1KYt+UfHlcT0RsBBhJoBYT2C3mI
 UVyA4mpFrbEIxwX0yZUN8P7bJRYo7/cDdQ+IX42ZedJ2E9P6cl+LxO9xT+sV/JLLQ9VQ
 owOg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:mime-version:in-reply-to:references:from:date
 :message-id:subject:to:cc;
 bh=r5IE+2py6+Lg1GCWXXxg3Th5Tj9MN7TwZmNviEByWy8=;
 b=O3QtBnSi64qVAQlASj3jxN+hl5sr+5jLCLfsgTlCun9ZHtugLhLS7gLHpCTqVQwmIK
 uj1uzBHYaokqLq3VDKNfl8jKhmNYafuWAiQmmfcoHBnRPvpakLjCRlB9iPyjlQVMyd+W
 RMbLwYD/a15g3gRGso2P5pfzEU3Yfy43qfbfXgJLKTyuxtuIjjItwLOYVuw0EtFDP4mM
 BAtC/p5Of9uRLEcJu3aKEEc1Jc7WNKCv+nqQZ/WrOh8HnpvkNrWqDOo85g7pstLW53Mq
 KOS00LMiCGtTxhM5VYR07dvG4UksECc3JaDkGlwZzqZmRq65cvuHfuK23WIGXYIDotj+
 +9vQ==
X-Gm-Message-State: APt69E1IuEjyg3WOvAlls/n3xyRwGKD26iQ4zgzwI5jeMtcRFg+ckCvd
 705XNJvQxbIIjrNjMGmzDMHUGTTaBYyNKtAluSneeg==
X-Google-Smtp-Source: AAOMgpdqzsNWm2DChRJGJdhQnK0r2PUkpmSwMel6KU7vAm7YV0P0lvKpW3o+/DHzAChtGf6SFiRVoH46w63Wm4q++eg=
X-Received: by 2002:a2e:3a0e:: with SMTP id
 h14-v6mr7504386lja.20.1530261305400; 
 Fri, 29 Jun 2018 01:35:05 -0700 (PDT)
MIME-Version: 1.0
Received: by 2002:a19:4a95:0:0:0:0:0 with HTTP; Fri, 29 Jun 2018 01:35:04
 -0700 (PDT)
In-Reply-To: <356e2563-5b3e-510e-7a01-f735f6c33641@intel.com>
References: <CAMG3L49v=nNc9RNweJn4WCpuKeNfCk18o-xv2vq29AUo5CKc=g@mail.gmail.com>
 <1530191753-18689-1-git-send-email-rk@semihalf.com>
 <1530191753-18689-2-git-send-email-rk@semihalf.com>
 <356e2563-5b3e-510e-7a01-f735f6c33641@intel.com>
From: =?UTF-8?B?UmFmYcWCIEtvemlr?= <rk@semihalf.com>
Date: Fri, 29 Jun 2018 10:35:04 +0200
Message-ID: <CAMG3L49WS5ZsNZic=Wej_bPSdK7ca=Bw4EvSjXEnMgmAnLB9ww@mail.gmail.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Cc: dev@dpdk.org, Marcin Wojtas <mw@semihalf.com>,
 =?UTF-8?Q?Micha=C5=82_Krawczyk?= <mk@semihalf.com>, 
 "Tzalik, Guy" <gtzalik@amazon.com>, "Schmeilin, Evgeny" <evgenys@amazon.com>, 
 "Matushevsky, Alexander" <matua@amazon.com>, "Chauskin,
 Igor" <igorch@amazon.com>, Thomas Monjalon <thomas@monjalon.net>
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [dpdk-dev] [PATCH v2 1/4] igb_uio: add wc option
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
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>
X-List-Received-Date: Fri, 29 Jun 2018 08:35:06 -0000

2018-06-28 16:32 GMT+02:00 Ferruh Yigit <ferruh.yigit@intel.com>:
> On 6/28/2018 2:15 PM, Rafal Kozik wrote:
>> Write combining (WC) increases NIC performance by making better
>> utilization of PCI bus, but cannot be use by all PMDs.
>>
>> To get internal_addr memory need to be mapped. But as memory could not be
>> mapped twice: with and without WC, it should be skipped for WC. [1]
>>
>> To do not spoil other drivers that potentially could use internal_addr,
>> parameter wc_activate adds possibility to skip it for those PMDs,
>> that do not use it.
>>
>> [1] https://www.kernel.org/doc/ols/2008/ols2008v2-pages-135-144.pdf
>>       section 5.3 and 5.4
>
> Hi Rafal,
>
> Thank you for more information but I have a few more question:
>
> - What do you mean "But as memory could not be mapped twice: with and without WC"?
>
> ioremap() maps the physical address for kernel usage, and via uio we are mapping
> it to userspace, do you mean these two?
>
> - "internal_addr" is should be for kernel sage not for DPDK drivers which are in
> the userspace, why it is a concern for us?
>
> - What happens if you don't update this code at all? Won't you able to map
> device address into userspace?
> I tested adding RTE_PCI_DRV_WC_ACTIVATE to i40e, on top of your patch, and able
> to map without igb_uio update.
> I am not able to understand need of the modification.
>

Hello Ferruh,

I was not precisely. Memory could be mapped multiple time,
but cannot be mapped with and without WC support simultaneously.
When not setting wc_activate memory mapping work, but silently
fall-back to non prefetchable mode.

I perform measurements of writing speed.
When parameter wc_activate was set I get 4.81 GB/s.
Without this parameter result was 0.07 GB/s.
Code used for testing is located here:
gist.github.com/semihalf-kozik-rafal/327208cd52a2fac2d12250028becf9b3

Best regards,
Rafal

>>
>> Signed-off-by: Rafal Kozik <rk@semihalf.com>
>> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
>> ---
>>  kernel/linux/igb_uio/igb_uio.c | 17 ++++++++++++++---
>>  1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/linux/igb_uio/igb_uio.c b/kernel/linux/igb_uio/igb_uio.c
>> index b3233f1..3382fb1 100644
>> --- a/kernel/linux/igb_uio/igb_uio.c
>> +++ b/kernel/linux/igb_uio/igb_uio.c
>> @@ -30,6 +30,7 @@ struct rte_uio_pci_dev {
>>       int refcnt;
>>  };
>>
>> +static int wc_activate;
>>  static char *intr_mode;
>>  static enum rte_intr_mode igbuio_intr_mode_preferred = RTE_INTR_MODE_MSIX;
>>  /* sriov sysfs */
>> @@ -375,9 +376,13 @@ igbuio_pci_setup_iomem(struct pci_dev *dev, struct uio_info *info,
>>       len = pci_resource_len(dev, pci_bar);
>>       if (addr == 0 || len == 0)
>>               return -1;
>> -     internal_addr = ioremap(addr, len);
>> -     if (internal_addr == NULL)
>> -             return -1;
>> +     if (wc_activate == 0) {
>> +             internal_addr = ioremap(addr, len);
>> +             if (internal_addr == NULL)
>> +                     return -1;
>> +     } else {
>> +             internal_addr = NULL;
>> +     }
>>       info->mem[n].name = name;
>>       info->mem[n].addr = addr;
>>       info->mem[n].internal_addr = internal_addr;
>> @@ -650,6 +655,12 @@ MODULE_PARM_DESC(intr_mode,
>>  "    " RTE_INTR_MODE_LEGACY_NAME "     Use Legacy interrupt\n"
>>  "\n");
>>
>> +module_param(wc_activate, int, 0);
>> +MODULE_PARM_DESC(wc_activate,
>> +"Activate support for write combining (WC) (default=0)\n"
>> +"    0 - disable\n"
>> +"    other - enable\n");
>> +
>>  MODULE_DESCRIPTION("UIO driver for Intel IGB PCI cards");
>>  MODULE_LICENSE("GPL");
>>  MODULE_AUTHOR("Intel Corporation");
>>
>