All of lore.kernel.org
 help / color / mirror / Atom feed
* Question regarding mempool changes impact to XEN PMD
@ 2016-06-13  7:34 Christian Ehrhardt
  2016-06-13  8:14 ` Olivier Matz
  0 siblings, 1 reply; 10+ messages in thread
From: Christian Ehrhardt @ 2016-06-13  7:34 UTC (permalink / raw)
  To: David Marchand, dev

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Question regarding mempool changes impact to XEN PMD
  2016-06-13  7:34 Question regarding mempool changes impact to XEN PMD Christian Ehrhardt
@ 2016-06-13  8:14 ` Olivier Matz
  2016-06-13  8:22   ` [PATCH] xenvirt: fix compilation after mempool changes Olivier Matz
  2016-06-13  8:30   ` Question regarding mempool changes impact to XEN PMD Christian Ehrhardt
  0 siblings, 2 replies; 10+ messages in thread
From: Olivier Matz @ 2016-06-13  8:14 UTC (permalink / raw)
  To: Christian Ehrhardt, David Marchand, dev

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] xenvirt: fix compilation after mempool changes
  2016-06-13  8:14 ` Olivier Matz
@ 2016-06-13  8:22   ` Olivier Matz
  2016-06-13  8:51     ` Christian Ehrhardt
  2016-06-13 11:24     ` [PATCH v2] " Olivier Matz
  2016-06-13  8:30   ` Question regarding mempool changes impact to XEN PMD Christian Ehrhardt
  1 sibling, 2 replies; 10+ messages in thread
From: Olivier Matz @ 2016-06-13  8:22 UTC (permalink / raw)
  To: christian.ehrhardt, david.marchand, dev

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: Question regarding mempool changes impact to XEN PMD
  2016-06-13  8:14 ` Olivier Matz
  2016-06-13  8:22   ` [PATCH] xenvirt: fix compilation after mempool changes Olivier Matz
@ 2016-06-13  8:30   ` Christian Ehrhardt
  1 sibling, 0 replies; 10+ messages in thread
From: Christian Ehrhardt @ 2016-06-13  8:30 UTC (permalink / raw)
  To: Olivier Matz; +Cc: David Marchand, dev

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
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] xenvirt: fix compilation after mempool changes
  2016-06-13  8:22   ` [PATCH] xenvirt: fix compilation after mempool changes Olivier Matz
@ 2016-06-13  8:51     ` Christian Ehrhardt
  2016-06-13  9:10       ` Christian Ehrhardt
  2016-06-13 11:24     ` [PATCH v2] " Olivier Matz
  1 sibling, 1 reply; 10+ messages in thread
From: Christian Ehrhardt @ 2016-06-13  8:51 UTC (permalink / raw)
  To: Olivier Matz; +Cc: David Marchand, dev

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
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] xenvirt: fix compilation after mempool changes
  2016-06-13  8:51     ` Christian Ehrhardt
@ 2016-06-13  9:10       ` Christian Ehrhardt
  2016-06-13  9:13         ` Olivier Matz
  0 siblings, 1 reply; 10+ messages in thread
From: Christian Ehrhardt @ 2016-06-13  9:10 UTC (permalink / raw)
  To: Olivier Matz; +Cc: David Marchand, dev

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
>>
>>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] xenvirt: fix compilation after mempool changes
  2016-06-13  9:10       ` Christian Ehrhardt
@ 2016-06-13  9:13         ` Olivier Matz
  0 siblings, 0 replies; 10+ messages in thread
From: Olivier Matz @ 2016-06-13  9:13 UTC (permalink / raw)
  To: Christian Ehrhardt; +Cc: David Marchand, dev



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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] xenvirt: fix compilation after mempool changes
  2016-06-13  8:22   ` [PATCH] xenvirt: fix compilation after mempool changes Olivier Matz
  2016-06-13  8:51     ` Christian Ehrhardt
@ 2016-06-13 11:24     ` Olivier Matz
  2016-06-13 11:54       ` Christian Ehrhardt
  1 sibling, 1 reply; 10+ messages in thread
From: Olivier Matz @ 2016-06-13 11:24 UTC (permalink / raw)
  To: christian.ehrhardt, david.marchand, dev

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] xenvirt: fix compilation after mempool changes
  2016-06-13 11:24     ` [PATCH v2] " Olivier Matz
@ 2016-06-13 11:54       ` Christian Ehrhardt
  2016-06-16 15:52         ` Bruce Richardson
  0 siblings, 1 reply; 10+ messages in thread
From: Christian Ehrhardt @ 2016-06-13 11:54 UTC (permalink / raw)
  To: Olivier Matz; +Cc: David Marchand, dev

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
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] xenvirt: fix compilation after mempool changes
  2016-06-13 11:54       ` Christian Ehrhardt
@ 2016-06-16 15:52         ` Bruce Richardson
  0 siblings, 0 replies; 10+ messages in thread
From: Bruce Richardson @ 2016-06-16 15:52 UTC (permalink / raw)
  To: Christian Ehrhardt; +Cc: Olivier Matz, David Marchand, dev

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-06-16 15:52 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-13  7:34 Question regarding mempool changes impact to XEN PMD Christian Ehrhardt
2016-06-13  8:14 ` Olivier Matz
2016-06-13  8:22   ` [PATCH] xenvirt: fix compilation after mempool changes Olivier Matz
2016-06-13  8:51     ` Christian Ehrhardt
2016-06-13  9:10       ` Christian Ehrhardt
2016-06-13  9:13         ` Olivier Matz
2016-06-13 11:24     ` [PATCH v2] " Olivier Matz
2016-06-13 11:54       ` Christian Ehrhardt
2016-06-16 15:52         ` Bruce Richardson
2016-06-13  8:30   ` Question regarding mempool changes impact to XEN PMD Christian Ehrhardt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.