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

[Christian Ehrhardt <christian.ehrhardt@canonical.com>]

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