Re: [dpdk-dev] [PATCH] mempool: accept user flags only

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

On 10/18/21 3:06 PM, David Marchand wrote:
> On Mon, Oct 18, 2021 at 11:05 AM Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru> wrote:
>>>>> +/**
>>>>> + * This macro lists all the mempool flags an application may request.
>>>>> + */
>>>>> +#define MEMPOOL_VALID_USER_FLAGS (MEMPOOL_F_NO_SPREAD \
>>>>
>>>> I think RTE_ prefix is missing here since it is in a public
>>>> header now.
>>>
>>> I discussed about this offline with David. I was ok to omit the
>>> RTE_ prefix for consistency.
>>>
>>>>> +   | MEMPOOL_F_NO_CACHE_ALIGN \
>>>>> +   | MEMPOOL_F_SP_PUT \
>>>>> +   | MEMPOOL_F_SC_GET \
>>>>> +   | MEMPOOL_F_NO_IOVA_CONTIG \
>>>>> +   )
>>>>>  /**
>>>>>   * @internal When debug is enabled, store some statistics.
>>>>>   *
>>>>>
>>>>
>>>> Should we make a patch to add defines with RTE_ prefix and
>>>> add a deprecation for old flags without RTE_ prefix?
>>>>
>>>> Olivier, what do you think? If you have time to care about it,
>>>> it would be great. (I'm unfamiliar with coccinelle yet).
>>>
>>> I was joking with David about keeping some work for 2022 :)
>>>
>>> I will try to have a look. As a side note, I was not that convinced by
>>> coccinelle for simple replacements like this, because it does not
>>> update the comments or documentation, it can (rarely) break the
>>> indentation, it is slow, and the syntax is quite complex.
>>>
>>
>> I dreamed to finish with renanamings in core libraries in 2021.
> 
> Sadly, you are not the only one.
> 
> 
>> May be it is to naive :(
>>
>> I see that rc1 is already delayed and adding a couple of
>> more patches could delay it even more. I'm ready to do it
>> without coccinelle today, so, it is a question to @Thomas and
>> @David if that late patch will be considered at all.
> 
> Thomas?
> 
> As usual, we must avoid unannounced API changes unless waived by Techboard.
> I am not against it if we keep compatilibity (marking RTE_DEPRECATED
> can wait next releases), and seeing how it only impacts a relatively
> low number of places in DPDK.
> 
> Risks are mainly on compilation side, and conflicts with other patches
> in mempool for rc1, but I can only think of Dmitry series and the
> conflict is easy to solve.
> 
> It should be quick to do: when could you send such a patch?

Done.

> 
> 
> What I have seen:
> 
> $ git grep -iE "(define|inline).*\<(|__)mempool" -- lib/mempool/*.h
> lib/mempool/rte_mempool.h:#define       MEMPOOL_PG_SHIFT_MAX
> (sizeof(uintptr_t) * CHAR_BIT - 1)
> lib/mempool/rte_mempool.h:#define       MEMPOOL_PG_NUM_DEFAULT  1
> ^^^
> bit-rot, both can be dropped.
> 
> lib/mempool/rte_mempool.h:#define MEMPOOL_F_NO_SPREAD      0x0001
> lib/mempool/rte_mempool.h:#define MEMPOOL_F_NO_CACHE_ALIGN 0x0002 /**<
> Do not align objs on cache lines.*/
> lib/mempool/rte_mempool.h:#define MEMPOOL_F_SP_PUT         0x0004 /**<
> Default put is "single-producer".*/
> lib/mempool/rte_mempool.h:#define MEMPOOL_F_SC_GET         0x0008 /**<
> Default get is "single-consumer".*/
> lib/mempool/rte_mempool.h:#define MEMPOOL_F_POOL_CREATED   0x0010 /**<
> Internal: pool is created. */
> lib/mempool/rte_mempool.h:#define MEMPOOL_F_NO_IOVA_CONTIG 0x0020 /**<
> Don't need IOVA contiguous objs. */
> lib/mempool/rte_mempool.h:#define MEMPOOL_VALID_USER_FLAGS
> (MEMPOOL_F_NO_SPREAD \
> 
> lib/mempool/rte_mempool.h:#define __MEMPOOL_STAT_ADD(mp, name, n) do {
>                    \
> lib/mempool/rte_mempool.h:#define __MEMPOOL_STAT_ADD(mp, name, n) do {} while(0)
> lib/mempool/rte_mempool.h:#define MEMPOOL_HEADER_SIZE(mp, cs) \
> lib/mempool/rte_mempool.h:static inline struct rte_mempool_objhdr
> *__mempool_get_header(void *obj)
> lib/mempool/rte_mempool.h:static inline struct rte_mempool_objtlr
> *__mempool_get_trailer(void *obj)
> lib/mempool/rte_mempool.h:#define __mempool_check_cookies(mp,
> obj_table_const, n, free) \
> lib/mempool/rte_mempool.h:#define __mempool_check_cookies(mp,
> obj_table_const, n, free) do {} while(0)
> lib/mempool/rte_mempool.h:#define
> __mempool_contig_blocks_check_cookies(mp, first_obj_table_const, n, \
> lib/mempool/rte_mempool.h:#define
> __mempool_contig_blocks_check_cookies(mp, first_obj_table_const, n, \
> ^^
> Internal stuff, but I would prefix in any case.
> 
> lib/mempool/rte_mempool.h:#define MEMPOOL_REGISTER_OPS(ops)
>                  \

Many thanks for the grep. I'd lost something without it.