[dpdk-dev,1/4] igb_uio: add wc option
Checks
Commit Message
Write combining (wc) increase NIC performance by making better
utilization of PCI bus, but cannot be use by all PMD.
Parameter wc_activate add possibility to enable it for
those PMD that could support it.
Signed-off-by: Rafal Kozik <rk@semihalf.com>
---
kernel/linux/igb_uio/igb_uio.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
Comments
On 4/11/2018 3:07 PM, Rafal Kozik wrote:
> Write combining (wc) increase NIC performance by making better
> utilization of PCI bus, but cannot be use by all PMD.
>
> Parameter wc_activate add possibility to enable it for
> those PMD that could support it.
>
> Signed-off-by: Rafal Kozik <rk@semihalf.com>
Hi Rafal,
First, sorry for delayed comment.
> ---
> 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 4cae4dd..42e3b3f 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;
> + }
Why we need this update at all?
What is the relation between internal_address and write combined?
As far as I can see we are not using internal_addr at all, what do you observe
in write combined usage without this modification?
> info->mem[n].name = name;
> info->mem[n].addr = addr;
> info->mem[n].internal_addr = internal_addr;
> @@ -638,6 +643,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");
>
2018-06-27 18:34 GMT+02:00 Ferruh Yigit <ferruh.yigit@intel.com>:
> On 4/11/2018 3:07 PM, Rafal Kozik wrote:
>> Write combining (wc) increase NIC performance by making better
>> utilization of PCI bus, but cannot be use by all PMD.
>>
>> Parameter wc_activate add possibility to enable it for
>> those PMD that could support it.
>>
>> Signed-off-by: Rafal Kozik <rk@semihalf.com>
>
> Hi Rafal,
>
> First, sorry for delayed comment.
>
>> ---
>> 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 4cae4dd..42e3b3f 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;
>> + }
>
> Why we need this update at all?
> What is the relation between internal_address and write combined?
>
> As far as I can see we are not using internal_addr at all, what do you observe
> in write combined usage without this modification?
>
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]
WC does not work without this modification.
[1] https://www.kernel.org/doc/ols/2008/ols2008v2-pages-135-144.pdf
section 5.3 and 5.4
>> info->mem[n].name = name;
>> info->mem[n].addr = addr;
>> info->mem[n].internal_addr = internal_addr;
>> @@ -638,6 +643,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");
>>
>
@@ -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;
@@ -638,6 +643,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");