* [dpdk-dev] [PATCH] pci/uio: enable prefetchable resources mapping
@ 2017-06-03 22:57 Changpeng Liu
2017-10-05 0:06 ` Ferruh Yigit
0 siblings, 1 reply; 9+ messages in thread
From: Changpeng Liu @ 2017-06-03 22:57 UTC (permalink / raw)
To: dev; +Cc: changpeng.liu
For PCI prefetchable resources, Linux will create a
write combined file as well, the library will try
to map resourceX_wc file first, if the file does
not exist, then it will map resourceX as usual.
Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
---
lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
index fa10329..d9fc20a 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
@@ -321,7 +321,7 @@
/* update devname for mmap */
snprintf(devname, sizeof(devname),
- "%s/" PCI_PRI_FMT "/resource%d",
+ "%s/" PCI_PRI_FMT "/resource%d_wc",
pci_get_sysfs_path(),
loc->domain, loc->bus, loc->devid,
loc->function, res_idx);
@@ -335,13 +335,22 @@
}
/*
- * open resource file, to mmap it
+ * open prefetchable resource file first, try to mmap it
*/
fd = open(devname, O_RDWR);
if (fd < 0) {
- RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
- devname, strerror(errno));
- goto error;
+ snprintf(devname, sizeof(devname),
+ "%s/" PCI_PRI_FMT "/resource%d",
+ 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;
+ }
}
/* try mapping somewhere close to the end of hugepages */
--
1.9.3
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [dpdk-dev] [PATCH] pci/uio: enable prefetchable resources mapping
2017-06-03 22:57 [dpdk-dev] [PATCH] pci/uio: enable prefetchable resources mapping Changpeng Liu
@ 2017-10-05 0:06 ` Ferruh Yigit
2017-10-05 8:28 ` Bruce Richardson
0 siblings, 1 reply; 9+ messages in thread
From: Ferruh Yigit @ 2017-10-05 0:06 UTC (permalink / raw)
To: Changpeng Liu, dev
On 6/3/2017 11:57 PM, Changpeng Liu wrote:
> For PCI prefetchable resources, Linux will create a
> write combined file as well, the library will try
> to map resourceX_wc file first, if the file does
> not exist, then it will map resourceX as usual.
Hi Changpeng,
Code part looks OK, but can you please describe more why we should try
write combined resource file first, what is the benefit of using it _wc
file?
Thanks,
ferruh
>
> Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
> ---
> lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> index fa10329..d9fc20a 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> @@ -321,7 +321,7 @@
>
> /* update devname for mmap */
> snprintf(devname, sizeof(devname),
> - "%s/" PCI_PRI_FMT "/resource%d",
> + "%s/" PCI_PRI_FMT "/resource%d_wc",
> pci_get_sysfs_path(),
> loc->domain, loc->bus, loc->devid,
> loc->function, res_idx);
> @@ -335,13 +335,22 @@
> }
>
> /*
> - * open resource file, to mmap it
> + * open prefetchable resource file first, try to mmap it
> */
> fd = open(devname, O_RDWR);
> if (fd < 0) {
> - RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
> - devname, strerror(errno));
> - goto error;
> + snprintf(devname, sizeof(devname),
> + "%s/" PCI_PRI_FMT "/resource%d",
> + 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;
> + }
> }
>
> /* try mapping somewhere close to the end of hugepages */
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [dpdk-dev] [PATCH] pci/uio: enable prefetchable resources mapping
2017-10-05 0:06 ` Ferruh Yigit
@ 2017-10-05 8:28 ` Bruce Richardson
2017-10-05 8:33 ` Bruce Richardson
0 siblings, 1 reply; 9+ messages in thread
From: Bruce Richardson @ 2017-10-05 8:28 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: Changpeng Liu, dev
On Thu, Oct 05, 2017 at 01:06:41AM +0100, Ferruh Yigit wrote:
> On 6/3/2017 11:57 PM, Changpeng Liu wrote:
> > For PCI prefetchable resources, Linux will create a
> > write combined file as well, the library will try
> > to map resourceX_wc file first, if the file does
> > not exist, then it will map resourceX as usual.
>
> Hi Changpeng,
>
> Code part looks OK, but can you please describe more why we should try
> write combined resource file first, what is the benefit of using it _wc
> file?
>
> Thanks,
> ferruh
>
Also, if we use a write combining resource file, I believe we will use
correct ordering of instructions within our drivers. Does applying this
patch not also mean that we would need memory barriers inside all the
drivers, so as to ensure that we don't have a queue doorbell write
reordered with the descriptor writes? I don't think it's safe to apply
this change on it's own, without driver changes, since all PMDs assume
strong ordering on IA.
Regards,
/Bruce
>
> >
> > Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
> > ---
> > lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 19 ++++++++++++++-----
> > 1 file changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> > index fa10329..d9fc20a 100644
> > --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> > +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> > @@ -321,7 +321,7 @@
> >
> > /* update devname for mmap */
> > snprintf(devname, sizeof(devname),
> > - "%s/" PCI_PRI_FMT "/resource%d",
> > + "%s/" PCI_PRI_FMT "/resource%d_wc",
> > pci_get_sysfs_path(),
> > loc->domain, loc->bus, loc->devid,
> > loc->function, res_idx);
> > @@ -335,13 +335,22 @@
> > }
> >
> > /*
> > - * open resource file, to mmap it
> > + * open prefetchable resource file first, try to mmap it
> > */
> > fd = open(devname, O_RDWR);
> > if (fd < 0) {
> > - RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
> > - devname, strerror(errno));
> > - goto error;
> > + snprintf(devname, sizeof(devname),
> > + "%s/" PCI_PRI_FMT "/resource%d",
> > + 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;
> > + }
> > }
> >
> > /* try mapping somewhere close to the end of hugepages */
> >
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [dpdk-dev] [PATCH] pci/uio: enable prefetchable resources mapping
2017-10-05 8:28 ` Bruce Richardson
@ 2017-10-05 8:33 ` Bruce Richardson
2018-10-28 1:59 ` Ferruh Yigit
0 siblings, 1 reply; 9+ messages in thread
From: Bruce Richardson @ 2017-10-05 8:33 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: Changpeng Liu, dev
On Thu, Oct 05, 2017 at 09:28:34AM +0100, Bruce Richardson wrote:
> On Thu, Oct 05, 2017 at 01:06:41AM +0100, Ferruh Yigit wrote:
> > On 6/3/2017 11:57 PM, Changpeng Liu wrote:
> > > For PCI prefetchable resources, Linux will create a
> > > write combined file as well, the library will try
> > > to map resourceX_wc file first, if the file does
> > > not exist, then it will map resourceX as usual.
> >
> > Hi Changpeng,
> >
> > Code part looks OK, but can you please describe more why we should try
> > write combined resource file first, what is the benefit of using it _wc
> > file?
> >
> > Thanks,
> > ferruh
> >
> Also, if we use a write combining resource file, I believe we will use
s/will use/will lose/
> correct ordering of instructions within our drivers. Does applying this
> patch not also mean that we would need memory barriers inside all the
> drivers, so as to ensure that we don't have a queue doorbell write
> reordered with the descriptor writes? I don't think it's safe to apply
> this change on it's own, without driver changes, since all PMDs assume
> strong ordering on IA.
>
> Regards,
> /Bruce
>
> >
> > >
> > > Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
> > > ---
> > > lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 19 ++++++++++++++-----
> > > 1 file changed, 14 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> > > index fa10329..d9fc20a 100644
> > > --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> > > +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> > > @@ -321,7 +321,7 @@
> > >
> > > /* update devname for mmap */
> > > snprintf(devname, sizeof(devname),
> > > - "%s/" PCI_PRI_FMT "/resource%d",
> > > + "%s/" PCI_PRI_FMT "/resource%d_wc",
> > > pci_get_sysfs_path(),
> > > loc->domain, loc->bus, loc->devid,
> > > loc->function, res_idx);
> > > @@ -335,13 +335,22 @@
> > > }
> > >
> > > /*
> > > - * open resource file, to mmap it
> > > + * open prefetchable resource file first, try to mmap it
> > > */
> > > fd = open(devname, O_RDWR);
> > > if (fd < 0) {
> > > - RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
> > > - devname, strerror(errno));
> > > - goto error;
> > > + snprintf(devname, sizeof(devname),
> > > + "%s/" PCI_PRI_FMT "/resource%d",
> > > + 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;
> > > + }
> > > }
> > >
> > > /* try mapping somewhere close to the end of hugepages */
> > >
> >
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [dpdk-dev] [PATCH] pci/uio: enable prefetchable resources mapping
2017-10-05 8:33 ` Bruce Richardson
@ 2018-10-28 1:59 ` Ferruh Yigit
2018-10-29 0:48 ` Liu, Changpeng
0 siblings, 1 reply; 9+ messages in thread
From: Ferruh Yigit @ 2018-10-28 1:59 UTC (permalink / raw)
To: Changpeng Liu; +Cc: Bruce Richardson, dpdk-dev, Rafal Kozik
On 10/5/2017 9:33 AM, bruce.richardson at intel.com (Bruce Richardson) wrote:
> On Thu, Oct 05, 2017 at 09:28:34AM +0100, Bruce Richardson wrote:
>> On Thu, Oct 05, 2017 at 01:06:41AM +0100, Ferruh Yigit wrote:
>>> On 6/3/2017 11:57 PM, Changpeng Liu wrote:
>>>> For PCI prefetchable resources, Linux will create a
>>>> write combined file as well, the library will try
>>>> to map resourceX_wc file first, if the file does
>>>> not exist, then it will map resourceX as usual.
>>>
>>> Hi Changpeng,
>>>
>>> Code part looks OK, but can you please describe more why we should try
>>> write combined resource file first, what is the benefit of using it _wc
>>> file?
>>>
>>> Thanks,
>>> ferruh
>>>
>> Also, if we use a write combining resource file, I believe we will use
> s/will use/will lose/
>
>> correct ordering of instructions within our drivers. Does applying this
>> patch not also mean that we would need memory barriers inside all the
>> drivers, so as to ensure that we don't have a queue doorbell write
>> reordered with the descriptor writes? I don't think it's safe to apply
>> this change on it's own, without driver changes, since all PMDs assume
>> strong ordering on IA.
>>
>> Regards,
>> /Bruce
Hi Changpeng,
Following patch looks like superseded this patch, can you please check it. I
will update the status of this patch in patchwork.
https://patches.dpdk.org/project/dpdk/list/?series=323&state=%2A&archive=both
https://patches.dpdk.org/patch/41971/
>>
>>>
>>>>
>>>> Signed-off-by: Changpeng Liu <changpeng.liu at intel.com>
>>>> ---
>>>> lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 19 ++++++++++++++-----
>>>> 1 file changed, 14 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>>>> index fa10329..d9fc20a 100644
>>>> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>>>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>>>> @@ -321,7 +321,7 @@
>>>>
>>>> /* update devname for mmap */
>>>> snprintf(devname, sizeof(devname),
>>>> - "%s/" PCI_PRI_FMT "/resource%d",
>>>> + "%s/" PCI_PRI_FMT "/resource%d_wc",
>>>> pci_get_sysfs_path(),
>>>> loc->domain, loc->bus, loc->devid,
>>>> loc->function, res_idx);
>>>> @@ -335,13 +335,22 @@
>>>> }
>>>>
>>>> /*
>>>> - * open resource file, to mmap it
>>>> + * open prefetchable resource file first, try to mmap it
>>>> */
>>>> fd = open(devname, O_RDWR);
>>>> if (fd < 0) {
>>>> - RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
>>>> - devname, strerror(errno));
>>>> - goto error;
>>>> + snprintf(devname, sizeof(devname),
>>>> + "%s/" PCI_PRI_FMT "/resource%d",
>>>> + 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;
>>>> + }
>>>> }
>>>>
>>>> /* try mapping somewhere close to the end of hugepages */
>>>>
>>>
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [dpdk-dev] [PATCH] pci/uio: enable prefetchable resources mapping
2018-10-28 1:59 ` Ferruh Yigit
@ 2018-10-29 0:48 ` Liu, Changpeng
0 siblings, 0 replies; 9+ messages in thread
From: Liu, Changpeng @ 2018-10-29 0:48 UTC (permalink / raw)
To: Yigit, Ferruh; +Cc: Richardson, Bruce, dpdk-dev, Rafal Kozik
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Sunday, October 28, 2018 9:59 AM
> To: Liu, Changpeng <changpeng.liu@intel.com>
> Cc: Richardson, Bruce <bruce.richardson@intel.com>; dpdk-dev
> <dev@dpdk.org>; Rafal Kozik <rk@semihalf.com>
> Subject: Re: [dpdk-dev] [PATCH] pci/uio: enable prefetchable resources mapping
>
> On 10/5/2017 9:33 AM, bruce.richardson at intel.com (Bruce Richardson) wrote:
> > On Thu, Oct 05, 2017 at 09:28:34AM +0100, Bruce Richardson wrote:
> >> On Thu, Oct 05, 2017 at 01:06:41AM +0100, Ferruh Yigit wrote:
> >>> On 6/3/2017 11:57 PM, Changpeng Liu wrote:
> >>>> For PCI prefetchable resources, Linux will create a
> >>>> write combined file as well, the library will try
> >>>> to map resourceX_wc file first, if the file does
> >>>> not exist, then it will map resourceX as usual.
> >>>
> >>> Hi Changpeng,
> >>>
> >>> Code part looks OK, but can you please describe more why we should try
> >>> write combined resource file first, what is the benefit of using it _wc
> >>> file?
> >>>
> >>> Thanks,
> >>> ferruh
> >>>
> >> Also, if we use a write combining resource file, I believe we will use
> > s/will use/will lose/
> >
> >> correct ordering of instructions within our drivers. Does applying this
> >> patch not also mean that we would need memory barriers inside all the
> >> drivers, so as to ensure that we don't have a queue doorbell write
> >> reordered with the descriptor writes? I don't think it's safe to apply
> >> this change on it's own, without driver changes, since all PMDs assume
> >> strong ordering on IA.
> >>
> >> Regards,
> >> /Bruce
>
> Hi Changpeng,
>
> Following patch looks like superseded this patch, can you please check it. I
> will update the status of this patch in patchwork.
>
> https://patches.dpdk.org/project/dpdk/list/?series=323&state=%2A&archive=bot
> h
> https://patches.dpdk.org/patch/41971/
Yeah, this feature has been enabled with DPDK now, you can update the patch status from me.
>
>
> >>
> >>>
> >>>>
> >>>> Signed-off-by: Changpeng Liu <changpeng.liu at intel.com>
> >>>> ---
> >>>> lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 19 ++++++++++++++-----
> >>>> 1 file changed, 14 insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> >>>> index fa10329..d9fc20a 100644
> >>>> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> >>>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> >>>> @@ -321,7 +321,7 @@
> >>>>
> >>>> /* update devname for mmap */
> >>>> snprintf(devname, sizeof(devname),
> >>>> - "%s/" PCI_PRI_FMT "/resource%d",
> >>>> + "%s/" PCI_PRI_FMT "/resource%d_wc",
> >>>> pci_get_sysfs_path(),
> >>>> loc->domain, loc->bus, loc->devid,
> >>>> loc->function, res_idx);
> >>>> @@ -335,13 +335,22 @@
> >>>> }
> >>>>
> >>>> /*
> >>>> - * open resource file, to mmap it
> >>>> + * open prefetchable resource file first, try to mmap it
> >>>> */
> >>>> fd = open(devname, O_RDWR);
> >>>> if (fd < 0) {
> >>>> - RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
> >>>> - devname, strerror(errno));
> >>>> - goto error;
> >>>> + snprintf(devname, sizeof(devname),
> >>>> + "%s/" PCI_PRI_FMT "/resource%d",
> >>>> + 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;
> >>>> + }
> >>>> }
> >>>>
> >>>> /* try mapping somewhere close to the end of hugepages */
> >>>>
> >>>
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
* [dpdk-dev] [PATCH] pci/uio: enable prefetchable resources mapping
@ 2018-02-01 1:18 Changpeng Liu
2018-02-01 9:59 ` Bruce Richardson
0 siblings, 1 reply; 9+ messages in thread
From: Changpeng Liu @ 2018-02-01 1:18 UTC (permalink / raw)
To: dev; +Cc: changpeng.liu, ferruh.yigit
For PCI prefetchable resources, Linux will create a
write combined file as well, the library will try
to map resourceX_wc file first, if the file does
not exist, then it will map resourceX as usual.
Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
---
drivers/bus/pci/linux/pci_uio.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/bus/pci/linux/pci_uio.c b/drivers/bus/pci/linux/pci_uio.c
index d423e4b..2957a5f 100644
--- a/drivers/bus/pci/linux/pci_uio.c
+++ b/drivers/bus/pci/linux/pci_uio.c
@@ -293,7 +293,7 @@
/* update devname for mmap */
snprintf(devname, sizeof(devname),
- "%s/" PCI_PRI_FMT "/resource%d",
+ "%s/" PCI_PRI_FMT "/resource%d_wc",
rte_pci_get_sysfs_path(),
loc->domain, loc->bus, loc->devid,
loc->function, res_idx);
@@ -307,13 +307,22 @@
}
/*
- * open resource file, to mmap it
+ * open prefetchable resource file first, try to mmap it
*/
fd = open(devname, O_RDWR);
if (fd < 0) {
- RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
- devname, strerror(errno));
- goto error;
+ 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;
+ }
}
/* try mapping somewhere close to the end of hugepages */
--
1.9.3
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [dpdk-dev] [PATCH] pci/uio: enable prefetchable resources mapping
2018-02-01 1:18 Changpeng Liu
@ 2018-02-01 9:59 ` Bruce Richardson
2018-02-01 10:08 ` Andrew Rybchenko
0 siblings, 1 reply; 9+ messages in thread
From: Bruce Richardson @ 2018-02-01 9:59 UTC (permalink / raw)
To: Changpeng Liu; +Cc: dev, ferruh.yigit
On Thu, Feb 01, 2018 at 09:18:22AM +0800, Changpeng Liu wrote:
> For PCI prefetchable resources, Linux will create a
> write combined file as well, the library will try
> to map resourceX_wc file first, if the file does
> not exist, then it will map resourceX as usual.
>
> Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
> ---
> drivers/bus/pci/linux/pci_uio.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
Hi,
Given the lack of ordering guarantees with write-combined memory, I
would have thought that this is very risky to do without a complete set
of changes inside the PMDs to add in the necessary memory barriers to
ensure ordering of operations to the BARs. Therefore, instead of
mapping one file or another, I think the change should be made to map
*both* in DPDK if available. Then each driver can chose whether to write
a given device register using uncacheable memory type or write-combining
memory type + any appropriate barriers.
For example, with many NICs the initialization of the device involves
many register writes in a pretty defined order, so wc operations are
probably to suitable as performance is not a concern. However, for data
path operations, a driver may chose to use wc memory for the occasional
device writes there, for performance reasons.
Regards,
/Bruce
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] pci/uio: enable prefetchable resources mapping
2018-02-01 9:59 ` Bruce Richardson
@ 2018-02-01 10:08 ` Andrew Rybchenko
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Rybchenko @ 2018-02-01 10:08 UTC (permalink / raw)
To: Bruce Richardson, Changpeng Liu; +Cc: dev, ferruh.yigit
On 02/01/2018 12:59 PM, Bruce Richardson wrote:
> On Thu, Feb 01, 2018 at 09:18:22AM +0800, Changpeng Liu wrote:
>> For PCI prefetchable resources, Linux will create a
>> write combined file as well, the library will try
>> to map resourceX_wc file first, if the file does
>> not exist, then it will map resourceX as usual.
>>
>> Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
>> ---
>> drivers/bus/pci/linux/pci_uio.c | 19 ++++++++++++++-----
>> 1 file changed, 14 insertions(+), 5 deletions(-)
>>
> Hi,
>
> Given the lack of ordering guarantees with write-combined memory, I
> would have thought that this is very risky to do without a complete set
> of changes inside the PMDs to add in the necessary memory barriers to
> ensure ordering of operations to the BARs. Therefore, instead of
> mapping one file or another, I think the change should be made to map
> *both* in DPDK if available. Then each driver can chose whether to write
> a given device register using uncacheable memory type or write-combining
> memory type + any appropriate barriers.
>
> For example, with many NICs the initialization of the device involves
> many register writes in a pretty defined order, so wc operations are
> probably to suitable as performance is not a concern. However, for data
> path operations, a driver may chose to use wc memory for the occasional
> device writes there, for performance reasons.
+1
I think so too that it would be useful to have both mappings available and
allow driver to choose which one to use.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-10-29 0:48 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-03 22:57 [dpdk-dev] [PATCH] pci/uio: enable prefetchable resources mapping Changpeng Liu
2017-10-05 0:06 ` Ferruh Yigit
2017-10-05 8:28 ` Bruce Richardson
2017-10-05 8:33 ` Bruce Richardson
2018-10-28 1:59 ` Ferruh Yigit
2018-10-29 0:48 ` Liu, Changpeng
2018-02-01 1:18 Changpeng Liu
2018-02-01 9:59 ` Bruce Richardson
2018-02-01 10:08 ` Andrew Rybchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).