[dpdk-dev,3/4] eal: enable WC during resources mapping
Checks
Commit Message
Write combining (wc) increase NIC performenca by making better
utilization of PCI bus, but cannot be used by all PMD.
It will be enable only if RTE_PCI_DRV_WC_ACTIVATE will be set in
drivers flags. For proper work also igb 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>
---
drivers/bus/pci/linux/pci_uio.c | 39 ++++++++++++++++++++++++++++-----------
drivers/bus/pci/rte_bus_pci.h | 2 ++
2 files changed, 30 insertions(+), 11 deletions(-)
Comments
On 4/11/2018 3:07 PM, Rafal Kozik wrote:
> Write combining (wc) increase NIC performenca by making better
> utilization of PCI bus, but cannot be used by all PMD.
>
> It will be enable only if RTE_PCI_DRV_WC_ACTIVATE will be set in
> drivers flags. For proper work also igb 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>
> ---
> drivers/bus/pci/linux/pci_uio.c | 39 ++++++++++++++++++++++++++++-----------
> drivers/bus/pci/rte_bus_pci.h | 2 ++
> 2 files changed, 30 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/bus/pci/linux/pci_uio.c b/drivers/bus/pci/linux/pci_uio.c
> index d423e4b..a1570c9 100644
> --- a/drivers/bus/pci/linux/pci_uio.c
> +++ b/drivers/bus/pci/linux/pci_uio.c
> @@ -287,17 +287,14 @@ pci_uio_map_resource_by_index(struct rte_pci_device *dev, int res_idx,
> 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);
> + }
> +
> + if (!wc_activate || fd < 0) {
"fd" may be used uninitialized here, although its value doesn't affect the
branch taken it can be good to initialize it for any possible static analyzer
warning.
> + 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 357afb9..b7bcce3 100644
> --- a/drivers/bus/pci/rte_bus_pci.h
> +++ b/drivers/bus/pci/rte_bus_pci.h
> @@ -132,6 +132,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 */
>
2018-06-27 18:41 GMT+02:00 Ferruh Yigit <ferruh.yigit@intel.com>:
> On 4/11/2018 3:07 PM, Rafal Kozik wrote:
>> Write combining (wc) increase NIC performenca by making better
>> utilization of PCI bus, but cannot be used by all PMD.
>>
>> It will be enable only if RTE_PCI_DRV_WC_ACTIVATE will be set in
>> drivers flags. For proper work also igb 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>
>> ---
>> drivers/bus/pci/linux/pci_uio.c | 39 ++++++++++++++++++++++++++++-----------
>> drivers/bus/pci/rte_bus_pci.h | 2 ++
>> 2 files changed, 30 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/bus/pci/linux/pci_uio.c b/drivers/bus/pci/linux/pci_uio.c
>> index d423e4b..a1570c9 100644
>> --- a/drivers/bus/pci/linux/pci_uio.c
>> +++ b/drivers/bus/pci/linux/pci_uio.c
>> @@ -287,17 +287,14 @@ pci_uio_map_resource_by_index(struct rte_pci_device *dev, int res_idx,
>> 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);
>> + }
>> +
>> + if (!wc_activate || fd < 0) {
>
> "fd" may be used uninitialized here, although its value doesn't affect the
> branch taken it can be good to initialize it for any possible static analyzer
> warning.
>
I will fix it in new, rebased patch set.
>> + 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 357afb9..b7bcce3 100644
>> --- a/drivers/bus/pci/rte_bus_pci.h
>> +++ b/drivers/bus/pci/rte_bus_pci.h
>> @@ -132,6 +132,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 */
>>
>
@@ -287,17 +287,14 @@ pci_uio_map_resource_by_index(struct rte_pci_device *dev, int res_idx,
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);
+ }
+
+ 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 */
@@ -132,6 +132,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 */