From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Hemminger Subject: Re: rte_mbuf library likely()/unlikely() Date: Mon, 23 Jul 2018 12:45:56 -0700 Message-ID: <20180723124556.73564e05@xeon-e3> References: <98CBD80474FA8B44BF855DF32C47DC35B421EE@smartserver.smartshare.dk> <20180723103757.47e4c26b@xeon-e3> <98CBD80474FA8B44BF855DF32C47DC35B421F0@smartserver.smartshare.dk> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: "Olivier Matz" , To: Morten =?UTF-8?B?QnLDuHJ1cA==?= Return-path: Received: from mail-pg1-f193.google.com (mail-pg1-f193.google.com [209.85.215.193]) by dpdk.org (Postfix) with ESMTP id 2E83D160 for ; Mon, 23 Jul 2018 21:46:00 +0200 (CEST) Received: by mail-pg1-f193.google.com with SMTP id e6-v6so1106510pgv.2 for ; Mon, 23 Jul 2018 12:46:00 -0700 (PDT) In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35B421F0@smartserver.smartshare.dk> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Mon, 23 Jul 2018 20:59:29 +0200 Morten Br=C3=B8rup wrote: > > -----Original Message----- > > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > > Sent: Monday, July 23, 2018 7:38 PM > > To: Morten Br=C3=B8rup > > Cc: Olivier Matz; dev@dpdk.org > > Subject: Re: [dpdk-dev] rte_mbuf library likely()/unlikely() > >=20 > > On Mon, 23 Jul 2018 15:53:42 +0200 > > Morten Br=C3=B8rup wrote: > > =20 > > > Hi Olivier, > > > > > > > > > > > > I noticed that __rte_pktmbuf_read() could do with an unlikely(), so I= =20 > > went through the entire library. Here are my suggested modifications. = =20 > > > > > > > > > > > > > > > > > > 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 =3D=3D NULL) > > > > > > + if (unlikely(m =3D=3D NULL)) > > > > > > rte_panic("mbuf is NULL\n"); > > > > > > =20 > >=20 > > Adding is unlikely is not necessary since rte_panic is marked with cold > > attribute > > which has the same effect. =20 >=20 > 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 tho= ught. Here's an updated patch for rte_mbuf.c, where it is relevant. The oth= er two suggested patches are unaffected. >=20 > 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 =3D m; > uint32_t buf_off =3D 0, copy_len; >=20 > - if (off + len > rte_pktmbuf_pkt_len(m)) > + if (unlikely(off + len > rte_pktmbuf_pkt_len(m))) > return NULL; >=20 > while (off >=3D rte_pktmbuf_data_len(seg)) { > @@ -257,7 +257,7 @@ > seg =3D seg->next; > } >=20 > - if (off + len <=3D rte_pktmbuf_data_len(seg)) > + if (likely(off + len <=3D rte_pktmbuf_data_len(seg))) > return rte_pktmbuf_mtod_offset(seg, char *, off); >=20 > /* rare case: header is split among several segments */ > @@ -344,7 +344,7 @@ > unsigned int i; > int ret; >=20 > - if (buflen =3D=3D 0) > + if (unlikely(buflen =3D=3D 0)) > return -1; >=20 > buf[0] =3D '\0'; > @@ -355,9 +355,9 @@ > if (name =3D=3D NULL) > name =3D rx_flags[i].default_name; > ret =3D snprintf(buf, buflen, "%s ", name); > - if (ret < 0) > + if (unlikely(ret < 0)) > return -1; > - if ((size_t)ret >=3D buflen) > + if (unlikely((size_t)ret >=3D buflen)) > return -1; > buf +=3D ret; > buflen -=3D ret; > @@ -440,7 +440,7 @@ > unsigned int i; > int ret; >=20 > - if (buflen =3D=3D 0) > + if (unlikely(buflen =3D=3D 0)) > return -1; >=20 > buf[0] =3D '\0'; > @@ -451,9 +451,9 @@ > if (name =3D=3D NULL) > name =3D tx_flags[i].default_name; > ret =3D snprintf(buf, buflen, "%s ", name); > - if (ret < 0) > + if (unlikely(ret < 0)) > return -1; > - if ((size_t)ret >=3D buflen) > + if (unlikely((size_t)ret >=3D buflen)) > return -1; > buf +=3D ret; > buflen -=3D ret; >=20 >=20 > Med venlig hilsen / kind regards > - Morten Br=C3=B8rup Yes, this makes sense. Please format patch with signed-off-by and submit according to the contributing guidelines Creating Patches section. https://doc.dpdk.org/guides/contributing/patches.html