diff mbox

[dpdk-dev,2/4] mempool: make cache flush threshold macro public

Message ID 1470084176-79932-3-git-send-email-rsanford@akamai.com (mailing list archive)
State Rejected, archived
Delegated to: Ferruh Yigit
Headers

Commit Message

Robert Sanford Aug. 1, 2016, 8:42 p.m. UTC 
  Rename macros that calculate a mempool cache flush threshold, and
move them from rte_mempool.c to rte_mempool.h, so that the bonding
driver can accurately calculate its mbuf requirements.

Signed-off-by: Robert Sanford <rsanford@akamai.com>
---
 lib/librte_mempool/rte_mempool.c |    8 ++------
 lib/librte_mempool/rte_mempool.h |    7 +++++++
 2 files changed, 9 insertions(+), 6 deletions(-)

Comments

Olivier Matz Aug. 23, 2016, 3:09 p.m. UTC | #1 
Hi Robert,

On 08/01/2016 10:42 PM, Robert Sanford wrote:
> Rename macros that calculate a mempool cache flush threshold, and
> move them from rte_mempool.c to rte_mempool.h, so that the bonding
> driver can accurately calculate its mbuf requirements.
>
> Signed-off-by: Robert Sanford <rsanford@akamai.com>
> ---
>   lib/librte_mempool/rte_mempool.c |    8 ++------
>   lib/librte_mempool/rte_mempool.h |    7 +++++++
>   2 files changed, 9 insertions(+), 6 deletions(-)
>
> [...]
>
> --- a/lib/librte_mempool/rte_mempool.h
> +++ b/lib/librte_mempool/rte_mempool.h
> @@ -263,6 +263,13 @@ struct rte_mempool {
>   #define MEMPOOL_F_NO_PHYS_CONTIG 0x0020 /**< Don't need physically contiguous objs. */
>
>   /**
> + * Calculate the threshold before we flush excess elements.
> + */
> +#define RTE_MEMPOOL_CACHE_FLUSHTHRESH_MULTIPLIER 1.5
> +#define RTE_MEMPOOL_CALC_CACHE_FLUSHTHRESH(c)	\
> +	((typeof(c))((c) * RTE_MEMPOOL_CACHE_FLUSHTHRESH_MULTIPLIER))
> +
> +/**
>    * @internal When debug is enabled, store some statistics.
>    *
>    * @param mp
>

What do you think of using a static inline function instead of a macro ?


Regards,
Olivier
Sanford, Robert Aug. 23, 2016, 4:07 p.m. UTC | #2 
Hi Olivier,

On 8/23/16, 11:09 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote:

    Hi Robert,
    
    On 08/01/2016 10:42 PM, Robert Sanford wrote:
    > Rename macros that calculate a mempool cache flush threshold, and

    > move them from rte_mempool.c to rte_mempool.h, so that the bonding

    > driver can accurately calculate its mbuf requirements.

    >

    > Signed-off-by: Robert Sanford <rsanford@akamai.com>

    > ---

    >   lib/librte_mempool/rte_mempool.c |    8 ++------

    >   lib/librte_mempool/rte_mempool.h |    7 +++++++

    >   2 files changed, 9 insertions(+), 6 deletions(-)

    >

    > [...]

    >

    > --- a/lib/librte_mempool/rte_mempool.h

    > +++ b/lib/librte_mempool/rte_mempool.h

    > @@ -263,6 +263,13 @@ struct rte_mempool {

    >   #define MEMPOOL_F_NO_PHYS_CONTIG 0x0020 /**< Don't need physically contiguous objs. */

    >

    >   /**

    > + * Calculate the threshold before we flush excess elements.

    > + */

    > +#define RTE_MEMPOOL_CACHE_FLUSHTHRESH_MULTIPLIER 1.5

    > +#define RTE_MEMPOOL_CALC_CACHE_FLUSHTHRESH(c)	\

    > +	((typeof(c))((c) * RTE_MEMPOOL_CACHE_FLUSHTHRESH_MULTIPLIER))

    > +

    > +/**

    >    * @internal When debug is enabled, store some statistics.

    >    *

    >    * @param mp

    >

    
    What do you think of using a static inline function instead of a macro ?
    
    
    Regards,
    Olivier
--

Yes, an inline function is better than a macro. We still need to move the #define MULTIPLIER from rte_mempool.c to rte_mempool.h.

How is this for the prototype?
static inline unsigned rte_mempool_calc_cache_flushthresh(unsigned cache_size)

Where in the .h should we place the function, right below the MULTIPLIER definition?

Regards,
Robert
Olivier Matz Aug. 24, 2016, 4:15 p.m. UTC | #3 
Hi Robert,

On 08/23/2016 06:07 PM, Sanford, Robert wrote:
> Hi Olivier,
>
> On 8/23/16, 11:09 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote:
>
>      Hi Robert,
>
>      On 08/01/2016 10:42 PM, Robert Sanford wrote:
>      > Rename macros that calculate a mempool cache flush threshold, and
>      > move them from rte_mempool.c to rte_mempool.h, so that the bonding
>      > driver can accurately calculate its mbuf requirements.
>      >
>      > Signed-off-by: Robert Sanford <rsanford@akamai.com>
>      > ---
>      >   lib/librte_mempool/rte_mempool.c |    8 ++------
>      >   lib/librte_mempool/rte_mempool.h |    7 +++++++
>      >   2 files changed, 9 insertions(+), 6 deletions(-)
>      >
>      > [...]
>      >
>      > --- a/lib/librte_mempool/rte_mempool.h
>      > +++ b/lib/librte_mempool/rte_mempool.h
>      > @@ -263,6 +263,13 @@ struct rte_mempool {
>      >   #define MEMPOOL_F_NO_PHYS_CONTIG 0x0020 /**< Don't need physically contiguous objs. */
>      >
>      >   /**
>      > + * Calculate the threshold before we flush excess elements.
>      > + */
>      > +#define RTE_MEMPOOL_CACHE_FLUSHTHRESH_MULTIPLIER 1.5
>      > +#define RTE_MEMPOOL_CALC_CACHE_FLUSHTHRESH(c)	\
>      > +	((typeof(c))((c) * RTE_MEMPOOL_CACHE_FLUSHTHRESH_MULTIPLIER))
>      > +
>      > +/**
>      >    * @internal When debug is enabled, store some statistics.
>      >    *
>      >    * @param mp
>      >
>
>      What do you think of using a static inline function instead of a macro ?
>
>
>      Regards,
>      Olivier
> --
>
> Yes, an inline function is better than a macro. We still need to move the #define MULTIPLIER from rte_mempool.c to rte_mempool.h.
>
> How is this for the prototype?
> static inline unsigned rte_mempool_calc_cache_flushthresh(unsigned cache_size)
>
> Where in the .h should we place the function, right below the MULTIPLIER definition?

Yep, looks good, thanks.

Maybe "unsigned" -> "unsigned int", because checkpatch may complain...


Regards,
Olivier
diff mbox

Patch

diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
index 2e28e2e..cca4843 100644
--- a/lib/librte_mempool/rte_mempool.c
+++ b/lib/librte_mempool/rte_mempool.c
@@ -69,10 +69,6 @@  static struct rte_tailq_elem rte_mempool_tailq = {
 };
 EAL_REGISTER_TAILQ(rte_mempool_tailq)
 
-#define CACHE_FLUSHTHRESH_MULTIPLIER 1.5
-#define CALC_CACHE_FLUSHTHRESH(c)	\
-	((typeof(c))((c) * CACHE_FLUSHTHRESH_MULTIPLIER))
-
 /*
  * return the greatest common divisor between a and b (fast algorithm)
  *
@@ -686,7 +682,7 @@  static void
 mempool_cache_init(struct rte_mempool_cache *cache, uint32_t size)
 {
 	cache->size = size;
-	cache->flushthresh = CALC_CACHE_FLUSHTHRESH(size);
+	cache->flushthresh = RTE_MEMPOOL_CALC_CACHE_FLUSHTHRESH(size);
 	cache->len = 0;
 }
 
@@ -762,7 +758,7 @@  rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
 
 	/* asked cache too big */
 	if (cache_size > RTE_MEMPOOL_CACHE_MAX_SIZE ||
-	    CALC_CACHE_FLUSHTHRESH(cache_size) > n) {
+	    RTE_MEMPOOL_CALC_CACHE_FLUSHTHRESH(cache_size) > n) {
 		rte_errno = EINVAL;
 		return NULL;
 	}
diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
index 059ad9e..4323c1b 100644
--- a/lib/librte_mempool/rte_mempool.h
+++ b/lib/librte_mempool/rte_mempool.h
@@ -263,6 +263,13 @@  struct rte_mempool {
 #define MEMPOOL_F_NO_PHYS_CONTIG 0x0020 /**< Don't need physically contiguous objs. */
 
 /**
+ * Calculate the threshold before we flush excess elements.
+ */
+#define RTE_MEMPOOL_CACHE_FLUSHTHRESH_MULTIPLIER 1.5
+#define RTE_MEMPOOL_CALC_CACHE_FLUSHTHRESH(c)	\
+	((typeof(c))((c) * RTE_MEMPOOL_CACHE_FLUSHTHRESH_MULTIPLIER))
+
+/**
  * @internal When debug is enabled, store some statistics.
  *
  * @param mp