From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-f195.google.com (mail-lj1-f195.google.com [209.85.208.195]) by dpdk.org (Postfix) with ESMTP id D35171B4DE for ; Fri, 29 Jun 2018 12:28:49 +0200 (CEST) Received: by mail-lj1-f195.google.com with SMTP id h2-v6so3669764ljb.10 for ; Fri, 29 Jun 2018 03:28:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=QI5bOo+we1KtchCtADRUzi3Qt0r1HbZ5BHHu4yBJcwc=; b=qEYizW3OxqJ59x1eAHOwFD/b0qBlBuKupUHtKukiiK3tM/fkeq3E6dp5x/ELufmpqa XEVm5RFUTiTfS+QJxr+cVKR+VXQNYTgoc26t0xmpfbxPiRDta+WrBPzHV5kh3scngToY DIWzv+7zjsXrkbRlfUhtWS2I34fd90p3YV0IBzkX6wDuEX6/o9Tw7M1wZlM0hKrdQdvQ 6US3P7PEBdDCRqcnYQZzpI/Ct3nqXMUyPGHYjI2hPx9p5InOC6kArNkS30JUuhY7UdXW GvqNAbtOJlDBBBc3XbkeqbgBQKtprjNRExNndGsn41oLdDkekSPy1HMl8bnqEwTTE2Sn QOjQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=QI5bOo+we1KtchCtADRUzi3Qt0r1HbZ5BHHu4yBJcwc=; b=Q1UOsd7pM/Z+EkGDcIE+HHSJFt6Bo3SO1qMdPHyxJQAckaaNIBAoFtaz5w/r+3ac1r uFHE5lS2+hbA9Z2G3QTWIyJP3cWg0QO3FylL7jm+fw+54i3G0jnpmyFq5V5eN+yR5Arm Cszoplb9LPwzCQCSwq7+bcrQCbOq9ypZWFRjYefLS9olBO4BTRZkQjihVpiLCuRISzBF LHgmiLzEkwETASKgLob0eaEN2R0xkXN+0YtW6PIpJ0kzLkbqkOXJfOVq+hVxz/Ew1KJi FZpudPNaxBJLxmJ/u+x09gjtkhkGCMyAyirhHzonmw5C6KcM3ULazGV8kE2Df2j5AFiH wWhg== X-Gm-Message-State: APt69E1407h1/OP9X8YH0iWF0MvGiEdh8ZZVlpoBoBefrKR4+8Kw57Dg iqwEHlGVQEP5XOXSEXZl06pMTV8MB+ZWHK0b1l/lhQ== X-Google-Smtp-Source: AAOMgpcdU1acdvgkO5NdVvztcItieSpZGLjZ9bpJDVoKf7DeMaLvbWgoYnVbJgeFunXxa0UtgIMzqTXBYK5n5m5hO1M= X-Received: by 2002:a2e:2bc9:: with SMTP id r70-v6mr5511527ljr.133.1530268129534; Fri, 29 Jun 2018 03:28:49 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a19:4a95:0:0:0:0:0 with HTTP; Fri, 29 Jun 2018 03:28:48 -0700 (PDT) In-Reply-To: <2cb4219e-b354-0983-e56a-c9a14336b596@intel.com> 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: =?UTF-8?B?UmFmYcWCIEtvemlr?= Date: Fri, 29 Jun 2018 12:28:48 +0200 Message-ID: To: Ferruh Yigit Cc: dev@dpdk.org, Marcin Wojtas , =?UTF-8?Q?Micha=C5=82_Krawczyk?= , "Tzalik, Guy" , "Schmeilin, Evgeny" , "Matushevsky, Alexander" , "Chauskin, Igor" , Thomas Monjalon Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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:28:50 -0000 2018-06-29 11:05 GMT+02:00 Ferruh Yigit : > On 6/29/2018 9:58 AM, Rafa=C5=82 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/p= ci_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 =3D -1; >>>> char devname[PATH_MAX]; >>>> void *mapaddr; >>>> struct rte_pci_addr *loc; >>>> struct pci_map *maps; >>>> + int wc_activate =3D 0; >>>> + >>>> + if (dev->driver !=3D NULL) >>>> + wc_activate =3D dev->driver->drv_flags & RTE_PCI_DRV_WC_= ACTIVATE; >>>> >>>> loc =3D &dev->addr; >>>> maps =3D 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 =3D rte_malloc(NULL, strlen(devname) + 1, 0); >>>> if (maps[map_idx].path =3D=3D NULL) { >>>> @@ -309,11 +306,31 @@ pci_uio_map_resource_by_index(struct rte_pci_dev= ice *dev, int res_idx, >>>> /* >>>> * open resource file, to mmap it >>>> */ >>>> - fd =3D 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 =3D open(devname, O_RDWR); >>> >>> What do you think adding an error log here? If opening resource$_wc fai= ls and >>> fallback to resource# file, there won't be any way for user to know abo= ut 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 >> >>>> + } >>>> + >>>> + 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 =3D 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_p= ci.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 */ >>>> >>> >