From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 6ED561B5F6 for ; Wed, 19 Dec 2018 13:04:31 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C537C7F3E8; Wed, 19 Dec 2018 12:04:30 +0000 (UTC) Received: from [10.36.112.26] (ovpn-112-26.ams2.redhat.com [10.36.112.26]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 1B4F660167; Wed, 19 Dec 2018 12:04:25 +0000 (UTC) To: Jens Freimann , "Gavin Hu (Arm Technology China)" Cc: "dev@dpdk.org" , "tiwei.bie@intel.com" , "zhihong.wang@intel.com" , nd References: <20181211134804.10318-1-maxime.coquelin@redhat.com> <20181211134804.10318-2-maxime.coquelin@redhat.com> <20181219092504.5t6c5mxwwvkfpkmm@jenstp.localdomain> <20181219105324.i4wlu7hfkq7cipzi@jenstp.localdomain> From: Maxime Coquelin Message-ID: Date: Wed, 19 Dec 2018 13:04:23 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 MIME-Version: 1.0 In-Reply-To: <20181219105324.i4wlu7hfkq7cipzi@jenstp.localdomain> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Wed, 19 Dec 2018 12:04:30 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH v2 1/3] net/virtio: inline refill and offload helpers X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 19 Dec 2018 12:04:31 -0000 On 12/19/18 11:53 AM, Jens Freimann wrote: > On Wed, Dec 19, 2018 at 10:26:10AM +0000, Gavin Hu (Arm Technology > China) wrote: >> >> >>> -----Original Message----- >>> From: dev On Behalf Of Jens Freimann >>> Sent: Wednesday, December 19, 2018 5:25 PM >>> To: Maxime Coquelin >>> Cc: dev@dpdk.org; tiwei.bie@intel.com; zhihong.wang@intel.com >>> Subject: Re: [dpdk-dev] [PATCH v2 1/3] net/virtio: inline refill and >>> offload >>> helpers >>> >>> On Tue, Dec 11, 2018 at 02:48:02PM +0100, Maxime Coquelin wrote: >>> >Signed-off-by: Maxime Coquelin >>> >--- >>> > drivers/net/virtio/virtio_rxtx.c | 8 ++++---- >>> > 1 file changed, 4 insertions(+), 4 deletions(-) >>> > >>> >diff --git a/drivers/net/virtio/virtio_rxtx.c >>> >b/drivers/net/virtio/virtio_rxtx.c >>> >index eb891433e..e1c270b1c 100644 >>> >--- a/drivers/net/virtio/virtio_rxtx.c >>> >+++ b/drivers/net/virtio/virtio_rxtx.c >>> >@@ -741,7 +741,7 @@ virtio_dev_tx_queue_setup_finish(struct >>> rte_eth_dev *dev, >>> >     return 0; >>> > } >>> > >>> >-static void >>> >+static inline void >>> > virtio_discard_rxbuf(struct virtqueue *vq, struct rte_mbuf *m)  { >>> >     int error; >>> >@@ -757,7 +757,7 @@ virtio_discard_rxbuf(struct virtqueue *vq, struct >>> rte_mbuf *m) >>> >     } >>> > } >>> > >>> >-static void >>> >+static inline void >>> > virtio_discard_rxbuf_inorder(struct virtqueue *vq, struct rte_mbuf *m) >>> >{ >>> >     int error; >>> >@@ -769,7 +769,7 @@ virtio_discard_rxbuf_inorder(struct virtqueue *vq, >>> struct rte_mbuf *m) >>> >     } >>> > } >>> > >>> >-static void >>> >+static inline void >>> > virtio_update_packet_stats(struct virtnet_stats *stats, struct >>> >rte_mbuf *mbuf)  { >>> >     uint32_t s = mbuf->pkt_len; >>> >@@ -811,7 +811,7 @@ virtio_rx_stats_updated(struct virtnet_rx *rxvq, >>> >struct rte_mbuf *m)  } >>> > >>> > /* Optionally fill offload information in structure */ -static int >>> >+static inline int >>> > virtio_rx_offload(struct rte_mbuf *m, struct virtio_net_hdr *hdr) { >>> >     struct rte_net_hdr_lens hdr_lens; >>> >>> since these are all static functions, does declaring them inline >>> actually help >>> or are they inlined anyway by the compiler? >>> >>> regards, >>> Jens >> >> By disassembling the code, static function calls translate to "bl >> XXX", that means they are not inline. >> Inline is not always working, maybe __rte_always_inline is required here? > > I think always_inline is only to force inline when no optimiziation is > done? https://gcc.gnu.org/onlinedocs/gcc/Inline.html > > I don't know if there's a way to really force the compiler to inline > it or if it's better anyway to trust the compiler. It doesn't hurt to > have the functions declared inline except that it > clutters code a bit, but I don't have strong feelings against leaving this > patch as is.  Leaving it to Maxime, so I need to consider the use of __rte_always_inline generally in virtio_rxtx.c. In this specific case, inline seems enough. > > Reviewed-by: Jens Freimann Thanks, Maxime > regards, > Jens