From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f173.google.com (mail-wi0-f173.google.com [209.85.212.173]) by dpdk.org (Postfix) with ESMTP id 0D1E3B3B0 for ; Thu, 18 Sep 2014 17:33:26 +0200 (CEST) Received: by mail-wi0-f173.google.com with SMTP id em10so3360437wid.12 for ; Thu, 18 Sep 2014 08:39:12 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding:content-type; bh=BJf4n8v3vU4axVS1VSlqvR+ii/yd6g9BirSqoeS0bjE=; b=LB2enJpknWy5xxrDuC2UR6zVr/wzKTmIJRp/MAPYHASPsGokKHURwV5AK1sKoOjP/Y BxkGIC/auToP04VmxKafe06NRl+D3Ak8OL3A+ajVQ/FKChvK7wgXpLnpiKhKGKO6V3Ue jFQEEVgTXZlCOu7cf1gv3+zqDG5z3JzYQlEOvZhYrJnej5V4oN84ZvdQzSkPRf3xVHbG w1DNxuTD1/pBJiV/cku7D6YQ88thlx3aqXfn9q6ccXA3qtTZ2Ig8nWOdGagf/eJgDZJR fdFebhAezNWLB5R9v/wvy1silRCw2LNgUwuxBjo58QM96fNuaY6E4losA314aS25UVde eMBQ== X-Gm-Message-State: ALoCoQmucslwjPLstBs+yHiOdlLZtckXLSQShSSp6mXLarQqS0q8JQxXFS4kG0LPsQVU5ygOmJOf X-Received: by 10.194.78.101 with SMTP id a5mr4386442wjx.118.1411054751139; Thu, 18 Sep 2014 08:39:11 -0700 (PDT) Received: from xps13.localnet (13.17.90.92.rev.sfr.net. [92.90.17.13]) by mx.google.com with ESMTPSA id bk6sm26435776wjb.26.2014.09.18.08.39.08 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 18 Sep 2014 08:39:10 -0700 (PDT) From: Thomas Monjalon To: Neil Horman Date: Thu, 18 Sep 2014 17:38:58 +0200 Message-ID: <2042613.z76ONrFlF0@xps13> Organization: 6WIND User-Agent: KMail/4.13.3 (Linux/3.15.8-1-ARCH; KDE/4.13.3; x86_64; ; ) In-Reply-To: <20140918150312.GF20389@hmsreliant.think-freely.org> References: <1411037752-8000-1-git-send-email-bruce.richardson@intel.com> <59AF69C657FD0841A61C55336867B5B0343F3406@IRSMSX103.ger.corp.intel.com> <20140918150312.GF20389@hmsreliant.think-freely.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used RHEL 6) X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Sep 2014 15:33:26 -0000 Hi Neil, 2014-09-18 11:03, Neil Horman: > Thank you, I've reproduced the problem here, and you're right, it is a > compiler bug, but I really hate the idea of just throwing braces around > something to shut the compiler up, especially when the compiler has since > been fixed. Looking at it a bit more closely it seems like the the thing > to do is actually just consistently use rte_mbuf_refcnt_set, since thats > what the rte_mbuf header documentation says to do, and protects you if the > internal structure changes, as well as prevents you from having to spread > ifdef RTE_MBUF_REFCNT all over the place. > > This patch does a bit more than that too. With a little creative > typedef-ing we don't need the anonymous union at all, and lets us just use > a single refcnt variable, and I think eliminate that odd refcnt_reserved > member of the union, that, as far as I can see, does nothing. > --- a/config/common_linuxapp > +++ b/config/common_linuxapp > @@ -123,7 +123,7 @@ CONFIG_RTE_LOG_HISTORY=256 > CONFIG_RTE_LIBEAL_USE_HPET=n > CONFIG_RTE_EAL_ALLOW_INV_SOCKET_ID=n > CONFIG_RTE_EAL_ALWAYS_PANIC_ON_ERROR=n > -CONFIG_RTE_EAL_IGB_UIO=y > +CONFIG_RTE_EAL_IGB_UIO=n > CONFIG_RTE_EAL_VFIO=y Hum, indeed this patch does a bit more ;) [...] > -CONFIG_RTE_LIBRTE_KNI=y > +CONFIG_RTE_LIBRTE_KNI=n > CONFIG_RTE_KNI_KO_DEBUG=n > --- a/lib/librte_mbuf/rte_mbuf.h > +++ b/lib/librte_mbuf/rte_mbuf.h > @@ -120,6 +120,15 @@ extern "C" { > typedef void *MARKER[0]; /**< generic marker for a point in a > structure */ typedef uint64_t MARKER64[0]; /**< marker that allows us to > overwrite 8 bytes * with a single assignment */ > + > +#ifdef RTE_MBUF_REFCNT > +#ifdef RTE_MBUF_REFCNT_ATOMIC > +typedef rte_atomic16_t rte_refcnt; > +#else > +typedef uint16_t rte_refcnt; > +#endif > +#endif > + > /** > * The generic rte_mbuf, containing a packet mbuf. > */ > @@ -142,13 +151,9 @@ struct rte_mbuf { > * or non-atomic) is controlled by the CONFIG_RTE_MBUF_REFCNT_ATOMIC > * config option. > */ > - union { > #ifdef RTE_MBUF_REFCNT > - rte_atomic16_t refcnt_atomic; /**< Atomically accessed refcnt */ > - uint16_t refcnt; /**< Non-atomically accessed refcnt */ > + rte_refcnt refcnt; > #endif > - uint16_t refcnt_reserved; /**< Do not use this field */ > - }; You are changing the mbuf alignment if MBUF_REFCNT is disabled. > --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c > +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c > @@ -722,11 +722,9 @@ ixgbe_rxq_vec_setup(struct igb_rx_queue *rxq) > static struct rte_mbuf mb_def = { > .nb_segs = 1, > .data_off = RTE_PKTMBUF_HEADROOM, > -#ifdef RTE_MBUF_REFCNT > - { .refcnt = 1, } > -#endif > }; > > + rte_mbuf_refcnt_set(&mb_def, 1); Performance impact must be checked here. -- Thomas