[dpdk-dev] Question regarding mempool changes impact to XEN PMD

[dpdk-dev] Question regarding mempool changes impact to XEN PMD

[Christian Ehrhardt]

Hi David,
it seems to be the first time I compiled with
CONFIG_RTE_LIBRTE_PMD_XENVIRT=y sinec the bigger mempool changes around
"587d684d doc: update release notes about mempool allocation".

I've seen related patch to mempool / xen in that regard "c042ba20 mempool:
rework support of Xen dom0"

But with above config symbol enabled I got:
drivers/net/xenvirt/rte_xen_lib.c: In function ‘grant_gntalloc_mbuf_pool’:
drivers/net/xenvirt/rte_xen_lib.c:440:69: error: ‘struct rte_mempool’ has
no member named ‘elt_va_start’
  if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR,
(uintptr_t)mpool->elt_va_start) == -1)
                                                                     ^
  SYMLINK-FILE include/rte_eth_bond.h
mk/internal/rte.compile-pre.mk:126: recipe for target 'rte_xen_lib.o' failed
make[4]: *** [rte_xen_lib.o] Error 1
make[4]: *** Waiting for unfinished jobs....

The change around the mempools is complex, so I don't see on the first look
if that needs a minor or major rework in the xen sources.
I mean I don't want it to compile, but to work and that could be more than
just fixing that changed structure :-)

So I wanted to ask if you as author would see if it is a trivial change
that has to be made?


Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

Re: [dpdk-dev] Question regarding mempool changes impact to XEN PMD

[Olivier Matz]

Hi Christian,

On 06/13/2016 09:34 AM, Christian Ehrhardt wrote:
> Hi David,

I guess this mail is for me, not for David :)

> it seems to be the first time I compiled with
> CONFIG_RTE_LIBRTE_PMD_XENVIRT=y sinec the bigger mempool changes around
> "587d684d doc: update release notes about mempool allocation".
> 
> I've seen related patch to mempool / xen in that regard "c042ba20 mempool:
> rework support of Xen dom0"
> 
> But with above config symbol enabled I got:
> drivers/net/xenvirt/rte_xen_lib.c: In function ‘grant_gntalloc_mbuf_pool’:
> drivers/net/xenvirt/rte_xen_lib.c:440:69: error: ‘struct rte_mempool’ has
> no member named ‘elt_va_start’
>   if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR,
> (uintptr_t)mpool->elt_va_start) == -1)
>                                                                      ^
>   SYMLINK-FILE include/rte_eth_bond.h
> mk/internal/rte.compile-pre.mk:126: recipe for target 'rte_xen_lib.o' failed
> make[4]: *** [rte_xen_lib.o] Error 1
> make[4]: *** Waiting for unfinished jobs....
> 
> The change around the mempools is complex, so I don't see on the first look
> if that needs a minor or major rework in the xen sources.
> I mean I don't want it to compile, but to work and that could be more than
> just fixing that changed structure :-)
> 
> So I wanted to ask if you as author would see if it is a trivial change
> that has to be made?

Sorry, I missed this reference to elt_va_start in my patches.

I'm not very familiar with the xen code in dpdk, but from what I see:

- in the PMD, grant_gntalloc_mbuf_pool() stores the mempool virtual
  address in the xen key/value database
- in examples/vhost_xen, the function parse_mpool_va() retrieves it
- this address is used in new_device()

I think the patch would be almost similar to what I did in mlx
drivers in this commit:
http://dpdk.org/browse/dpdk/commit/?id=84121f1971873c9f45b2939c316c66126d8754a1

or in librte_kni in this commit:
http://dpdk.org/browse/dpdk/commit?id=d1d914ebbc2514f334a3ed24057e63c8bb76363d

To give more precisions:

- before the patchset, mp->elt_va_start was the virtual address of the
  mempool objects table. It was always virtually contiguous

- now, a mempool can be fragmented in several virtually contiguous
  chunks. In case there is only one chunk, it can be safely replaced
  by STAILQ_FIRST(&mp->mem_list)->addr (= the virtual address of the
  first chunk).

