Re: [dpdk-dev] [PATCH v3 2/4] mempool: add non-IO flag

[Dmitry Kozlyuk <dkozlyuk@oss.nvidia.com>]

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: 12 октября 2021 г. 15:53
> To: Dmitry Kozlyuk <dkozlyuk@nvidia.com>; dev@dpdk.org
> Cc: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Matan Azrad
> <matan@nvidia.com>; Olivier Matz <olivier.matz@6wind.com>
> Subject: Re: [PATCH v3 2/4] mempool: add non-IO flag
> 
> External email: Use caution opening links or attachments
> 
> 
> On 10/12/21 3:40 PM, Dmitry Kozlyuk wrote:
> >> [...]
> >>> +#define MEMPOOL_F_NON_IO         0x0040 /**< Not used for device IO
> >> (DMA). */
> >>
> >> Doesn't it imply MEMPOOL_F_NO_IOVA_CONTIG?
> >
> > Let's leave this explicit. NO_IOVA_CONFIG could result in
> MEMZONE_IOVA_CONTIG (although it doesn't now), which can affect how
> many pages are used, which may affect performance due to TLB caches.
> 
> It sounds like a usage of a side effect of MEMPOOL_F_NO_IOVA_CONTIG
> absence. It does not sound good.

I agree, but my point is that behavior should not change when specifying a hint flag.
NO_IOVA_CONTIG  => NON_IO is feasible, NON_IO => NO_IOVA_CONTIG is against
the declared NON_IO properties.

> 
> >
> >> Shouldn't it reject mempool population with not RTE_BAD_IOVA iova
> >> parameter?
> >>
> >> I see that it is just a hint, but just trying to make full picture consistent.
> >>
> >> As the second thought: isn't iova==RTE_BAD_IOVA sufficient as a hint?
> >
> > 1. It looks true that if RTE_BAD_IOVA is used, we can infer it's a non-IO
> mempool.
> > 2. The new flag is needed or at least handly, because otherwise to check this
> property of a mempool, but how? Allocating a test mbuf is doable but looks like
> a hack. Or we can pass this information to the callback, complicating its
> signature. Do you think it's better?
> 
> mempool knows it when the mempool is populated.
> So, it can just set the flag itself.

Of course, I'm only arguing that to analyze mempool properties this flag
is needed, even if the user isn't supposed to set it themselves.
Looks like we agree on this one.

> > 3. Theoretically, user may want to use mempools for objects that are used for
> IO, but not with DPDK. In this case IOVA will be valid, but the flag can also be
> set.
> 
> It sounds very artificial.
> Also in this case I guess MEMPOOL_F_NON_IO should be clear anyway.

I see NON_IO as a hint to DPDK components, not sure about non-DPDK ones.
But since there isn't a use case indeed, the flag doesn't need to eb exposed now.

To summarize:
1. MEMPOOL_F_NON_IO is considered internal, like MEMPOOL_F_CREATED.
2. It is set automatically:
a) for MEMPOOL_F_NO_IOVA_CONTIG mempools;
b) if RTE_BAD_IOVA is used to populate.
I doubt HINT should be added to the name, because it's not a hint, it's a conclusion.