Re: [dpdk-dev] [PATCH 1/4] igb_uio: add wc option

[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?

>  	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");
>