[v4,1/4] igb_uio: add wc option
Checks
Commit Message
From: Kozik <rk@semihalf.com>
Write combining (WC) increases NIC performance by making better
utilization of PCI bus, but cannot be use by all PMD.
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
Signed-off-by: Rafal Kozik <rk@semihalf.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
kernel/linux/igb_uio/igb_uio.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
Comments
> How can I confirm this silently fall-back behavior, is there any log can I turn
> on in kernel or anything from proc/sysfs?
I cannot find any.
I check it by measuring write speed.
2018-06-29 14:17 GMT+02:00 Rafal Kozik <rk@semihalf.com>:
> From: Kozik <rk@semihalf.com>
>
> Write combining (WC) increases NIC performance by making better
> utilization of PCI bus, but cannot be use by all PMD.
>
> 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
>
> Signed-off-by: Rafal Kozik <rk@semihalf.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> kernel/linux/igb_uio/igb_uio.c | 18 +++++++++++++++---
> 1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/linux/igb_uio/igb_uio.c b/kernel/linux/igb_uio/igb_uio.c
> index b3233f1..e16e760 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,14 @@ 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;
> + pr_info("wc_activate is set\n");
> + }
> info->mem[n].name = name;
> info->mem[n].addr = addr;
> info->mem[n].internal_addr = internal_addr;
> @@ -650,6 +656,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");
> --
> 2.7.4
>
On 6/29/2018 1:17 PM, Rafal Kozik wrote:
> From: Kozik <rk@semihalf.com>
>
> Write combining (WC) increases NIC performance by making better
> utilization of PCI bus, but cannot be use by all PMD.
>
> 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
>
> Signed-off-by: Rafal Kozik <rk@semihalf.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
(I am still not able to confirm this is needed, but since the behavior change is
controlled by module param and internal_addr is not used at all, no need to
block the patch)
On 6/29/2018 1:17 PM, Rafal Kozik wrote:
> From: Kozik <rk@semihalf.com>
>
> Write combining (WC) increases NIC performance by making better
> utilization of PCI bus, but cannot be use by all PMD.
>
> 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
>
> Signed-off-by: Rafal Kozik <rk@semihalf.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> kernel/linux/igb_uio/igb_uio.c | 18 +++++++++++++++---
> 1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/linux/igb_uio/igb_uio.c b/kernel/linux/igb_uio/igb_uio.c
> index b3233f1..e16e760 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,14 @@ 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;
> + pr_info("wc_activate is set\n");
This location has been reached during device probe, so this log has been printed
for each BAR of each device, which is unnecessary.
My intention was print the log once when wc_activate is set, similar to
"intr_mode", may bad that I point to wrong place,
Would you mind making one more version to fix this, you can keep the ack.
Thanks,
ferruh
> + }
> info->mem[n].name = name;
> info->mem[n].addr = addr;
> info->mem[n].internal_addr = internal_addr;
> @@ -650,6 +656,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,14 @@ 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;
+ pr_info("wc_activate is set\n");
+ }
info->mem[n].name = name;
info->mem[n].addr = addr;
info->mem[n].internal_addr = internal_addr;
@@ -650,6 +656,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");