[v3] lib/librte_eal/linuxapp: fix runtime config mmap issue
Checks
Commit Message
In rte_eal_config_reattach(),the secondary mmap may fail
due to the rte_mem_cfg_addr already be used by others.do
the change just as the rte_fbarray_init() do.if no
base_virtaddr,use the default 0x100000000.
v2/v3:
-fix code style issues
Signed-off-by: Li Han <han.li1@zte.com.cn>
---
lib/librte_eal/linux/eal/eal.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
Comments
On 23-Oct-19 11:42 AM, Li Han wrote:
> In rte_eal_config_reattach(),the secondary mmap may fail
> due to the rte_mem_cfg_addr already be used by others.do
> the change just as the rte_fbarray_init() do.if no
> base_virtaddr,use the default 0x100000000.
>
> v2/v3:
> -fix code style issues
>
> Signed-off-by: Li Han <han.li1@zte.com.cn>
> ---
Nack. There's a reason why we map it at the same address, and it's to
have all pointers working across processes. Remapping it at a different
address has potential to break things.
On Wed, Oct 23, 2019 at 7:47 PM Burakov, Anatoly
<anatoly.burakov@intel.com> wrote:
>
> On 23-Oct-19 11:42 AM, Li Han wrote:
> > In rte_eal_config_reattach(),the secondary mmap may fail
> > due to the rte_mem_cfg_addr already be used by others.do
> > the change just as the rte_fbarray_init() do.if no
> > base_virtaddr,use the default 0x100000000.
> >
> > v2/v3:
> > -fix code style issues
> >
> > Signed-off-by: Li Han <han.li1@zte.com.cn>
> > ---
>
> Nack. There's a reason why we map it at the same address, and it's to
> have all pointers working across processes. Remapping it at a different
> address has potential to break things.
Marked as rejected.
Thanks.
I didn't modified the attach function,I modified the rte_eal_config_create.
primay process first mmap's address, use a lower address, to avoid
secondary process mmap the fixed address fail. the secondary still
mmap the same address as the primary.
dpdk do the same thing in rte_fbarray_init() function.
------------------origin------------------
from：DavidMarchand <david.marchand@redhat.com>
to：Burakov, Anatoly <anatoly.burakov@intel.com>;
copy：hanli.li1@zte.com.cn;Thomas Monjalon <thomas@monjalon.net>;dev <dev@dpdk.org>;
date ：2019年10月24日 15:38
title ：Re: [dpdk-dev] [PATCH v3] lib/librte_eal/linuxapp: fix runtime configmmap issue
On Wed, Oct 23, 2019 at 7:47 PM Burakov, Anatoly
<anatoly.burakov@intel.com> wrote:
>
> On 23-Oct-19 11:42 AM, Li Han wrote:
> > In rte_eal_config_reattach(),the secondary mmap may fail
> > due to the rte_mem_cfg_addr already be used by others.do
> > the change just as the rte_fbarray_init() do.if no
> > base_virtaddr,use the default 0x100000000.
> >
> > v2/v3:
> > -fix code style issues
> >
> > Signed-off-by: Li Han <han.li1@zte.com.cn>
> > ---
>
> Nack. There's a reason why we map it at the same address, and it's to
> have all pointers working across processes. Remapping it at a different
> address has potential to break things.
Marked as rejected.
Thanks.
--
David Marchand
On 23-Oct-19 11:42 AM, Li Han wrote:
> In rte_eal_config_reattach(),the secondary mmap may fail
> due to the rte_mem_cfg_addr already be used by others.do
> the change just as the rte_fbarray_init() do.if no
> base_virtaddr,use the default 0x100000000.
>
> v2/v3:
> -fix code style issues
>
> Signed-off-by: Li Han <han.li1@zte.com.cn>
> ---
My apologies for earlier NACK, apparently didn't have enough coffee...
I've looked through the patch, and it's a good change.
Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>
On 24-Oct-19 8:37 AM, David Marchand wrote:
> On Wed, Oct 23, 2019 at 7:47 PM Burakov, Anatoly
> <anatoly.burakov@intel.com> wrote:
>>
>> On 23-Oct-19 11:42 AM, Li Han wrote:
>>> In rte_eal_config_reattach(),the secondary mmap may fail
>>> due to the rte_mem_cfg_addr already be used by others.do
>>> the change just as the rte_fbarray_init() do.if no
>>> base_virtaddr,use the default 0x100000000.
>>>
>>> v2/v3:
>>> -fix code style issues
>>>
>>> Signed-off-by: Li Han <han.li1@zte.com.cn>
>>> ---
>>
>> Nack. There's a reason why we map it at the same address, and it's to
>> have all pointers working across processes. Remapping it at a different
>> address has potential to break things.
>
> Marked as rejected.
> Thanks.
>
Hi David,
My apologies, I've misinterpreted the intent of the patch. I am
rescinding my NACK.
On Thu, Oct 24, 2019 at 1:14 PM Burakov, Anatoly
<anatoly.burakov@intel.com> wrote:
>
> On 24-Oct-19 8:37 AM, David Marchand wrote:
> > On Wed, Oct 23, 2019 at 7:47 PM Burakov, Anatoly
> > <anatoly.burakov@intel.com> wrote:
> >>
> >> On 23-Oct-19 11:42 AM, Li Han wrote:
> >>> In rte_eal_config_reattach(),the secondary mmap may fail
> >>> due to the rte_mem_cfg_addr already be used by others.do
> >>> the change just as the rte_fbarray_init() do.if no
> >>> base_virtaddr,use the default 0x100000000.
> >>>
> >>> v2/v3:
> >>> -fix code style issues
> >>>
> >>> Signed-off-by: Li Han <han.li1@zte.com.cn>
> >>> ---
> >>
> >> Nack. There's a reason why we map it at the same address, and it's to
> >> have all pointers working across processes. Remapping it at a different
> >> address has potential to break things.
> >
> > Marked as rejected.
> > Thanks.
> >
>
> Hi David,
>
> My apologies, I've misinterpreted the intent of the patch. I am
> rescinding my NACK.
Ok, I will put it back in my queue.
No conflict with the work on
http://patchwork.dpdk.org/project/dpdk/list/?series=5854 ?
There was a comment by Stephen, btw.
--
David Marchand
On 24-Oct-19 12:23 PM, David Marchand wrote:
> On Thu, Oct 24, 2019 at 1:14 PM Burakov, Anatoly
> <anatoly.burakov@intel.com> wrote:
>>
>> On 24-Oct-19 8:37 AM, David Marchand wrote:
>>> On Wed, Oct 23, 2019 at 7:47 PM Burakov, Anatoly
>>> <anatoly.burakov@intel.com> wrote:
>>>>
>>>> On 23-Oct-19 11:42 AM, Li Han wrote:
>>>>> In rte_eal_config_reattach(),the secondary mmap may fail
>>>>> due to the rte_mem_cfg_addr already be used by others.do
>>>>> the change just as the rte_fbarray_init() do.if no
>>>>> base_virtaddr,use the default 0x100000000.
>>>>>
>>>>> v2/v3:
>>>>> -fix code style issues
>>>>>
>>>>> Signed-off-by: Li Han <han.li1@zte.com.cn>
>>>>> ---
>>>>
>>>> Nack. There's a reason why we map it at the same address, and it's to
>>>> have all pointers working across processes. Remapping it at a different
>>>> address has potential to break things.
>>>
>>> Marked as rejected.
>>> Thanks.
>>>
>>
>> Hi David,
>>
>> My apologies, I've misinterpreted the intent of the patch. I am
>> rescinding my NACK.
>
> Ok, I will put it back in my queue.
> No conflict with the work on
> http://patchwork.dpdk.org/project/dpdk/list/?series=5854 ?
> There was a comment by Stephen, btw.
>
Oh, i forgot about that patchset...
Yes, it seems to be doing the same thing, but in a more general way
(because i was actually fixing an issue on FreeBSD...). Let me get back
to you on this.
On 24-Oct-19 12:13 PM, Burakov, Anatoly wrote:
> On 23-Oct-19 11:42 AM, Li Han wrote:
>> In rte_eal_config_reattach(),the secondary mmap may fail
>> due to the rte_mem_cfg_addr already be used by others.do
>> the change just as the rte_fbarray_init() do.if no
>> base_virtaddr,use the default 0x100000000.
>>
>> v2/v3:
>> -fix code style issues
>>
>> Signed-off-by: Li Han <han.li1@zte.com.cn>
>> ---
>
> My apologies for earlier NACK, apparently didn't have enough coffee...
>
> I've looked through the patch, and it's a good change.
>
> Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>
>
Hi,
It seems that you've basically reimplemented an older patchset [1] that
i already sent. Could you please test it and see if it fixes your issue
as well?
[1] http://patchwork.dpdk.org/project/dpdk/list/?series=7043
On 24-Oct-19 1:22 PM, Burakov, Anatoly wrote:
> On 24-Oct-19 12:23 PM, David Marchand wrote:
>> On Thu, Oct 24, 2019 at 1:14 PM Burakov, Anatoly
>> <anatoly.burakov@intel.com> wrote:
>>>
>>> On 24-Oct-19 8:37 AM, David Marchand wrote:
>>>> On Wed, Oct 23, 2019 at 7:47 PM Burakov, Anatoly
>>>> <anatoly.burakov@intel.com> wrote:
>>>>>
>>>>> On 23-Oct-19 11:42 AM, Li Han wrote:
>>>>>> In rte_eal_config_reattach(),the secondary mmap may fail
>>>>>> due to the rte_mem_cfg_addr already be used by others.do
>>>>>> the change just as the rte_fbarray_init() do.if no
>>>>>> base_virtaddr,use the default 0x100000000.
>>>>>>
>>>>>> v2/v3:
>>>>>> -fix code style issues
>>>>>>
>>>>>> Signed-off-by: Li Han <han.li1@zte.com.cn>
>>>>>> ---
>>>>>
>>>>> Nack. There's a reason why we map it at the same address, and it's to
>>>>> have all pointers working across processes. Remapping it at a
>>>>> different
>>>>> address has potential to break things.
>>>>
>>>> Marked as rejected.
>>>> Thanks.
>>>>
>>>
>>> Hi David,
>>>
>>> My apologies, I've misinterpreted the intent of the patch. I am
>>> rescinding my NACK.
>>
>> Ok, I will put it back in my queue.
>> No conflict with the work on
>> http://patchwork.dpdk.org/project/dpdk/list/?series=5854 ?
>> There was a comment by Stephen, btw.
>>
>
> Oh, i forgot about that patchset...
>
> Yes, it seems to be doing the same thing, but in a more general way
> (because i was actually fixing an issue on FreeBSD...). Let me get back
> to you on this.
>
Yes, that patch is a superset of this one, so this can be discarded, i
think. Apologies for the back and forth.
it seems your patch is later then mine?
you rejected mine patch and then committed yours?
I can't understand....
------------------origin------------------
from：Burakov,Anatoly <anatoly.burakov@intel.com>
to：David Marchand <david.marchand@redhat.com>;
copy：han.li1;Thomas Monjalon <thomas@monjalon.net>;dev <dev@dpdk.org>;
date ：2019/10/24 20:39
title ：Re: [dpdk-dev] [PATCH v3] lib/librte_eal/linuxapp: fix runtime configmmap issue
On 24-Oct-19 1:22 PM, Burakov, Anatoly wrote:
> On 24-Oct-19 12:23 PM, David Marchand wrote:
>> On Thu, Oct 24, 2019 at 1:14 PM Burakov, Anatoly
>> <anatoly.burakov@intel.com> wrote:
>>>
>>> On 24-Oct-19 8:37 AM, David Marchand wrote:
>>>> On Wed, Oct 23, 2019 at 7:47 PM Burakov, Anatoly
>>>> <anatoly.burakov@intel.com> wrote:
>>>>>
>>>>> On 23-Oct-19 11:42 AM, Li Han wrote:
>>>>>> In rte_eal_config_reattach(),the secondary mmap may fail
>>>>>> due to the rte_mem_cfg_addr already be used by others.do
>>>>>> the change just as the rte_fbarray_init() do.if no
>>>>>> base_virtaddr,use the default 0x100000000.
>>>>>>
>>>>>> v2/v3:
>>>>>> -fix code style issues
>>>>>>
>>>>>> Signed-off-by: Li Han <han.li1@zte.com.cn>
>>>>>> ---
>>>>>
>>>>> Nack. There's a reason why we map it at the same address, and it's to
>>>>> have all pointers working across processes. Remapping it at a
>>>>> different
>>>>> address has potential to break things.
>>>>
>>>> Marked as rejected.
>>>> Thanks.
>>>>
>>>
>>> Hi David,
>>>
>>> My apologies, I've misinterpreted the intent of the patch. I am
>>> rescinding my NACK.
>>
>> Ok, I will put it back in my queue.
>> No conflict with the work on
>> http://patchwork.dpdk.org/project/dpdk/list/?series=5854 ?
>> There was a comment by Stephen, btw.
>>
>
> Oh, i forgot about that patchset...
>
> Yes, it seems to be doing the same thing, but in a more general way
> (because i was actually fixing an issue on FreeBSD...). Let me get back
> to you on this.
>
Yes, that patch is a superset of this one, so this can be discarded, i
think. Apologies for the back and forth.
--
Thanks,
Anatoly
On 25-Oct-19 12:37 PM, han.li1@zte.com.cn wrote:
> it seems your patch is later then mine?
> you rejected mine patch and then committed yours?
> I can't understand....
>
Hi,
Apologies for the confusion. My patch's v1 was sent in July this year,
much earlier than yours:
http://patches.dpdk.org/project/dpdk/list/?series=5813&state=%2A&archive=both
And it's a more general patch, that also fixes the issue for FreeBSD.
This is the patch that got committed - it came in first, and it fixes
the issue in a more general way, for Linux and for FreeBSD. It's just
that for some reason i was under the impression that it was already
accepted, but it was merely deferred till 19.11.
Again, my apologies for the confusion. I should have looked at your
patch more closely.
On Fri, Oct 25, 2019 at 2:20 PM Burakov, Anatoly
<anatoly.burakov@intel.com> wrote:
>
> On 25-Oct-19 12:37 PM, han.li1@zte.com.cn wrote:
> > it seems your patch is later then mine?
> > you rejected mine patch and then committed yours?
> > I can't understand....
> >
>
> Hi,
>
> Apologies for the confusion. My patch's v1 was sent in July this year,
> much earlier than yours:
>
> http://patches.dpdk.org/project/dpdk/list/?series=5813&state=%2A&archive=both
>
> And it's a more general patch, that also fixes the issue for FreeBSD.
> This is the patch that got committed - it came in first, and it fixes
> the issue in a more general way, for Linux and for FreeBSD. It's just
> that for some reason i was under the impression that it was already
> accepted, but it was merely deferred till 19.11.
>
> Again, my apologies for the confusion. I should have looked at your
> patch more closely.
I took Anatoly patches since his work on the subject preceded your patch.
Please, test the current master branch and see if you still have issues.
Thanks.
Hello Li Han,
On Sat, Oct 26, 2019 at 6:07 PM David Marchand
<david.marchand@redhat.com> wrote:
> I took Anatoly patches since his work on the subject preceded your patch.
> Please, test the current master branch and see if you still have issues.
Did you have a chance to test with the -rc1 tag?
Thanks.
Hello Li Han,
On Tue, Nov 5, 2019 at 3:14 PM David Marchand <david.marchand@redhat.com> wrote:
> On Sat, Oct 26, 2019 at 6:07 PM David Marchand
> <david.marchand@redhat.com> wrote:
> > I took Anatoly patches since his work on the subject preceded your patch.
> > Please, test the current master branch and see if you still have issues.
>
> Did you have a chance to test with the -rc1 tag?
I will mark this patch as rejected.
If you think it should still be considered, then please set this patch
state back to new in patchwork and explain what is missing in this
thread.
Thanks.
@@ -308,20 +308,32 @@ enum rte_iova_mode
{
void *rte_mem_cfg_addr;
int retval;
+ size_t page_sz, mmap_len;
const char *pathname = eal_runtime_config_path();
if (internal_config.no_shconf)
return 0;
-
+ mmap_len = sizeof(*rte_config.mem_config);
/* map the config before hugepage address so that we don't waste a page */
if (internal_config.base_virtaddr != 0)
rte_mem_cfg_addr = (void *)
RTE_ALIGN_FLOOR(internal_config.base_virtaddr -
sizeof(struct rte_mem_config), sysconf(_SC_PAGE_SIZE));
- else
- rte_mem_cfg_addr = NULL;
-
+ else {
+ page_sz = sysconf(_SC_PAGESIZE);
+ if (page_sz == (size_t)-1) {
+ RTE_LOG(ERR, EAL,
+ "Cannot get SC_PAGESIZE for rte_mem_config\n");
+ return -1;
+ }
+ rte_mem_cfg_addr = eal_get_virtual_area(NULL, &mmap_len, page_sz, 0, 0);
+ if (rte_mem_cfg_addr == NULL) {
+ RTE_LOG(ERR, EAL,
+ "Cannot get virtual addr for rte_mem_config\n");
+ return -1;
+ }
+ }
if (mem_cfg_fd < 0){
mem_cfg_fd = open(pathname, O_RDWR | O_CREAT, 0600);
if (mem_cfg_fd < 0) {
@@ -349,8 +361,10 @@ enum rte_iova_mode
return -1;
}
- rte_mem_cfg_addr = mmap(rte_mem_cfg_addr, sizeof(*rte_config.mem_config),
- PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd, 0);
+ rte_mem_cfg_addr = mmap(rte_mem_cfg_addr, mmap_len,
+ PROT_READ | PROT_WRITE,
+ MAP_FIXED | MAP_SHARED,
+ mem_cfg_fd, 0);
if (rte_mem_cfg_addr == MAP_FAILED){
close(mem_cfg_fd);