From: Ferruh Yigit <ferruh.yigit@intel.com>
To: "Rafał Kozik" <rk@semihalf.com>
Cc: dev@dpdk.org, "Marcin Wojtas" <mw@semihalf.com>,
"Michał 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>
Subject: Re: [dpdk-dev] [PATCH v2 3/4] eal: enable WC during resources mapping
Date: Fri, 29 Jun 2018 10:05:59 +0100 [thread overview]
Message-ID: <2cb4219e-b354-0983-e56a-c9a14336b596@intel.com> (raw)
In-Reply-To: <CAMG3L4_01BvOyDeJ0FyZ6x1QUTCMOMo0TFEd8MM_Z_NipW7=jw@mail.gmail.com>
On 6/29/2018 9:58 AM, Rafał Kozik wrote:
> 2018-06-28 16:50 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 used by all PMDs.
>>>
>>> It will be enabled only if RTE_PCI_DRV_WC_ACTIVATE will be set in
>>> drivers flags. For proper work also igb_uio driver must be loaded with
>>> wc_activate set to 1.
>>>
>>> When mapping PCI resources, firstly try to us WC.
>>> If it is not supported it will fallback to normal mode.
>>>
>>> Signed-off-by: Rafal Kozik <rk@semihalf.com>
>>> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
>>> ---
>>> drivers/bus/pci/linux/pci_uio.c | 41 +++++++++++++++++++++++++++++------------
>>> drivers/bus/pci/rte_bus_pci.h | 2 ++
>>> 2 files changed, 31 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/bus/pci/linux/pci_uio.c b/drivers/bus/pci/linux/pci_uio.c
>>> index d423e4b..e3947c2 100644
>>> --- a/drivers/bus/pci/linux/pci_uio.c
>>> +++ b/drivers/bus/pci/linux/pci_uio.c
>>> @@ -282,22 +282,19 @@ int
>>> pci_uio_map_resource_by_index(struct rte_pci_device *dev, int res_idx,
>>> struct mapped_pci_resource *uio_res, int map_idx)
>>> {
>>> - int fd;
>>> + int fd = -1;
>>> char devname[PATH_MAX];
>>> void *mapaddr;
>>> struct rte_pci_addr *loc;
>>> struct pci_map *maps;
>>> + int wc_activate = 0;
>>> +
>>> + if (dev->driver != NULL)
>>> + wc_activate = dev->driver->drv_flags & RTE_PCI_DRV_WC_ACTIVATE;
>>>
>>> loc = &dev->addr;
>>> maps = uio_res->maps;
>>>
>>> - /* update devname for mmap */
>>> - snprintf(devname, sizeof(devname),
>>> - "%s/" PCI_PRI_FMT "/resource%d",
>>> - rte_pci_get_sysfs_path(),
>>> - loc->domain, loc->bus, loc->devid,
>>> - loc->function, res_idx);
>>> -
>>> /* allocate memory to keep path */
>>> maps[map_idx].path = rte_malloc(NULL, strlen(devname) + 1, 0);
>>> if (maps[map_idx].path == NULL) {
>>> @@ -309,11 +306,31 @@ pci_uio_map_resource_by_index(struct rte_pci_device *dev, int res_idx,
>>> /*
>>> * open resource file, to mmap it
>>> */
>>> - fd = open(devname, O_RDWR);
>>> - if (fd < 0) {
>>> - RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
>>> + if (wc_activate) {
>>> + /* update devname for mmap */
>>> + snprintf(devname, sizeof(devname),
>>> + "%s/" PCI_PRI_FMT "/resource%d_wc",
>>> + rte_pci_get_sysfs_path(),
>>> + loc->domain, loc->bus, loc->devid,
>>> + loc->function, res_idx);
>>> +
>>> + fd = open(devname, O_RDWR);
>>
>> What do you think adding an error log here? If opening resource$_wc fails and
>> fallback to resource# file, there won't be any way for user to know about it.
>>
>
> This error will be misleading for two reasons:
> * even if open return success, it could silently fall-back to non
> prefetchable mode,
> * NICs could have multiple BARs, but not all support WC. I such case
> fails will be desirable.
OK, perhaps not error log to prevent mislead, but what do you think "info" level
log, to notify the user that write combined version in not used.
>
>>> + }
>>> +
>>> + if (!wc_activate || fd < 0) {
>>> + snprintf(devname, sizeof(devname),
>>> + "%s/" PCI_PRI_FMT "/resource%d",
>>> + rte_pci_get_sysfs_path(),
>>> + loc->domain, loc->bus, loc->devid,
>>> + loc->function, res_idx);
>>> +
>>> + /* then try to map resource file */
>>> + fd = open(devname, O_RDWR);
>>> + if (fd < 0) {
>>> + RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
>>> devname, strerror(errno));
>>> - goto error;
>>> + goto error;
>>> + }
>>> }
>>>
>>> /* try mapping somewhere close to the end of hugepages */
>>> diff --git a/drivers/bus/pci/rte_bus_pci.h b/drivers/bus/pci/rte_bus_pci.h
>>> index 458e6d0..828acc5 100644
>>> --- a/drivers/bus/pci/rte_bus_pci.h
>>> +++ b/drivers/bus/pci/rte_bus_pci.h
>>> @@ -135,6 +135,8 @@ struct rte_pci_bus {
>>>
>>> /** Device needs PCI BAR mapping (done with either IGB_UIO or VFIO) */
>>> #define RTE_PCI_DRV_NEED_MAPPING 0x0001
>>> +/** Device needs PCI BAR mapping with enabled write combining (wc) */
>>> +#define RTE_PCI_DRV_WC_ACTIVATE 0x0002
>>> /** Device driver supports link state interrupt */
>>> #define RTE_PCI_DRV_INTR_LSC 0x0008
>>> /** Device driver supports device removal interrupt */
>>>
>>
next prev parent reply other threads:[~2018-06-29 9:06 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-11 14:07 [dpdk-dev] [PATCH 0/4] support for write combining Rafal Kozik
2018-04-11 14:07 ` [dpdk-dev] [PATCH 1/4] igb_uio: add wc option Rafal Kozik
2018-06-27 16:34 ` Ferruh Yigit
2018-06-28 13:08 ` Rafał Kozik
2018-06-28 13:15 ` [dpdk-dev] [PATCH v2 0/4] support for write combining Rafal Kozik
2018-06-28 13:15 ` [dpdk-dev] [PATCH v2 1/4] igb_uio: add wc option Rafal Kozik
2018-06-28 14:32 ` Ferruh Yigit
2018-06-29 8:35 ` Rafał Kozik
2018-06-29 10:08 ` Ferruh Yigit
2018-06-29 10:24 ` [dpdk-dev] [PATCH v3 0/4] support for write combining Rafal Kozik
2018-06-29 10:24 ` [dpdk-dev] [PATCH v3 1/4] igb_uio: add wc option Rafal Kozik
2018-06-29 10:53 ` Ferruh Yigit
2018-06-29 12:17 ` [dpdk-dev] [PATCH v4 0/4] Support for write combining Rafal Kozik
2018-06-29 12:17 ` [dpdk-dev] [PATCH v4 1/4] igb_uio: add wc option Rafal Kozik
2018-06-29 13:11 ` Rafał Kozik
2018-06-29 13:20 ` Ferruh Yigit
2018-06-29 13:40 ` Ferruh Yigit
2018-06-29 12:17 ` [dpdk-dev] [PATCH v4 2/4] bus/pci: reference driver structure Rafal Kozik
2018-06-29 12:17 ` [dpdk-dev] [PATCH v4 3/4] eal: enable WC during resources mapping Rafal Kozik
2018-06-29 12:17 ` [dpdk-dev] [PATCH v4 4/4] net/ena: enable WC Rafal Kozik
2018-06-29 13:54 ` [dpdk-dev] [PATCH v5 0/4] Support for write combining Rafal Kozik
2018-06-29 13:54 ` [dpdk-dev] [PATCH v5 1/4] igb_uio: add wc option Rafal Kozik
2018-06-29 13:54 ` [dpdk-dev] [PATCH v5 2/4] bus/pci: reference driver structure Rafal Kozik
2018-06-29 13:54 ` [dpdk-dev] [PATCH v5 3/4] eal: enable WC during resources mapping Rafal Kozik
2018-06-29 13:54 ` [dpdk-dev] [PATCH v5 4/4] net/ena: enable WC Rafal Kozik
2018-06-29 14:26 ` [dpdk-dev] [PATCH v5 0/4] Support for write combining Ferruh Yigit
2018-06-29 22:13 ` Thomas Monjalon
2018-06-29 10:24 ` [dpdk-dev] [PATCH v3 2/4] bus/pci: reference driver structure Rafal Kozik
2018-06-29 10:24 ` [dpdk-dev] [PATCH v3 3/4] eal: enable WC during resources mapping Rafal Kozik
2018-06-29 10:24 ` [dpdk-dev] [PATCH v3 4/4] net/ena: enable WC Rafal Kozik
2018-06-28 13:15 ` [dpdk-dev] [PATCH v2 2/4] bus/pci: reference driver structure Rafal Kozik
2018-06-28 13:15 ` [dpdk-dev] [PATCH v2 3/4] eal: enable WC during resources mapping Rafal Kozik
2018-06-28 14:50 ` Ferruh Yigit
2018-06-29 8:58 ` Rafał Kozik
2018-06-29 9:05 ` Ferruh Yigit [this message]
2018-06-29 10:28 ` Rafał Kozik
2018-06-29 10:37 ` Ferruh Yigit
2018-06-28 13:15 ` [dpdk-dev] [PATCH v2 4/4] net/ena: enable WC Rafal Kozik
2018-07-02 6:52 ` Michał Krawczyk
2018-04-11 14:07 ` [dpdk-dev] [PATCH 2/4] bus/pci: reference driver structure Rafal Kozik
2018-06-27 16:36 ` Ferruh Yigit
2018-06-28 13:05 ` Rafał Kozik
2018-04-11 14:07 ` [dpdk-dev] [PATCH 3/4] eal: enable WC during resources mapping Rafal Kozik
2018-06-27 16:41 ` Ferruh Yigit
2018-06-28 13:06 ` Rafał Kozik
2018-04-11 14:07 ` [dpdk-dev] [PATCH 4/4] net/ena: enable WC Rafal Kozik
2018-06-27 16:11 ` Thomas Monjalon
2018-06-28 13:04 ` Rafał Kozik
2018-04-11 14:42 ` [dpdk-dev] [PATCH 0/4] support for write combining Bruce Richardson
2018-04-16 11:36 ` Rafał Kozik
2018-04-27 8:27 ` Rafał Kozik
2018-04-27 11:30 ` Bruce Richardson
2018-04-30 8:07 ` Rafał Kozik
2018-04-30 8:58 ` Bruce Richardson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2cb4219e-b354-0983-e56a-c9a14336b596@intel.com \
--to=ferruh.yigit@intel.com \
--cc=dev@dpdk.org \
--cc=evgenys@amazon.com \
--cc=gtzalik@amazon.com \
--cc=igorch@amazon.com \
--cc=matua@amazon.com \
--cc=mk@semihalf.com \
--cc=mw@semihalf.com \
--cc=rk@semihalf.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).