[dpdk-dev] [PATCH 1/2] drivers/mempool: add stack mempool handler as driver

Shreyansh Jain shreyansh.jain at nxp.com
Mon Mar 27 06:54:51 CEST 2017


Hello Olivier,

On Friday 24 March 2017 09:52 PM, Olivier Matz wrote:
> Hi Shreyansh,
>
> On Mon, 20 Mar 2017 15:33:09 +0530, Shreyansh Jain <shreyansh.jain at nxp.com> wrote:
>> CONFIG_RTE_DRIVER_MEMPOOL_STACK option added to common_base.
>> Stack mempool handler moved from lib/librte_mempool into drivers/mempool.
>>
>> With this patch, the Stack mempool handler registration is optional and
>> toggled via the configuration file. In case disabled (N), it would imply
>> request for creating of mempool would fail.
>>
>> Signed-off-by: Shreyansh Jain <shreyansh.jain at nxp.com>
>> ---
>>  config/common_base                                 |   5 +
>>  drivers/Makefile                                   |   1 +
>>  drivers/mempool/Makefile                           |  36 +++++
>>  drivers/mempool/stack/Makefile                     |  55 ++++++++
>>  drivers/mempool/stack/rte_mempool_stack.c          | 147 +++++++++++++++++++++
>>  .../mempool/stack/rte_mempool_stack_version.map    |   4 +
>>  lib/librte_mempool/Makefile                        |   1 -
>>  lib/librte_mempool/rte_mempool_stack.c             | 147 ---------------------
>>  8 files changed, 248 insertions(+), 148 deletions(-)
>>  create mode 100644 drivers/mempool/Makefile
>>  create mode 100644 drivers/mempool/stack/Makefile
>>  create mode 100644 drivers/mempool/stack/rte_mempool_stack.c
>>  create mode 100644 drivers/mempool/stack/rte_mempool_stack_version.map
>>  delete mode 100644 lib/librte_mempool/rte_mempool_stack.c
>
>
>
> I tried to pass the mempool autotest, and it issues a segfault.
> I think the libraries are missing in rte.app.mk, so no handler is
> registered.
>
> Adding the following code in lib/librte_mempool/rte_mempool_ops.c
> fixes the crash.
>
>         ops = rte_mempool_get_ops(mp->ops_index);
> +       if (ops == NULL || ops->alloc == NULL)
> +               return -ENOTSUP;
>         return ops->alloc(mp);
>
> Now that drivers are not linked to the mempool library, it can
> happen that there is no handler. Could you please add this patch in your
> patchset?

Indeed. I will add this code set as first patch. Thanks for suggestion/fix.

>
> I also checked that compilation works in shared lib mode. It looks good,
> but I think there will be a behavior change if CONFIG_RTE_EAL_PMD_PATH
> is empty (default). This option is probably used by distros to indicate
> where dpdk plugins are installed, and when it is set, all drivers of
> this directory are loaded, so in that case it won't change.
>
> But when unset, the drivers have to be loaded manually, and with this
> change, the mempool driver will have to be loaded with the -d eal option.
> Could you please check what occurs in that case? At least see if it
> displays a nice error or if it crashes. I suspect it will crash

Ok. I will try this and if there is any issue, fix it.

>
> Also, the MAINTAINERS file should be updated.

Yes, that is something I thought of updating but left it out before 
sending the patch.
One confirmation: I assume that maintainers need to be added with
"drivers/mempool/ring" and "drivers/mempool/stack" folders.
Who should be marked as maintainer - You (because of existing
lib/librte_mempool ownership) or I (because I have moved the code from
lib/librte_mempool)?

I think it would continue to be you but wanted to take your
confirmation before adding the lines.

>
>
> Thanks,
> Olivier
>
>

-
Shreyansh


More information about the dev mailing list