From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 5899F1B445 for ; Fri, 29 Jun 2018 12:37:34 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 29 Jun 2018 03:37:33 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,285,1526367600"; d="scan'208";a="62564905" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.83]) ([10.237.221.83]) by fmsmga002.fm.intel.com with ESMTP; 29 Jun 2018 03:37:31 -0700 To: =?UTF-8?Q?Rafa=c5=82_Kozik?= Cc: dev@dpdk.org, Marcin Wojtas , =?UTF-8?Q?Micha=c5=82_Krawczyk?= , "Tzalik, Guy" , "Schmeilin, Evgeny" , "Matushevsky, Alexander" , "Chauskin, Igor" , Thomas Monjalon References: <1530191753-18689-1-git-send-email-rk@semihalf.com> <1530191753-18689-4-git-send-email-rk@semihalf.com> <2cb4219e-b354-0983-e56a-c9a14336b596@intel.com> From: Ferruh Yigit Openpgp: preference=signencrypt Autocrypt: addr=ferruh.yigit@intel.com; prefer-encrypt=mutual; keydata= xsFNBFXZCFABEADCujshBOAaqPZpwShdkzkyGpJ15lmxiSr3jVMqOtQS/sB3FYLT0/d3+bvy qbL9YnlbPyRvZfnP3pXiKwkRoR1RJwEo2BOf6hxdzTmLRtGtwWzI9MwrUPj6n/ldiD58VAGQ +iR1I/z9UBUN/ZMksElA2D7Jgg7vZ78iKwNnd+vLBD6I61kVrZ45Vjo3r+pPOByUBXOUlxp9 GWEKKIrJ4eogqkVNSixN16VYK7xR+5OUkBYUO+sE6etSxCr7BahMPKxH+XPlZZjKrxciaWQb +dElz3Ab4Opl+ZT/bK2huX+W+NJBEBVzjTkhjSTjcyRdxvS1gwWRuXqAml/sh+KQjPV1PPHF YK5LcqLkle+OKTCa82OvUb7cr+ALxATIZXQkgmn+zFT8UzSS3aiBBohg3BtbTIWy51jNlYdy ezUZ4UxKSsFuUTPt+JjHQBvF7WKbmNGS3fCid5Iag4tWOfZoqiCNzxApkVugltxoc6rG2TyX CmI2rP0mQ0GOsGXA3+3c1MCdQFzdIn/5tLBZyKy4F54UFo35eOX8/g7OaE+xrgY/4bZjpxC1 1pd66AAtKb3aNXpHvIfkVV6NYloo52H+FUE5ZDPNCGD0/btFGPWmWRmkPybzColTy7fmPaGz cBcEEqHK4T0aY4UJmE7Ylvg255Kz7s6wGZe6IR3N0cKNv++O7QARAQABzSVGZXJydWggWWln aXQgPGZlcnJ1aC55aWdpdEBpbnRlbC5jb20+wsF+BBMBAgAoAhsDBgsJCAcDAgYVCAIJCgsE FgIDAQIeAQIXgAUCWZR3VQUJB33WBQAKCRD5M+tD3xNhH6DWEACVhEb8q1epPwZrUDoxzu7E TS1b8tmabOmnjXZRs6+EXgUVHkp2xxkCfDmL3pa5bC0G/74aJnWjNsdvE05V1cb4YK4kRQ62 FwDQ+hlrFrwFB3PtDZk1tpkzCRHvJgnIil+0MuEh32Y57ig6hy8yO8ql7Lohyrnpfk/nNpm4 jQGEF5qEeHcEFe1AZQlPHN/STno8NZSz2nl0b2cw+cujN1krmvB52Ah/2KugQ6pprVyrGrzB c34ZQO9OsmSjJlETCZk6EZzuhfe16iqBFbOSadi9sPcJRwaUQBid+xdFWl7GQ8qC3zNPibSF HmU43yBZUqJDZlhIcl6/cFpOSjv2sDWdtjEXTDn5y/0FsuY0mFE78ItC4kCTIVk17VZoywcd fmbbnwOSWzDq7hiUYuQGkIudJw5k/A1CMsyLkoUEGN3sLfsw6KASgS4XrrmPO4UVr3mH5bP1 yC7i1OVNpzvOxtahmzm481ID8sk72GC2RktTOHb0cX+qdoiMMfYgo3wRRDYCBt6YoGYUxF1p msjocXyqToKhhnFbXLaZlVfnQ9i2i8jsj9SKig+ewC2p3lkPj6ncye9q95bzhmUeJO6sFhJg Hiz6syOMg8yCcq60j07airybAuHIDNFWk0gaWAmtHZxLObZx2PVn2nv9kLYGohFekw0AOsIW ta++5m48dnCoAc7BTQRX1ky+ARAApzQNvXvE2q1LAS+Z+ni2R13Bb1cDS1ZYq1jgpR13+OKN ipzd8MPngRJilXxBaPTErhgzR0vGcNTYhjGMSyFIHVOoBq1VbP1a0Fi/NqWzJOowo/fDfgVy K4vuitc/gCJs+2se4hdZA4EQJxVlNM51lgYDNpjPGIA43MX15OLAip73+ho6NPBMuc5qse3X pAClNhBKfENRCWN428pi3WVkT+ABRTE0taxjJNP7bb+9TQYNRqGwnGzX5/XISv44asWIQCaq vOkXSUJLd//cdVNTqtL1wreCVVR5pMXj7VIrlk07fmmJVALCmGbFr53BMb8O+8dgK2A5mitM n44d+8KdJWOwziRxcaMk/LclmZS3Iv1TERtiWt98Y9AjeAtcgYPkA3ld0BcUKONogP8pHVz1 Ed3s5rDQ91yr1S0wuAzW91fxGUO4wY+uPmxCtFVuBgd9VT9NAKTUL0qHM7CDgCnZPe0TW6Zj 8OqtdCCyAfvU9cW5xWM7Icxhde6AtPxhDSBwE8fL2ZmrDmaA4jmUKXp3i4JxRPSX84S08b+s DWXHPxy10UFU5A7EK/BEbZAKBwn9ROfm+WK+6X5xOGLoRE++OqNuUudxC1GDyLOPaqCbBCS9 +P6HsTHzxsjyJa27n4jcrcuY3P9TEcFJYSZSeSDh8mVGvugi0exnSJrrBZDyVCcAEQEAAcLB ZQQYAQIADwIbDAUCWZR1ZwUJA59cIQAKCRD5M+tD3xNhH5b+D/9XG44Ci6STdcA5RO/ur05J EE3Ux1DCHZ5V7vNAtX/8Wg4l4GZfweauXwuJ1w7Sp7fklwcNC6wsceI+EmNjGMqfIaukGetG +jBGqsQ7moOZodfXUoCK98gblKgt/BPYMVidzlGC8Q/+lZg1+o29sPnwImW+MXt/Z5az/Z17 Qc265g+p5cqJHzq6bpQdnF7Fu6btKU/kv6wJghENvgMXBuyThqsyFReJWFh2wfaKyuix3Zyj ccq7/blkhzIKmtFWgDcgaSc2UAuJU+x9nuYjihW6WobpKP/nlUDu3BIsbIq09UEke+uE/QK+ FJ8PTJkAsXOf1Bc2C0XbW4Y2hf103+YY6L8weUCBsWC5VH5VtVmeuh26ENURclwfeXhWQ9Og 77yzpTXWr5g1Z0oLpYpWPv745J4bE7pv+dzxOrFdM1xNkzY2pvXph/A8OjxZNQklDkHQ7PIB Lki5L2F4XkEOddUUQchJwzMqTPsggPDmGjgLZrqgO+s4ECZK5+nLD3HEpAbPa3JLDaScy+90 Nu1lAqPUHSnP3vYZVw85ZYm6UCxHE4VLMnnJsN09ZhsOSVR+GyP5Nyw9rT1V3lcsuH7M5Naa 2Xobn9m7l9bRCD/Ji8kG15eV1WTxx1HXVQGjdUYDI7UwegBNbwMLh17XDy+3sn/6SgcqtECA Q6pZKA2mTQxEKMLBZQQYAQIADwIbDAUCWZR3hQUJA59eRwAKCRD5M+tD3xNhH4a/D/4jLAZu UhvU1swWcNEVVCELZ0D3LOV14XcY2MXa3QOpeZ9Bgq7YYJ4S5YXK+SBQS0FkRZdjGNvlGZoG ZdpU+NsQmQFhqHGwX0IT9MeTFM8uvKgxNKGwMVcV9g0IOqwBhGHne+BFboRA9362fgGW5AYQ zT0mzzRKEoOh4r3AQvbM6kLISxo0k1ujdYiI5nj/5WoKDqxTwwfuN1uDUHsWo3tzenRmpMyU NyW3Dc+1ajvXLyo09sRRq7BnM99Rix1EGL8Qhwy+j0YAv+FuspWxUX9FxXYho5PvGLHLsHfK FYQ7x/RRbpMjkJWVfIe/xVnfvn4kz+MTA5yhvsuNi678fLwY9hBP0y4lO8Ob2IhEPdfnTuIs tFVxXuelJ9xAe5TyqP0f+fQjf1ixsBZkqOohsBXDfje0iaUpYa/OQ/BBeej0dUdg2JEu4jAC x41HpVCnP9ipLpD0fYz1d/dX0F/VY2ovW6Eba/y/ngOSAR6C+u881m7oH2l0G47MTwkaQCBA bLGXPj4TCdX3lftqt4bcBPBJ+rFAnJmRHtUuyyaewBnZ81ZU2YAptqFM1kTh+aSvMvGhfVsQ qZL2rk2OPN1hg+KXhErlbTZ6oPtLCFhSHQmuxQ4oc4U147wBTUuOdwNjtnNatUhRCp8POc+3 XphVR5G70mnca1E2vzC77z+XSlTyRA== Message-ID: <9f8fb00b-28fa-eb23-0567-db25298444da@intel.com> Date: Fri, 29 Jun 2018 11:37:30 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v2 3/4] eal: enable WC during resources mapping X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Jun 2018 10:37:35 -0000 On 6/29/2018 11:28 AM, Rafał Kozik wrote: > 2018-06-29 11:05 GMT+02:00 Ferruh Yigit : >> On 6/29/2018 9:58 AM, Rafał Kozik wrote: >>> 2018-06-28 16:50 GMT+02:00 Ferruh Yigit : >>>> 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 >>>>> Acked-by: Bruce Richardson >>>>> --- >>>>> 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 Isn't this too verbose now? This is info level, not debug and which means will be visible many cases and mapping resource0 is most common way and it will be keep repeated. What I asked was, if _wc is requested and failed it will fallback to default resource file and user won't know anything about it, add a log about fallback so that user can know what actually happens is different from what requested. > >>> >>>>> + } >>>>> + >>>>> + 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 */ >>>>> >>>> >>