In case there are more chunks in the mempool, it would require deeper
modifications I think. But we should keep in mind that having a
virtually fragmented mempool was not possible before the patchset
(it would have fail at init). If if fails later in xen code because
xen does not support fragmented mempools, there is no regression
compared to what we had before.

I'll send a draft patch, if you could give it a try, it would be great!

Thanks for reporting,
Olivier

[dpdk-dev] [PATCH] xenvirt: fix compilation after mempool changes

[Olivier Matz]

The field elt_va_start has been removed from the mempool structure,
and it was not replaced in xenvirt.

Fix this by getting the mempool objects address by using the address of
the first memory chunk list.

Note that it won't work with mempool composed of several chunks,
but it was already the case before.

Fixes: 84121f197187 ("mempool: store memory chunks in a list")
Reported-by: Christian Ehrhard <christian.ehrhardt@canonical.com>
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 drivers/net/xenvirt/rte_xen_lib.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/xenvirt/rte_xen_lib.c b/drivers/net/xenvirt/rte_xen_lib.c
index de63cd3..997e56e 100644
--- a/drivers/net/xenvirt/rte_xen_lib.c
+++ b/drivers/net/xenvirt/rte_xen_lib.c
@@ -423,6 +423,7 @@ grant_gntalloc_mbuf_pool(struct rte_mempool *mpool, uint32_t pg_num, uint32_t *g
 {
 	char key_str[PATH_MAX] = {0};
 	char val_str[PATH_MAX] = {0};
+	void *mempool_obj_va;
 
 	if (grant_node_create(pg_num, gref_arr, pa_arr, val_str, sizeof(val_str))) {
 		return -1;
@@ -437,7 +438,14 @@ grant_gntalloc_mbuf_pool(struct rte_mempool *mpool, uint32_t pg_num, uint32_t *g
 	if (snprintf(key_str, sizeof(key_str),
 		DPDK_XENSTORE_PATH"%d"MEMPOOL_VA_XENSTORE_STR, mempool_idx) == -1)
 		return -1;
-	if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR, (uintptr_t)mpool->elt_va_start) == -1)
+	if (mp->nb_mem_chunks != 1) {
+		RTE_LOG(ERR, PMD,
+			"mempool with more than 1 chunk is not supported\n");
+		return -1;
+	}
+	mempool_obj_va = STAILQ_FIRST(&mp->mem_list)->addr;
+	if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR,
+			(uintptr_t)mempool_obj_va) == -1)
 		return -1;
 	if (xenstore_write(key_str, val_str) == -1)
 		return -1;
-- 
2.8.0.rc3

Re: [dpdk-dev] [PATCH] xenvirt: fix compilation after mempool changes

[Christian Ehrhardt]

Hi Oliver,
thanks for the fast response!

It fixes the compilation issue and I totally agree to your argument of the
multi-chunk issues being out of scope for this as they never worked.
Unfortunately I lack an environment to actually test this in real-life if
we need any more follow up than this.

Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>



Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Mon, Jun 13, 2016 at 10:22 AM, Olivier Matz <olivier.matz@6wind.com>
wrote:

