mbuf: add explanation for confusing checks when freeing

[dpdk.git] / lib / librte_mbuf / rte_mbuf.h

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index ace6736..0265172 100644 (file)
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -189,6 +189,8 @@ extern "C" {
  */
 #define PKT_TX_OUTER_IPV6    (1ULL << 60)
 
+#define IND_ATTACHED_MBUF    (1ULL << 62) /**< Indirect attached mbuf */
+
 /* Use final bit of flags to indicate a control mbuf */
 #define CTRL_MBUF_FLAG       (1ULL << 63) /**< Mbuf contains control data */
 
@@ -245,11 +247,8 @@ struct rte_mbuf {
         * config option.
         */
        union {
-#ifdef RTE_MBUF_REFCNT
                rte_atomic16_t refcnt_atomic; /**< Atomically accessed refcnt */
                uint16_t refcnt;              /**< Non-atomically accessed refcnt */
-#endif
-               uint16_t refcnt_reserved;     /**< Do not use this field */
        };
        uint8_t nb_segs;          /**< Number of segments. */
        uint8_t port;             /**< Input port. */
@@ -286,11 +285,10 @@ struct rte_mbuf {
                             PKT_RX_FDIR_* flag in ol_flags. */
                } fdir;           /**< Filter identifier if FDIR enabled */
                uint32_t sched;   /**< Hierarchical scheduler */
-               uint32_t usr;     /**< User defined tags. See @rte_distributor_process */
+               uint32_t usr;     /**< User defined tags. See rte_distributor_process() */
        } hash;                   /**< hash information */
 
-       /* sequence number - field used in distributor and reorder library */
-       uint32_t seqn;
+       uint32_t seqn; /**< Sequence number. See also rte_reorder_insert() */
 
        /* second cache line - fields only used in slow path or on TX */
        MARKER cacheline1 __rte_cache_aligned;
@@ -335,13 +333,12 @@ struct rte_mbuf {
 /**
  * Returns TRUE if given mbuf is indirect, or FALSE otherwise.
  */
-#define RTE_MBUF_INDIRECT(mb)   (RTE_MBUF_FROM_BADDR((mb)->buf_addr) != (mb))
+#define RTE_MBUF_INDIRECT(mb)   ((mb)->ol_flags & IND_ATTACHED_MBUF)
 
 /**
  * Returns TRUE if given mbuf is direct, or FALSE otherwise.
  */
-#define RTE_MBUF_DIRECT(mb)     (RTE_MBUF_FROM_BADDR((mb)->buf_addr) == (mb))
-
+#define RTE_MBUF_DIRECT(mb)     (!RTE_MBUF_INDIRECT(mb))
 
 /**
  * Private data in case of pktmbuf pool.
@@ -383,7 +380,6 @@ if (!(exp)) {                                                        \
 
 #endif /*  RTE_LIBRTE_MBUF_DEBUG */
 
-#ifdef RTE_MBUF_REFCNT
 #ifdef RTE_MBUF_REFCNT_ATOMIC
 
 /**
@@ -465,15 +461,6 @@ rte_mbuf_refcnt_set(struct rte_mbuf *m, uint16_t new_value)
                rte_prefetch0(m);               \
 } while (0)
 
-#else /* ! RTE_MBUF_REFCNT */
-
-/** Mbuf prefetch */
-#define RTE_MBUF_PREFETCH_TO_FREE(m) do { } while(0)
-
-#define rte_mbuf_refcnt_set(m,v) do { } while(0)
-
-#endif /* RTE_MBUF_REFCNT */
-
 
 /**
  * Sanity checks on an mbuf.
@@ -508,10 +495,8 @@ static inline struct rte_mbuf *__rte_mbuf_raw_alloc(struct rte_mempool *mp)
        if (rte_mempool_get(mp, &mb) < 0)
                return NULL;
        m = (struct rte_mbuf *)mb;
-#ifdef RTE_MBUF_REFCNT
        RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(m) == 0);
        rte_mbuf_refcnt_set(m, 1);
-#endif /* RTE_MBUF_REFCNT */
        return (m);
 }
 
@@ -526,9 +511,7 @@ static inline struct rte_mbuf *__rte_mbuf_raw_alloc(struct rte_mempool *mp)
 static inline void __attribute__((always_inline))
 __rte_mbuf_raw_free(struct rte_mbuf *m)
 {
-#ifdef RTE_MBUF_REFCNT
        RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(m) == 0);
-#endif /* RTE_MBUF_REFCNT */
        rte_mempool_put(m->pool, m);
 }
 
