[dpdk-dev,v2,5/6] eal: Use map_idx in pci_uio_map_resource() of bsdapp to work same as linuxapp
| Message ID | 1427170717-13879-6-git-send-email-mukawa@igel.co.jp (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 9CB339ACA; Tue, 24 Mar 2015 05:19:07 +0100 (CET) Received: from mail-pd0-f179.google.com (mail-pd0-f179.google.com [209.85.192.179]) by dpdk.org (Postfix) with ESMTP id CA4A49AB0 for <dev@dpdk.org>; Tue, 24 Mar 2015 05:19:02 +0100 (CET) Received: by pdnc3 with SMTP id c3so208775833pdn.0 for <dev@dpdk.org>; Mon, 23 Mar 2015 21:19:02 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=v+dCzMoXby6ChDb9oDRYoNdPvo7YorKaE8X/xfVS7mQ=; b=Q5cEP3XgO/L4z6UdNjuj56qke6KwDxTa2CD+vyRl+9HjgdPMbLJRC8HVrB99jKzoYh wNr0L/+kEtIU0walV75yjzMGwMhYf7QXmTaXYCc089irjVkiYsbmgXLB/jkRVQV2sF6H z0q3zwhSRiAGYNBMdnz17oLP/ev0kxCoZ4G5iYvWMAnm0fRFUDEqRxEUoG25QYMHpgRF Izr8/MOudhoRblPHZjCIUl6nqSWpPvjTysBwxWF2VoDHBnJBiMDYakrDBKyDfYDCXxN8 uidhYRXhcINiNptH6oNrowml1wDK7c9CBrlotc83MY41L6GkVT/mN+jvWodtpe9HVXcJ sEnQ== X-Gm-Message-State: ALoCoQkgV3lAZvTIq4Z+cDxWEKpO1TXVaw5fre4+J0y8CUFBcujVH0297t414+HCpr/uxWJrsdzp X-Received: by 10.70.131.107 with SMTP id ol11mr4090297pdb.63.1427170742246; Mon, 23 Mar 2015 21:19:02 -0700 (PDT) Received: from localhost.localdomain (napt.igel.co.jp. [219.106.231.132]) by mx.google.com with ESMTPSA id jx5sm2482078pbc.85.2015.03.23.21.19.00 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 23 Mar 2015 21:19:01 -0700 (PDT) From: Tetsuya Mukawa <mukawa@igel.co.jp> To: dev@dpdk.org Date: Tue, 24 Mar 2015 13:18:36 +0900 Message-Id: <1427170717-13879-6-git-send-email-mukawa@igel.co.jp> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1427170717-13879-1-git-send-email-mukawa@igel.co.jp> References: <1426584645-28828-7-git-send-email-mukawa@igel.co.jp> <1427170717-13879-1-git-send-email-mukawa@igel.co.jp> Subject: [dpdk-dev] [PATCH v2 5/6] eal: Use map_idx in pci_uio_map_resource() of bsdapp to work same as linuxapp X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Tetsuya Mukawa
March 24, 2015, 4:18 a.m. UTC
This patch changes code that maps pci resources in bsdapp.
Linuxapp has almost same code. To consolidate both, fix implementation
of bsdapp to work same as linuxapp.
Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
lib/librte_eal/bsdapp/eal/eal_pci.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
Comments
> -----Original Message----- > From: Tetsuya Mukawa [mailto:mukawa@igel.co.jp] > Sent: Tuesday, March 24, 2015 4:19 AM > To: dev@dpdk.org > Cc: Iremonger, Bernard; Richardson, Bruce; david.marchand@6wind.com; Tetsuya Mukawa > Subject: [PATCH v2 5/6] eal: Use map_idx in pci_uio_map_resource() of bsdapp to work same as > linuxapp > > This patch changes code that maps pci resources in bsdapp. > Linuxapp has almost same code. To consolidate both, fix implementation of bsdapp to work same as > linuxapp. > > Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp> > --- > lib/librte_eal/bsdapp/eal/eal_pci.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c > index 85f8671..08b91b4 100644 > --- a/lib/librte_eal/bsdapp/eal/eal_pci.c > +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c > @@ -195,7 +195,7 @@ pci_uio_map_secondary(struct rte_pci_device *dev) static int > pci_uio_map_resource(struct rte_pci_device *dev) { > - int i, j; > + int i, map_idx; > char devname[PATH_MAX]; /* contains the /dev/uioX */ > void *mapaddr; > uint64_t phaddr; > @@ -247,31 +247,31 @@ pci_uio_map_resource(struct rte_pci_device *dev) > pagesz = sysconf(_SC_PAGESIZE); > > maps = uio_res->maps; > - for (i = uio_res->nb_maps = 0; i != PCI_MAX_RESOURCE; i++) { > + for (i = 0, map_idx = 0; i != PCI_MAX_RESOURCE; i++) { > > - j = uio_res->nb_maps; > /* skip empty BAR */ > if ((phaddr = dev->mem_resource[i].phys_addr) == 0) > continue; > > /* if matching map is found, then use it */ > offset = i * pagesz; > - maps[j].offset = offset; > - maps[j].phaddr = dev->mem_resource[i].phys_addr; > - maps[j].size = dev->mem_resource[i].len; > - if (maps[j].addr != NULL || > - (mapaddr = pci_map_resource(NULL, devname, (off_t)offset, > - (size_t)maps[j].size) > - ) == NULL) { > + maps[map_idx].offset = offset; > + maps[map_idx].phaddr = dev->mem_resource[i].phys_addr; > + maps[map_idx].size = dev->mem_resource[i].len; > + mapaddr = pci_map_resource(NULL, devname, (off_t)offset, > + (size_t)maps[map_idx].size); > + if ((maps[map_idx].addr != NULL) || (mapaddr == NULL)) { Hi Tetsuya, Should be checking for if (mapaddr == MAP_FAILED) here. Seems to be fixed in patch 6/6 though. Regards, Bernard. > rte_free(uio_res); > return -1; > } > > - maps[j].addr = mapaddr; > - uio_res->nb_maps++; > + maps[map_idx].addr = mapaddr; > + map_idx++; > dev->mem_resource[i].addr = mapaddr; > } > > + uio_res->nb_maps = map_idx; > + > TAILQ_INSERT_TAIL(uio_res_list, uio_res, next); > > return 0; > -- > 1.9.1
On 2015/03/26 0:27, Iremonger, Bernard wrote: > >> -----Original Message----- >> From: Tetsuya Mukawa [mailto:mukawa@igel.co.jp] >> Sent: Tuesday, March 24, 2015 4:19 AM >> To: dev@dpdk.org >> Cc: Iremonger, Bernard; Richardson, Bruce; david.marchand@6wind.com; Tetsuya Mukawa >> Subject: [PATCH v2 5/6] eal: Use map_idx in pci_uio_map_resource() of bsdapp to work same as >> linuxapp >> >> This patch changes code that maps pci resources in bsdapp. >> Linuxapp has almost same code. To consolidate both, fix implementation of bsdapp to work same as >> linuxapp. >> >> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp> >> --- >> lib/librte_eal/bsdapp/eal/eal_pci.c | 24 ++++++++++++------------ >> 1 file changed, 12 insertions(+), 12 deletions(-) >> >> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c >> index 85f8671..08b91b4 100644 >> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c >> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c >> @@ -195,7 +195,7 @@ pci_uio_map_secondary(struct rte_pci_device *dev) static int >> pci_uio_map_resource(struct rte_pci_device *dev) { >> - int i, j; >> + int i, map_idx; >> char devname[PATH_MAX]; /* contains the /dev/uioX */ >> void *mapaddr; >> uint64_t phaddr; >> @@ -247,31 +247,31 @@ pci_uio_map_resource(struct rte_pci_device *dev) >> pagesz = sysconf(_SC_PAGESIZE); >> >> maps = uio_res->maps; >> - for (i = uio_res->nb_maps = 0; i != PCI_MAX_RESOURCE; i++) { >> + for (i = 0, map_idx = 0; i != PCI_MAX_RESOURCE; i++) { >> >> - j = uio_res->nb_maps; >> /* skip empty BAR */ >> if ((phaddr = dev->mem_resource[i].phys_addr) == 0) >> continue; >> >> /* if matching map is found, then use it */ >> offset = i * pagesz; >> - maps[j].offset = offset; >> - maps[j].phaddr = dev->mem_resource[i].phys_addr; >> - maps[j].size = dev->mem_resource[i].len; >> - if (maps[j].addr != NULL || >> - (mapaddr = pci_map_resource(NULL, devname, (off_t)offset, >> - (size_t)maps[j].size) >> - ) == NULL) { >> + maps[map_idx].offset = offset; >> + maps[map_idx].phaddr = dev->mem_resource[i].phys_addr; >> + maps[map_idx].size = dev->mem_resource[i].len; >> + mapaddr = pci_map_resource(NULL, devname, (off_t)offset, >> + (size_t)maps[map_idx].size); >> + if ((maps[map_idx].addr != NULL) || (mapaddr == NULL)) { > Hi Tetsuya, > > Should be checking for if (mapaddr == MAP_FAILED) here. > Seems to be fixed in patch 6/6 though. Hi Bernard, Here, bsdapp still has old pci_map_resource(). Old pci_map_resource() return NULL when mapping is failed. And this behavior will be changed in 6/6. This is why MAP_FAILED is checked in 6/6. Regards, Tetsuya > Regards, > > Bernard. > >> rte_free(uio_res); >> return -1; >> } >> >> - maps[j].addr = mapaddr; >> - uio_res->nb_maps++; >> + maps[map_idx].addr = mapaddr; >> + map_idx++; >> dev->mem_resource[i].addr = mapaddr; >> } >> >> + uio_res->nb_maps = map_idx; >> + >> TAILQ_INSERT_TAIL(uio_res_list, uio_res, next); >> >> return 0; >> -- >> 1.9.1
> -----Original Message----- > From: Tetsuya Mukawa [mailto:mukawa@igel.co.jp] > Sent: Thursday, March 26, 2015 2:51 AM > To: Iremonger, Bernard; dev@dpdk.org > Cc: Richardson, Bruce; david.marchand@6wind.com > Subject: Re: [PATCH v2 5/6] eal: Use map_idx in pci_uio_map_resource() of bsdapp to work same as > linuxapp > > On 2015/03/26 0:27, Iremonger, Bernard wrote: > > > >> -----Original Message----- > >> From: Tetsuya Mukawa [mailto:mukawa@igel.co.jp] > >> Sent: Tuesday, March 24, 2015 4:19 AM > >> To: dev@dpdk.org > >> Cc: Iremonger, Bernard; Richardson, Bruce; david.marchand@6wind.com; > >> Tetsuya Mukawa > >> Subject: [PATCH v2 5/6] eal: Use map_idx in pci_uio_map_resource() of > >> bsdapp to work same as linuxapp > >> > >> This patch changes code that maps pci resources in bsdapp. > >> Linuxapp has almost same code. To consolidate both, fix > >> implementation of bsdapp to work same as linuxapp. > >> > >> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp> > >> --- > >> lib/librte_eal/bsdapp/eal/eal_pci.c | 24 ++++++++++++------------ > >> 1 file changed, 12 insertions(+), 12 deletions(-) > >> > >> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c > >> b/lib/librte_eal/bsdapp/eal/eal_pci.c > >> index 85f8671..08b91b4 100644 > >> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c > >> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c > >> @@ -195,7 +195,7 @@ pci_uio_map_secondary(struct rte_pci_device *dev) > >> static int pci_uio_map_resource(struct rte_pci_device *dev) { > >> - int i, j; > >> + int i, map_idx; > >> char devname[PATH_MAX]; /* contains the /dev/uioX */ > >> void *mapaddr; > >> uint64_t phaddr; > >> @@ -247,31 +247,31 @@ pci_uio_map_resource(struct rte_pci_device *dev) > >> pagesz = sysconf(_SC_PAGESIZE); > >> > >> maps = uio_res->maps; > >> - for (i = uio_res->nb_maps = 0; i != PCI_MAX_RESOURCE; i++) { > >> + for (i = 0, map_idx = 0; i != PCI_MAX_RESOURCE; i++) { > >> > >> - j = uio_res->nb_maps; > >> /* skip empty BAR */ > >> if ((phaddr = dev->mem_resource[i].phys_addr) == 0) > >> continue; > >> > >> /* if matching map is found, then use it */ > >> offset = i * pagesz; > >> - maps[j].offset = offset; > >> - maps[j].phaddr = dev->mem_resource[i].phys_addr; > >> - maps[j].size = dev->mem_resource[i].len; > >> - if (maps[j].addr != NULL || > >> - (mapaddr = pci_map_resource(NULL, devname, (off_t)offset, > >> - (size_t)maps[j].size) > >> - ) == NULL) { > >> + maps[map_idx].offset = offset; > >> + maps[map_idx].phaddr = dev->mem_resource[i].phys_addr; > >> + maps[map_idx].size = dev->mem_resource[i].len; > >> + mapaddr = pci_map_resource(NULL, devname, (off_t)offset, > >> + (size_t)maps[map_idx].size); > >> + if ((maps[map_idx].addr != NULL) || (mapaddr == NULL)) { > > Hi Tetsuya, > > > > Should be checking for if (mapaddr == MAP_FAILED) here. > > Seems to be fixed in patch 6/6 though. > > Hi Bernard, > > Here, bsdapp still has old pci_map_resource(). > Old pci_map_resource() return NULL when mapping is failed. > And this behavior will be changed in 6/6. This is why MAP_FAILED is checked in 6/6. > > Regards, > Tetsuya > Hi Tetsuya, Would it make sense to squash patches 5/6 and 6/6 ? Regards, Bernard.
On 2015/03/26 19:41, Iremonger, Bernard wrote: > >> -----Original Message----- >> From: Tetsuya Mukawa [mailto:mukawa@igel.co.jp] >> Sent: Thursday, March 26, 2015 2:51 AM >> To: Iremonger, Bernard; dev@dpdk.org >> Cc: Richardson, Bruce; david.marchand@6wind.com >> Subject: Re: [PATCH v2 5/6] eal: Use map_idx in pci_uio_map_resource() of bsdapp to work same as >> linuxapp >> >> On 2015/03/26 0:27, Iremonger, Bernard wrote: >>>> -----Original Message----- >>>> From: Tetsuya Mukawa [mailto:mukawa@igel.co.jp] >>>> Sent: Tuesday, March 24, 2015 4:19 AM >>>> To: dev@dpdk.org >>>> Cc: Iremonger, Bernard; Richardson, Bruce; david.marchand@6wind.com; >>>> Tetsuya Mukawa >>>> Subject: [PATCH v2 5/6] eal: Use map_idx in pci_uio_map_resource() of >>>> bsdapp to work same as linuxapp >>>> >>>> This patch changes code that maps pci resources in bsdapp. >>>> Linuxapp has almost same code. To consolidate both, fix >>>> implementation of bsdapp to work same as linuxapp. >>>> >>>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp> >>>> --- >>>> lib/librte_eal/bsdapp/eal/eal_pci.c | 24 ++++++++++++------------ >>>> 1 file changed, 12 insertions(+), 12 deletions(-) >>>> >>>> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c >>>> b/lib/librte_eal/bsdapp/eal/eal_pci.c >>>> index 85f8671..08b91b4 100644 >>>> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c >>>> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c >>>> @@ -195,7 +195,7 @@ pci_uio_map_secondary(struct rte_pci_device *dev) >>>> static int pci_uio_map_resource(struct rte_pci_device *dev) { >>>> - int i, j; >>>> + int i, map_idx; >>>> char devname[PATH_MAX]; /* contains the /dev/uioX */ >>>> void *mapaddr; >>>> uint64_t phaddr; >>>> @@ -247,31 +247,31 @@ pci_uio_map_resource(struct rte_pci_device *dev) >>>> pagesz = sysconf(_SC_PAGESIZE); >>>> >>>> maps = uio_res->maps; >>>> - for (i = uio_res->nb_maps = 0; i != PCI_MAX_RESOURCE; i++) { >>>> + for (i = 0, map_idx = 0; i != PCI_MAX_RESOURCE; i++) { >>>> >>>> - j = uio_res->nb_maps; >>>> /* skip empty BAR */ >>>> if ((phaddr = dev->mem_resource[i].phys_addr) == 0) >>>> continue; >>>> >>>> /* if matching map is found, then use it */ >>>> offset = i * pagesz; >>>> - maps[j].offset = offset; >>>> - maps[j].phaddr = dev->mem_resource[i].phys_addr; >>>> - maps[j].size = dev->mem_resource[i].len; >>>> - if (maps[j].addr != NULL || >>>> - (mapaddr = pci_map_resource(NULL, devname, (off_t)offset, >>>> - (size_t)maps[j].size) >>>> - ) == NULL) { >>>> + maps[map_idx].offset = offset; >>>> + maps[map_idx].phaddr = dev->mem_resource[i].phys_addr; >>>> + maps[map_idx].size = dev->mem_resource[i].len; >>>> + mapaddr = pci_map_resource(NULL, devname, (off_t)offset, >>>> + (size_t)maps[map_idx].size); >>>> + if ((maps[map_idx].addr != NULL) || (mapaddr == NULL)) { >>> Hi Tetsuya, >>> >>> Should be checking for if (mapaddr == MAP_FAILED) here. >>> Seems to be fixed in patch 6/6 though. >> Hi Bernard, >> >> Here, bsdapp still has old pci_map_resource(). >> Old pci_map_resource() return NULL when mapping is failed. >> And this behavior will be changed in 6/6. This is why MAP_FAILED is checked in 6/6. >> >> Regards, >> Tetsuya >> > Hi Tetsuya, > > Would it make sense to squash patches 5/6 and 6/6 ? Hi Bernard, I don't have strong opinion about it. So let's squash patches. And then if you feel to want to split it again, let me know. Regards, Tetsuya > > Regards, > > Bernard. >
diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c index 85f8671..08b91b4 100644 --- a/lib/librte_eal/bsdapp/eal/eal_pci.c +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c @@ -195,7 +195,7 @@ pci_uio_map_secondary(struct rte_pci_device *dev) static int pci_uio_map_resource(struct rte_pci_device *dev) { - int i, j; + int i, map_idx; char devname[PATH_MAX]; /* contains the /dev/uioX */ void *mapaddr; uint64_t phaddr; @@ -247,31 +247,31 @@ pci_uio_map_resource(struct rte_pci_device *dev) pagesz = sysconf(_SC_PAGESIZE); maps = uio_res->maps; - for (i = uio_res->nb_maps = 0; i != PCI_MAX_RESOURCE; i++) { + for (i = 0, map_idx = 0; i != PCI_MAX_RESOURCE; i++) { - j = uio_res->nb_maps; /* skip empty BAR */ if ((phaddr = dev->mem_resource[i].phys_addr) == 0) continue; /* if matching map is found, then use it */ offset = i * pagesz; - maps[j].offset = offset; - maps[j].phaddr = dev->mem_resource[i].phys_addr; - maps[j].size = dev->mem_resource[i].len; - if (maps[j].addr != NULL || - (mapaddr = pci_map_resource(NULL, devname, (off_t)offset, - (size_t)maps[j].size) - ) == NULL) { + maps[map_idx].offset = offset; + maps[map_idx].phaddr = dev->mem_resource[i].phys_addr; + maps[map_idx].size = dev->mem_resource[i].len; + mapaddr = pci_map_resource(NULL, devname, (off_t)offset, + (size_t)maps[map_idx].size); + if ((maps[map_idx].addr != NULL) || (mapaddr == NULL)) { rte_free(uio_res); return -1; } - maps[j].addr = mapaddr; - uio_res->nb_maps++; + maps[map_idx].addr = mapaddr; + map_idx++; dev->mem_resource[i].addr = mapaddr; } + uio_res->nb_maps = map_idx; + TAILQ_INSERT_TAIL(uio_res_list, uio_res, next); return 0;