From: "Rafał Kozik" <rk@semihalf.com>
To: Ferruh Yigit <ferruh.yigit@intel.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 12:28:48 +0200 [thread overview]
Message-ID: <CAMG3L4-MsNkM2A+Z4bnLdw2R2NheptAm4Lyw+JvfCV3R_PaHVQ@mail.gmail.com> (raw)
In-Reply-To: <2cb4219e-b354-0983-e56a-c9a14336b596@intel.com>
2018-06-29 11:05 GMT+02:00 Ferruh Yigit <ferruh.yigit@intel.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.
>
In new revision of patch set I add logging of device name.
For every resources it provides information if mapped with or without WC.
It looks like:
EAL: /sys/bus/pci/devices/0000:00:06.0/resource0 mapped
EAL: /sys/bus/pci/devices/0000:00:06.0/resource2_wc mapped
EAL: /sys/bus/pci/devices/0000:00:06.0/resource4 mapped
>>
>>>> + }
>>>> +
>>>> + 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 10:28 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
2018-06-29 10:28 ` Rafał Kozik [this message]
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=CAMG3L4-MsNkM2A+Z4bnLdw2R2NheptAm4Lyw+JvfCV3R_PaHVQ@mail.gmail.com \
--to=rk@semihalf.com \
--cc=dev@dpdk.org \
--cc=evgenys@amazon.com \
--cc=ferruh.yigit@intel.com \
--cc=gtzalik@amazon.com \
--cc=igorch@amazon.com \
--cc=matua@amazon.com \
--cc=mk@semihalf.com \
--cc=mw@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).