Re: [dpdk-dev] [PATCH] log: deprecate history dump

[Thomas Monjalon <thomas.monjalon@6wind.com>]

2016-06-09 16:45, David Marchand:
> On Thu, Jun 9, 2016 at 4:09 PM, Thomas Monjalon
> <thomas.monjalon@6wind.com> wrote:
> > The log history uses rte_mempool. In order to remove the mempool
> > dependency in EAL (and improve the build), this feature is deprecated.
> > The ABI is kept but the behaviour is now voided because it seems this
> > function was not used. The history can be read from syslog.
> 
> It does look like it is not really used.
> I am for this change unless someone complains.
> 
> Comments below.

All your comments will be addressed in a v2. Thanks

> - Since you are looking at this, what keeps us from removing the
> dependency on librte_ring ?

Please see this first small cleanup:
	http://dpdk.org/ml/archives/dev/2016-June/040798.html

> I would say it was mainly because of mempool.
> Maybe ivshmem ?

Yes CONFIG_RTE_LIBRTE_IVSHMEM brings dependencies to rte_ring and rte_ivshmem.
This "feature" also pollute the memory allocator and makes rework harder.
That's why I would be in favor of removing CONFIG_RTE_LIBRTE_IVSHMEM.

Otherwise, as an alternative proposal, the file
	lib/librte_eal/linuxapp/eal/eal_ivshmem.c
could be moved outside of EAL. Probably that lib/librte_ivshmem/
would be a good place.
The tricky operation would be to remove ivshmem init from eal:

#ifdef RTE_LIBRTE_IVSHMEM
    if (rte_eal_ivshmem_init() < 0)                                                                              
        rte_panic("Cannot init IVSHMEM\n");
#endif

    if (rte_eal_memory_init() < 0)
        rte_panic("Cannot init memory\n");

    /* the directories are locked during eal_hugepage_info_init */
    eal_hugedirs_unlock();

    if (rte_eal_memzone_init() < 0)
        rte_panic("Cannot init memzone\n");

    if (rte_eal_tailqs_init() < 0)
        rte_panic("Cannot init tail queues for objects\n");

#ifdef RTE_LIBRTE_IVSHMEM
    if (rte_eal_ivshmem_obj_init() < 0)
        rte_panic("Cannot init IVSHMEM objects\n");
#endif