> The field elt_va_start has been removed from the mempool structure,
> and it was not replaced in xenvirt.
>
> Fix this by getting the mempool objects address by using the address of
> the first memory chunk list.
>
> Note that it won't work with mempool composed of several chunks,
> but it was already the case before.
>
> Fixes: 84121f197187 ("mempool: store memory chunks in a list")
> Reported-by: Christian Ehrhard <christian.ehrhardt@canonical.com>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>  drivers/net/xenvirt/rte_xen_lib.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/xenvirt/rte_xen_lib.c
> b/drivers/net/xenvirt/rte_xen_lib.c
> index de63cd3..997e56e 100644
> --- a/drivers/net/xenvirt/rte_xen_lib.c
> +++ b/drivers/net/xenvirt/rte_xen_lib.c
> @@ -423,6 +423,7 @@ grant_gntalloc_mbuf_pool(struct rte_mempool *mpool,
> uint32_t pg_num, uint32_t *g
>  {
>         char key_str[PATH_MAX] = {0};
>         char val_str[PATH_MAX] = {0};
> +       void *mempool_obj_va;
>
>         if (grant_node_create(pg_num, gref_arr, pa_arr, val_str,
> sizeof(val_str))) {
>                 return -1;
> @@ -437,7 +438,14 @@ grant_gntalloc_mbuf_pool(struct rte_mempool *mpool,
> uint32_t pg_num, uint32_t *g
>         if (snprintf(key_str, sizeof(key_str),
>                 DPDK_XENSTORE_PATH"%d"MEMPOOL_VA_XENSTORE_STR,
> mempool_idx) == -1)
>                 return -1;
> -       if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR,
> (uintptr_t)mpool->elt_va_start) == -1)
> +       if (mp->nb_mem_chunks != 1) {
> +               RTE_LOG(ERR, PMD,
> +                       "mempool with more than 1 chunk is not
> supported\n");
> +               return -1;
> +       }
> +       mempool_obj_va = STAILQ_FIRST(&mp->mem_list)->addr;
> +       if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR,
> +                       (uintptr_t)mempool_obj_va) == -1)
>                 return -1;
>         if (xenstore_write(key_str, val_str) == -1)
>                 return -1;
> --
> 2.8.0.rc3
>
>

Re: [dpdk-dev] [PATCH] xenvirt: fix compilation after mempool changes

[Christian Ehrhardt]

Hmm,
Hi again Oliver.

I was too fast saying yes.
I don't know what is different now but I clearly tested it wrong the first
time.
Now I get:

  CC rte_xen_lib.o
/mnt/nvme/dpdk-16-07-pre-linking/drivers/net/xenvirt/rte_xen_lib.c: In
function ‘grant_gntalloc_mbuf_pool’:
/mnt/nvme/dpdk-16-07-pre-linking/drivers/net/xenvirt/rte_xen_lib.c:441:6:
error:
‘mp’ undeclared (first use in this function)
 if (mp->nb_mem_chunks != 1) {
     ^
/mnt/nvme/dpdk-16-07-pre-linking/drivers/net/xenvirt/rte_xen_lib.c:441:6: note:
each undeclared identifier is reported only once for each function it
appears in
/mnt/nvme/dpdk-16-07-pre-linking/drivers/net/xenvirt/rte_xen_lib.c:422:46:
error:
unused parameter ‘mpool’ [-Werror=unused-parameter]
grant_gntalloc_mbuf_pool(struct rte_mempool *mpool, uint32_t pg_num,
uint32_t *gref_arr, phys_addr_t *pa_arr, int mempool_idx)
                                             ^
cc1: all warnings being treated as errors


Not too hard, changing the mp to mpool on the two places the patch has
inserted it gets it working.




Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Mon, Jun 13, 2016 at 10:51 AM, Christian Ehrhardt <
christian.ehrhardt@canonical.com> wrote:

> Hi Oliver,
> thanks for the fast response!
>
> It fixes the compilation issue and I totally agree to your argument of the
> multi-chunk issues being out of scope for this as they never worked.
> Unfortunately I lack an environment to actually test this in real-life if
> we need any more follow up than this.
>
> Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
>
>
>
> Christian Ehrhardt
> Software Engineer, Ubuntu Server
> Canonical Ltd
>
> On Mon, Jun 13, 2016 at 10:22 AM, Olivier Matz <olivier.matz@6wind.com>
> wrote:
>
>> The field elt_va_start has been removed from the mempool structure,
>> and it was not replaced in xenvirt.
>>
>> Fix this by getting the mempool objects address by using the address of
>> the first memory chunk list.
>>
>> Note that it won't work with mempool composed of several chunks,
>> but it was already the case before.
>>
>> Fixes: 84121f197187 ("mempool: store memory chunks in a list")
>> Reported-by: Christian Ehrhard <christian.ehrhardt@canonical.com>
>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
>> ---
>>  drivers/net/xenvirt/rte_xen_lib.c | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/xenvirt/rte_xen_lib.c
>> b/drivers/net/xenvirt/rte_xen_lib.c
>> index de63cd3..997e56e 100644
>> --- a/drivers/net/xenvirt/rte_xen_lib.c
>> +++ b/drivers/net/xenvirt/rte_xen_lib.c
>> @@ -423,6 +423,7 @@ grant_gntalloc_mbuf_pool(struct rte_mempool *mpool,
>> uint32_t pg_num, uint32_t *g
>>  {
>>         char key_str[PATH_MAX] = {0};
>>         char val_str[PATH_MAX] = {0};
>> +       void *mempool_obj_va;
>>
>>         if (grant_node_create(pg_num, gref_arr, pa_arr, val_str,
>> sizeof(val_str))) {
>>                 return -1;
>> @@ -437,7 +438,14 @@ grant_gntalloc_mbuf_pool(struct rte_mempool *mpool,
>> uint32_t pg_num, uint32_t *g
>>         if (snprintf(key_str, sizeof(key_str),
>>                 DPDK_XENSTORE_PATH"%d"MEMPOOL_VA_XENSTORE_STR,
>> mempool_idx) == -1)
>>                 return -1;
>> -       if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR,
>> (uintptr_t)mpool->elt_va_start) == -1)
>> +       if (mp->nb_mem_chunks != 1) {
>> +               RTE_LOG(ERR, PMD,
>> +                       "mempool with more than 1 chunk is not
>> supported\n");
>> +               return -1;
>> +       }
>> +       mempool_obj_va = STAILQ_FIRST(&mp->mem_list)->addr;
>> +       if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR,
>> +                       (uintptr_t)mempool_obj_va) == -1)
>>                 return -1;
>>         if (xenstore_write(key_str, val_str) == -1)
>>                 return -1;
>> --
>> 2.8.0.rc3
>>
>>
>

Re: [dpdk-dev] [PATCH] xenvirt: fix compilation after mempool changes

[Olivier Matz]

On 06/13/2016 11:10 AM, Christian Ehrhardt wrote:
> Hmm,
> Hi again Oliver.
> 
> I was too fast saying yes.
> I don't know what is different now but I clearly tested it wrong the
> first time.
> Now I get:
> 
>   CC rte_xen_lib.o
> /mnt/nvme/dpdk-16-07-pre-linking/drivers/net/xenvirt/rte_xen_lib.c:In
> function ‘grant_gntalloc_mbuf_pool’:
> /mnt/nvme/dpdk-16-07-pre-linking/drivers/net/xenvirt/rte_xen_lib.c:441:6:error:
> ‘mp’ undeclared (first use in this function)
>  if (mp->nb_mem_chunks != 1) {
>      ^
> /mnt/nvme/dpdk-16-07-pre-linking/drivers/net/xenvirt/rte_xen_lib.c:441:6:note:
> each undeclared identifier is reported only once for each function it
> appears in
> /mnt/nvme/dpdk-16-07-pre-linking/drivers/net/xenvirt/rte_xen_lib.c:422:46:error:
> unused parameter ‘mpool’ [-Werror=unused-parameter]
> grant_gntalloc_mbuf_pool(struct rte_mempool *mpool, uint32_t pg_num,
> uint32_t *gref_arr, phys_addr_t *pa_arr, int mempool_idx)
>                                              ^
> cc1: all warnings being treated as errors
> 
> 
> Not too hard, changing the mp to mpool on the two places the patch has
> inserted it gets it working.
> 

Thanks, and sorry I did not test the compilation because I don't have
the xen libraries installed.

This shows that even reading the patch several times is less efficient
than a compiler ;)

I'll send a v2 soon.

[dpdk-dev] [PATCH v2] xenvirt: fix compilation after mempool changes

[Olivier Matz]

The field elt_va_start has been removed from the mempool structure,
and it was not replaced in xenvirt.

Fix this by getting the mempool objects address by using the address of
the first memory chunk list.

Note that it won't work with mempool composed of several chunks,
but it was already the case before.

Fixes: 84121f197187 ("mempool: store memory chunks in a list")
Reported-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
---

v1->v2:
- fix mempool variable name
- fix typo in Reported-by

 drivers/net/xenvirt/rte_xen_lib.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/xenvirt/rte_xen_lib.c b/drivers/net/xenvirt/rte_xen_lib.c
index de63cd3..6c9a1d4 100644
--- a/drivers/net/xenvirt/rte_xen_lib.c
+++ b/drivers/net/xenvirt/rte_xen_lib.c
@@ -423,6 +423,7 @@ grant_gntalloc_mbuf_pool(struct rte_mempool *mpool, uint32_t pg_num, uint32_t *g
 {
 	char key_str[PATH_MAX] = {0};
 	char val_str[PATH_MAX] = {0};
+	void *mempool_obj_va;
 
 	if (grant_node_create(pg_num, gref_arr, pa_arr, val_str, sizeof(val_str))) {
 		return -1;
@@ -437,7 +438,14 @@ grant_gntalloc_mbuf_pool(struct rte_mempool *mpool, uint32_t pg_num, uint32_t *g
 	if (snprintf(key_str, sizeof(key_str),
 		DPDK_XENSTORE_PATH"%d"MEMPOOL_VA_XENSTORE_STR, mempool_idx) == -1)
 		return -1;
-	if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR, (uintptr_t)mpool->elt_va_start) == -1)
+	if (mpool->nb_mem_chunks != 1) {
+		RTE_LOG(ERR, PMD,
+			"mempool with more than 1 chunk is not supported\n");
+		return -1;
+	}
+	mempool_obj_va = STAILQ_FIRST(&mpool->mem_list)->addr;
+	if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR,
+			(uintptr_t)mempool_obj_va) == -1)
 		return -1;
 	if (xenstore_write(key_str, val_str) == -1)
 		return -1;
-- 
2.8.0.rc3

Re: [dpdk-dev] [PATCH v2] xenvirt: fix compilation after mempool changes

[Christian Ehrhardt]

Yeah, working now - thanks for the fast update!

Kind Regards,
Christian

Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Mon, Jun 13, 2016 at 1:24 PM, Olivier Matz <olivier.matz@6wind.com>
wrote:

> The field elt_va_start has been removed from the mempool structure,
> and it was not replaced in xenvirt.
>
> Fix this by getting the mempool objects address by using the address of
> the first memory chunk list.
>
> Note that it won't work with mempool composed of several chunks,
> but it was already the case before.
>
> Fixes: 84121f197187 ("mempool: store memory chunks in a list")
> Reported-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> ---
>
> v1->v2:
> - fix mempool variable name
> - fix typo in Reported-by
>
>  drivers/net/xenvirt/rte_xen_lib.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/xenvirt/rte_xen_lib.c
> b/drivers/net/xenvirt/rte_xen_lib.c
> index de63cd3..6c9a1d4 100644
> --- a/drivers/net/xenvirt/rte_xen_lib.c
> +++ b/drivers/net/xenvirt/rte_xen_lib.c
> @@ -423,6 +423,7 @@ grant_gntalloc_mbuf_pool(struct rte_mempool *mpool,
> uint32_t pg_num, uint32_t *g
>  {
>         char key_str[PATH_MAX] = {0};
>         char val_str[PATH_MAX] = {0};
> +       void *mempool_obj_va;
>
>         if (grant_node_create(pg_num, gref_arr, pa_arr, val_str,
> sizeof(val_str))) {
>                 return -1;
> @@ -437,7 +438,14 @@ grant_gntalloc_mbuf_pool(struct rte_mempool *mpool,
> uint32_t pg_num, uint32_t *g
>         if (snprintf(key_str, sizeof(key_str),
>                 DPDK_XENSTORE_PATH"%d"MEMPOOL_VA_XENSTORE_STR,
> mempool_idx) == -1)
>                 return -1;
> -       if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR,
> (uintptr_t)mpool->elt_va_start) == -1)
> +       if (mpool->nb_mem_chunks != 1) {
> +               RTE_LOG(ERR, PMD,
> +                       "mempool with more than 1 chunk is not
> supported\n");
> +               return -1;
> +       }
> +       mempool_obj_va = STAILQ_FIRST(&mpool->mem_list)->addr;
> +       if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR,
> +                       (uintptr_t)mempool_obj_va) == -1)
>                 return -1;
>         if (xenstore_write(key_str, val_str) == -1)
>                 return -1;
> --
> 2.8.0.rc3
>
>

Re: [dpdk-dev] [PATCH v2] xenvirt: fix compilation after mempool changes

[Bruce Richardson]

On Mon, Jun 13, 2016 at 01:54:29PM +0200, Christian Ehrhardt wrote:
> Yeah, working now - thanks for the fast update!
> 
> Kind Regards,
> Christian
> 
> Christian Ehrhardt
> Software Engineer, Ubuntu Server
> Canonical Ltd
> 
Applied to dpdk-next-net/rel_16_07

/Bruce

Re: [dpdk-dev] Question regarding mempool changes impact to XEN PMD

[Christian Ehrhardt]

On Mon, Jun 13, 2016 at 10:14 AM, Olivier Matz <olivier.matz@6wind.com>
wrote:

> Hi Christian,
>
> On 06/13/2016 09:34 AM, Christian Ehrhardt wrote:
> > Hi David,
>
> I guess this mail is for me, not for David :)
>

Absolutely yes, sorry to both of you to - probably read too much patch
headers this morning :-)


> > it seems to be the first time I compiled with
> > CONFIG_RTE_LIBRTE_PMD_XENVIRT=y sinec the bigger mempool changes around
> > "587d684d doc: update release notes about mempool allocation".
> >
> > I've seen related patch to mempool / xen in that regard "c042ba20
> mempool:
> > rework support of Xen dom0"
> >
> > But with above config symbol enabled I got:
> > drivers/net/xenvirt/rte_xen_lib.c: In function
> ‘grant_gntalloc_mbuf_pool’:
> > drivers/net/xenvirt/rte_xen_lib.c:440:69: error: ‘struct rte_mempool’ has
> > no member named ‘elt_va_start’
> >   if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR,
> > (uintptr_t)mpool->elt_va_start) == -1)
> >                                                                      ^
> >   SYMLINK-FILE include/rte_eth_bond.h
> > mk/internal/rte.compile-pre.mk:126: recipe for target 'rte_xen_lib.o'
> failed
> > make[4]: *** [rte_xen_lib.o] Error 1
> > make[4]: *** Waiting for unfinished jobs....
> >
> > The change around the mempools is complex, so I don't see on the first
> look
> > if that needs a minor or major rework in the xen sources.
> > I mean I don't want it to compile, but to work and that could be more
> than
> > just fixing that changed structure :-)
> >
> > So I wanted to ask if you as author would see if it is a trivial change
> > that has to be made?
>
> Sorry, I missed this reference to elt_va_start in my patches.
>
> I'm not very familiar with the xen code in dpdk, but from what I see:
>
> - in the PMD, grant_gntalloc_mbuf_pool() stores the mempool virtual
>   address in the xen key/value database
> - in examples/vhost_xen, the function parse_mpool_va() retrieves it
> - this address is used in new_device()
>
> I think the patch would be almost similar to what I did in mlx
> drivers in this commit:
>
> http://dpdk.org/browse/dpdk/commit/?id=84121f1971873c9f45b2939c316c66126d8754a1
>
> or in librte_kni in this commit:
>
> http://dpdk.org/browse/dpdk/commit?id=d1d914ebbc2514f334a3ed24057e63c8bb76363d
>
> To give more precisions:
>
> - before the patchset, mp->elt_va_start was the virtual address of the
>   mempool objects table. It was always virtually contiguous
>
> - now, a mempool can be fragmented in several virtually contiguous
>   chunks. In case there is only one chunk, it can be safely replaced
>   by STAILQ_FIRST(&mp->mem_list)->addr (= the virtual address of the
>   first chunk).
>
> In case there are more chunks in the mempool, it would require deeper
> modifications I think. But we should keep in mind that having a
> virtually fragmented mempool was not possible before the patchset
> (it would have fail at init). If if fails later in xen code because
> xen does not support fragmented mempools, there is no regression
> compared to what we had before.
>

Ack to that, I only cared about the regression and that I think you covered
excellently.
To make fragmented pools work would be a task for one who "cares" to use it.


>
> I'll send a draft patch, if you could give it a try, it would be great!
>

I can compile and review the patch, but I neither have a setup to actually
run it.
Maybe someone else on the list have, please feel encouraged to do so.


> Thanks for reporting,
> Olivier
>
>