> -----Original Message-----
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Monday, July 23, 2018 7:38 PM
> To: Morten Brørup
> Cc: Olivier Matz; dev@dpdk.org
> Subject: Re: [dpdk-dev] rte_mbuf library likely()/unlikely()
>
> On Mon, 23 Jul 2018 15:53:42 +0200
> Morten Brørup <m...@smartsharesystems.com> wrote:
>
> > Hi Olivier,
> >
> >
> >
> > I noticed that __rte_pktmbuf_read() could do with an unlikely(), so I
> went through the entire library. Here are my suggested modifications.
> >
> >
> >
> >
> >
> > diff -bu rte_mbuf.c.orig rte_mbuf.c
> >
> > --- rte_mbuf.c.orig 2018-07-23 15:13:22.000000000 +0200
> >
> > +++ rte_mbuf.c 2018-07-23 15:32:53.000000000 +0200
> >
> > @@ -173,19 +173,19 @@
> >
> > {
> >
> > unsigned int nb_segs, pkt_len;
> >
> >
> >
> > - if (m == NULL)
> >
> > + if (unlikely(m == NULL))
> >
> > rte_panic("mbuf is NULL\n");
> >
> >
>
> Adding is unlikely is not necessary since rte_panic is marked with cold
> attribute
> which has the same effect.
I was not aware of this. Although it is not visible from the source code files
using rte_panic(), it probably means we shouldn't as so much as I thought.
Here's an updated patch for rte_mbuf.c, where it is relevant. The other two
suggested patches are unaffected.
diff -bu rte_mbuf.c.orig rte_mbuf.c
--- rte_mbuf.c.orig 2018-07-23 15:13:22.000000000 +0200
+++ rte_mbuf.c 2018-07-23 20:52:35.000000000 +0200
@@ -249,7 +249,7 @@
const struct rte_mbuf *seg = m;
uint32_t buf_off = 0, copy_len;
- if (off + len > rte_pktmbuf_pkt_len(m))
+ if (unlikely(off + len > rte_pktmbuf_pkt_len(m)))
return NULL;
while (off >= rte_pktmbuf_data_len(seg)) {
@@ -257,7 +257,7 @@
seg = seg->next;
}
- if (off + len <= rte_pktmbuf_data_len(seg))
+ if (likely(off + len <= rte_pktmbuf_data_len(seg)))
return rte_pktmbuf_mtod_offset(seg, char *, off);
/* rare case: header is split among several segments */
@@ -344,7 +344,7 @@
unsigned int i;
int ret;
- if (buflen == 0)
+ if (unlikely(buflen == 0))
return -1;
buf[0] = '\0';
@@ -355,9 +355,9 @@
if (name == NULL)
name = rx_flags[i].default_name;
ret = snprintf(buf, buflen, "%s ", name);
- if (ret < 0)
+ if (unlikely(ret < 0))
return -1;
- if ((size_t)ret >= buflen)
+ if (unlikely((size_t)ret >= buflen))
return -1;
buf += ret;
buflen -= ret;
@@ -440,7 +440,7 @@
unsigned int i;
int ret;
- if (buflen == 0)
+ if (unlikely(buflen == 0))
return -1;
buf[0] = '\0';
@@ -451,9 +451,9 @@
if (name == NULL)
name = tx_flags[i].default_name;
ret = snprintf(buf, buflen, "%s ", name);
- if (ret < 0)
+ if (unlikely(ret < 0))
return -1;
- if ((size_t)ret >= buflen)
+ if (unlikely((size_t)ret >= buflen))
return -1;
buf += ret;
buflen -= ret;
Med venlig hilsen / kind regards
- Morten Brørup