@@ -703,8 +686,6 @@ static inline struct rte_mbuf *rte_pktmbuf_alloc(struct rte_mempool *mp)
        return (m);
 }
 
-#ifdef RTE_MBUF_REFCNT
-
 /**
  * Attach packet mbuf to another packet mbuf.
  * After attachment we refer the mbuf we attached as 'indirect',
@@ -743,7 +724,7 @@ static inline void rte_pktmbuf_attach(struct rte_mbuf *mi, struct rte_mbuf *md)
        mi->next = NULL;
        mi->pkt_len = mi->data_len;
        mi->nb_segs = 1;
-       mi->ol_flags = md->ol_flags;
+       mi->ol_flags = md->ol_flags | IND_ATTACHED_MBUF;
        mi->packet_type = md->packet_type;
 
        __rte_mbuf_sanity_check(mi, 1);
@@ -774,20 +755,27 @@ static inline void rte_pktmbuf_detach(struct rte_mbuf *m)
                        RTE_PKTMBUF_HEADROOM : m->buf_len;
 
        m->data_len = 0;
-}
-
-#endif /* RTE_MBUF_REFCNT */
 
+       m->ol_flags = 0;
+}
 
 static inline struct rte_mbuf* __attribute__((always_inline))
 __rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
 {
        __rte_mbuf_sanity_check(m, 0);
 
-#ifdef RTE_MBUF_REFCNT
+       /*
+        * Check to see if this is the last reference to the mbuf.
+        * Note: the double check here is deliberate. If the ref_cnt is "atomic"
+        * the call to "refcnt_update" is a very expensive operation, so we
+        * don't want to call it in the case where we know we are the holder
+        * of the last reference to this mbuf i.e. ref_cnt == 1.
+        * If however, ref_cnt != 1, it's still possible that we may still be
+        * the final decrementer of the count, so we need to check that
+        * result also, to make sure the mbuf is freed properly.
+        */
        if (likely (rte_mbuf_refcnt_read(m) == 1) ||
                        likely (rte_mbuf_refcnt_update(m, -1) == 0)) {
-               struct rte_mbuf *md = RTE_MBUF_FROM_BADDR(m->buf_addr);
 
                rte_mbuf_refcnt_set(m, 0);
 
@@ -795,17 +783,15 @@ __rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
                 *  - detach mbuf
                 *  - free attached mbuf segment
                 */
-               if (unlikely (md != m)) {
+               if (RTE_MBUF_INDIRECT(m)) {
+                       struct rte_mbuf *md = RTE_MBUF_FROM_BADDR(m->buf_addr);
                        rte_pktmbuf_detach(m);
                        if (rte_mbuf_refcnt_update(md, -1) == 0)
                                __rte_mbuf_raw_free(md);
                }
-#endif
                return(m);
-#ifdef RTE_MBUF_REFCNT
        }
        return (NULL);
-#endif
 }
 
 /**
@@ -848,8 +834,6 @@ static inline void rte_pktmbuf_free(struct rte_mbuf *m)
        }
 }
 
-#ifdef RTE_MBUF_REFCNT
-
 /**
  * Creates a "clone" of the given packet mbuf.
  *
@@ -924,8 +908,6 @@ static inline void rte_pktmbuf_refcnt_update(struct rte_mbuf *m, int16_t v)
        } while ((m = m->next) != NULL);
 }
 
-#endif /* RTE_MBUF_REFCNT */
-
 /**
  * Get the headroom in a packet mbuf.
  *