Patch Comment List
List patch comments
GET /api/patches/427/comments/?format=api
[ { "id": 860, "web_url": "http://patches.dpdk.org/comment/860/", "msgid": "<20140918151600.GA12120@BRICHA3-MOBL>", "list_archive_url": "https://inbox.dpdk.org/dev/20140918151600.GA12120@BRICHA3-MOBL", "date": "2014-09-18T15:16:00", "subject": "Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used\n\tRHEL 6)", "submitter": { "id": 20, "url": "http://patches.dpdk.org/api/people/20/?format=api", "name": "Bruce Richardson", "email": "bruce.richardson@intel.com" }, "content": "On Thu, Sep 18, 2014 at 11:03:12AM -0400, Neil Horman wrote:\n> On Thu, Sep 18, 2014 at 12:35:21PM +0000, Richardson, Bruce wrote:\n> > > -----Original Message-----\n> > > From: Neil Horman [mailto:nhorman@tuxdriver.com]\n> > > Sent: Thursday, September 18, 2014 1:25 PM\n> > > To: Thomas Monjalon\n> > > Cc: Richardson, Bruce; dev@dpdk.org\n> > > Subject: Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used RHEL\n> > > 6)\n> > > \n> > > On Thu, Sep 18, 2014 at 01:09:16PM +0200, Thomas Monjalon wrote:\n> > > > > The refcnt field is contained within an anonymous union within the mbuf\n> > > > > data structure, and gcc 4.4 gives an error about an unknown field unless\n> > > > > the initialiser for the field is contained within extra braces.\n> > > > >\n> > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>\n> > > >\n> > > > Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>\n> > > >\n> > > > Thanks Bruce, it is now applied.\n> > > \n> > > Hang on here, we use anonymous unions all the time in RHEL6, and make\n> > > assignments to them frequently, and the compiler doesn't complain (see the\n> > > dropcount variable in sk_buff for an example). Not saying that this is a big\n> > > deal, but can you explain a little more about what you're seeing when this error\n> > > occurs, before we just paper over it?\n> > > \n> > \n> > Originally reported on RHEL6 as a build failure. When I use gcc4.4 on Fedora 20, I get the following without this change:\n> > \n> > CC ixgbe_rxtx_vec.o\n> > == Build lib/librte_table\n> > /home/bruce/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c: In function 'ixgbe_rxq_vec_setup':\n> > /home/bruce/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c:726: error: unknown field 'refcnt' specified in initializer\n> > compilation terminated due to -Wfatal-errors.\n> > make[5]: *** [ixgbe_rxtx_vec.o] Error 1\n> > make[4]: *** [librte_pmd_ixgbe] Error 2\n> > make[4]: *** Waiting for unfinished jobs....\n> > make[3]: *** [lib] Error 2\n> > make[2]: *** [all] Error 2\n> > make[1]: *** [x86_64-native-linuxapp-gcc_install] Error 2\n> > make: *** [install] Error 2\n> > \n> > Regards,\n> > /Bruce\n> > \n> \n> \n> Thank you, I've reproduced the problem here, and you're right, it is a compiler\n> bug, but I really hate the idea of just throwing braces around something to shut\n> the compiler up, especially when the compiler has since been fixed. Looking at\n> it a bit more closely it seems like the the thing to do is actually just\n> consistently use rte_mbuf_refcnt_set, since thats what the rte_mbuf header\n> documentation says to do, and protects you if the internal structure changes, as\n> well as prevents you from having to spread ifdef RTE_MBUF_REFCNT all over the\n> place.\n> \n> This patch does a bit more than that too. With a little creative typedef-ing we\n> don't need the anonymous union at all, and lets us just use a single refcnt\n> variable, and I think eliminate that odd refcnt_reserved member of the union,\n> that, as far as I can see, does nothing.\n> \n> Thoughts?\n> \n\nI like the idea of using a typedef, though are we not adjusting the mbuf \nlayout if the refcount is disabled? A follow-on question would also be - do \nwe really need an option to disable the reference count, do we not always \njust leave it on?\n\nAs for using refcnt_set - that would be a solution that would work too, but \nthat approach needs to be checked for performance impacts as we are going \nfrom a constant initializer value to having an extra assignment instruction \nin the fast-path.\n\nThe simplest fix is probably just to add the braces, which isn't really much \nof a big deal.\n\n/Bruce", "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id 1C5C9B3AC;\n\tThu, 18 Sep 2014 17:11:25 +0200 (CEST)", "from mga01.intel.com (mga01.intel.com [192.55.52.88])\n\tby dpdk.org (Postfix) with ESMTP id 80FA4B3AA\n\tfor <dev@dpdk.org>; Thu, 18 Sep 2014 17:11:23 +0200 (CEST)", "from fmsmga001.fm.intel.com ([10.253.24.23])\n\tby fmsmga101.fm.intel.com with ESMTP; 18 Sep 2014 08:16:03 -0700", "from bricha3-mobl.ger.corp.intel.com (HELO\n\tbricha3-mobl.ir.intel.com) ([10.237.220.58])\n\tby fmsmga001.fm.intel.com with SMTP; 18 Sep 2014 08:16:01 -0700", "by bricha3-mobl.ir.intel.com (sSMTP sendmail emulation);\n\tThu, 18 Sep 2014 16:16:00 +0001" ], "X-ExtLoop1": "1", "X-IronPort-AV": "E=Sophos;i=\"5.04,548,1406617200\"; d=\"scan'208\";a=\"593304974\"", "Date": "Thu, 18 Sep 2014 16:16:00 +0100", "From": "Bruce Richardson <bruce.richardson@intel.com>", "To": "Neil Horman <nhorman@tuxdriver.com>", "Message-ID": "<20140918151600.GA12120@BRICHA3-MOBL>", "References": "<1411037752-8000-1-git-send-email-bruce.richardson@intel.com>\n\t<5076244.KSjCyF24zI@xps13>\n\t<20140918122527.GE20389@hmsreliant.think-freely.org>\n\t<59AF69C657FD0841A61C55336867B5B0343F3406@IRSMSX103.ger.corp.intel.com>\n\t<20140918150312.GF20389@hmsreliant.think-freely.org>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=us-ascii", "Content-Disposition": "inline", "In-Reply-To": "<20140918150312.GF20389@hmsreliant.think-freely.org>", "Organization": "Intel Shannon Ltd.", "User-Agent": "Mutt/1.5.22 (2013-10-16)", "Cc": "\"dev@dpdk.org\" <dev@dpdk.org>", "Subject": "Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used\n\tRHEL 6)", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "addressed": null }, { "id": 862, "web_url": "http://patches.dpdk.org/comment/862/", "msgid": "<2042613.z76ONrFlF0@xps13>", "list_archive_url": "https://inbox.dpdk.org/dev/2042613.z76ONrFlF0@xps13", "date": "2014-09-18T15:38:58", "subject": "Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used\n\tRHEL 6)", "submitter": { "id": 1, "url": "http://patches.dpdk.org/api/people/1/?format=api", "name": "Thomas Monjalon", "email": "thomas.monjalon@6wind.com" }, "content": "Hi Neil,\n\n2014-09-18 11:03, Neil Horman:\n> Thank you, I've reproduced the problem here, and you're right, it is a\n> compiler bug, but I really hate the idea of just throwing braces around\n> something to shut the compiler up, especially when the compiler has since\n> been fixed. Looking at it a bit more closely it seems like the the thing\n> to do is actually just consistently use rte_mbuf_refcnt_set, since thats\n> what the rte_mbuf header documentation says to do, and protects you if the\n> internal structure changes, as well as prevents you from having to spread\n> ifdef RTE_MBUF_REFCNT all over the place.\n> \n> This patch does a bit more than that too. With a little creative\n> typedef-ing we don't need the anonymous union at all, and lets us just use\n> a single refcnt variable, and I think eliminate that odd refcnt_reserved\n> member of the union, that, as far as I can see, does nothing.\n\n> --- a/config/common_linuxapp\n> +++ b/config/common_linuxapp\n> @@ -123,7 +123,7 @@ CONFIG_RTE_LOG_HISTORY=256\n> CONFIG_RTE_LIBEAL_USE_HPET=n\n> CONFIG_RTE_EAL_ALLOW_INV_SOCKET_ID=n\n> CONFIG_RTE_EAL_ALWAYS_PANIC_ON_ERROR=n\n> -CONFIG_RTE_EAL_IGB_UIO=y\n> +CONFIG_RTE_EAL_IGB_UIO=n\n> CONFIG_RTE_EAL_VFIO=y\n\nHum, indeed this patch does a bit more ;)\n\n[...]\n> -CONFIG_RTE_LIBRTE_KNI=y\n> +CONFIG_RTE_LIBRTE_KNI=n\n> CONFIG_RTE_KNI_KO_DEBUG=n\n\n> --- a/lib/librte_mbuf/rte_mbuf.h\n> +++ b/lib/librte_mbuf/rte_mbuf.h\n> @@ -120,6 +120,15 @@ extern \"C\" {\n> typedef void *MARKER[0]; /**< generic marker for a point in a\n> structure */ typedef uint64_t MARKER64[0]; /**< marker that allows us to\n> overwrite 8 bytes * with a single assignment */\n> +\n> +#ifdef RTE_MBUF_REFCNT\n> +#ifdef RTE_MBUF_REFCNT_ATOMIC\n> +typedef rte_atomic16_t rte_refcnt;\n> +#else\n> +typedef uint16_t rte_refcnt;\n> +#endif\n> +#endif\n> +\n> /**\n> * The generic rte_mbuf, containing a packet mbuf.\n> */\n> @@ -142,13 +151,9 @@ struct rte_mbuf {\n> \t * or non-atomic) is controlled by the CONFIG_RTE_MBUF_REFCNT_ATOMIC\n> \t * config option.\n> \t */\n> -\tunion {\n> #ifdef RTE_MBUF_REFCNT\n> -\t\trte_atomic16_t refcnt_atomic; /**< Atomically accessed refcnt */\n> -\t\tuint16_t refcnt; /**< Non-atomically accessed refcnt */\n> +\trte_refcnt refcnt;\n> #endif\n> -\t\tuint16_t refcnt_reserved; /**< Do not use this field */\n> -\t};\n\nYou are changing the mbuf alignment if MBUF_REFCNT is disabled.\n\n> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c\n> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c\n> @@ -722,11 +722,9 @@ ixgbe_rxq_vec_setup(struct igb_rx_queue *rxq)\n> \tstatic struct rte_mbuf mb_def = {\n> \t\t.nb_segs = 1,\n> \t\t.data_off = RTE_PKTMBUF_HEADROOM,\n> -#ifdef RTE_MBUF_REFCNT\n> -\t\t{ .refcnt = 1, }\n> -#endif\n> \t};\n> \n> +\trte_mbuf_refcnt_set(&mb_def, 1);\n\nPerformance impact must be checked here.", "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id A7455B3B1;\n\tThu, 18 Sep 2014 17:33:27 +0200 (CEST)", "from mail-wi0-f173.google.com (mail-wi0-f173.google.com\n\t[209.85.212.173]) by dpdk.org (Postfix) with ESMTP id 0D1E3B3B0\n\tfor <dev@dpdk.org>; Thu, 18 Sep 2014 17:33:26 +0200 (CEST)", "by mail-wi0-f173.google.com with SMTP id em10so3360437wid.12\n\tfor <dev@dpdk.org>; Thu, 18 Sep 2014 08:39:12 -0700 (PDT)", "from xps13.localnet (13.17.90.92.rev.sfr.net. [92.90.17.13])\n\tby mx.google.com with ESMTPSA id\n\tbk6sm26435776wjb.26.2014.09.18.08.39.08 for <multiple recipients>\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tThu, 18 Sep 2014 08:39:10 -0700 (PDT)" ], "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20130820;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:organization\n\t:user-agent:in-reply-to:references:mime-version\n\t:content-transfer-encoding:content-type;\n\tbh=BJf4n8v3vU4axVS1VSlqvR+ii/yd6g9BirSqoeS0bjE=;\n\tb=LB2enJpknWy5xxrDuC2UR6zVr/wzKTmIJRp/MAPYHASPsGokKHURwV5AK1sKoOjP/Y\n\tBxkGIC/auToP04VmxKafe06NRl+D3Ak8OL3A+ajVQ/FKChvK7wgXpLnpiKhKGKO6V3Ue\n\tjFQEEVgTXZlCOu7cf1gv3+zqDG5z3JzYQlEOvZhYrJnej5V4oN84ZvdQzSkPRf3xVHbG\n\tw1DNxuTD1/pBJiV/cku7D6YQ88thlx3aqXfn9q6ccXA3qtTZ2Ig8nWOdGagf/eJgDZJR\n\tfdFebhAezNWLB5R9v/wvy1silRCw2LNgUwuxBjo58QM96fNuaY6E4losA314aS25UVde\n\teMBQ==", "X-Gm-Message-State": "ALoCoQmucslwjPLstBs+yHiOdlLZtckXLSQShSSp6mXLarQqS0q8JQxXFS4kG0LPsQVU5ygOmJOf", "X-Received": "by 10.194.78.101 with SMTP id a5mr4386442wjx.118.1411054751139; \n\tThu, 18 Sep 2014 08:39:11 -0700 (PDT)", "From": "Thomas Monjalon <thomas.monjalon@6wind.com>", "To": "Neil Horman <nhorman@tuxdriver.com>", "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>\n\t<59AF69C657FD0841A61C55336867B5B0343F3406@IRSMSX103.ger.corp.intel.com>\n\t<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\n\tRHEL 6)", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "addressed": null }, { "id": 864, "web_url": "http://patches.dpdk.org/comment/864/", "msgid": "<20140918154622.GH20389@hmsreliant.think-freely.org>", "list_archive_url": "https://inbox.dpdk.org/dev/20140918154622.GH20389@hmsreliant.think-freely.org", "date": "2014-09-18T15:46:22", "subject": "Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used\n\tRHEL 6)", "submitter": { "id": 32, "url": "http://patches.dpdk.org/api/people/32/?format=api", "name": "Neil Horman", "email": "nhorman@tuxdriver.com" }, "content": "On Thu, Sep 18, 2014 at 04:16:00PM +0100, Bruce Richardson wrote:\n> On Thu, Sep 18, 2014 at 11:03:12AM -0400, Neil Horman wrote:\n> > On Thu, Sep 18, 2014 at 12:35:21PM +0000, Richardson, Bruce wrote:\n> > > > -----Original Message-----\n> > > > From: Neil Horman [mailto:nhorman@tuxdriver.com]\n> > > > Sent: Thursday, September 18, 2014 1:25 PM\n> > > > To: Thomas Monjalon\n> > > > Cc: Richardson, Bruce; dev@dpdk.org\n> > > > Subject: Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used RHEL\n> > > > 6)\n> > > > \n> > > > On Thu, Sep 18, 2014 at 01:09:16PM +0200, Thomas Monjalon wrote:\n> > > > > > The refcnt field is contained within an anonymous union within the mbuf\n> > > > > > data structure, and gcc 4.4 gives an error about an unknown field unless\n> > > > > > the initialiser for the field is contained within extra braces.\n> > > > > >\n> > > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>\n> > > > >\n> > > > > Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>\n> > > > >\n> > > > > Thanks Bruce, it is now applied.\n> > > > \n> > > > Hang on here, we use anonymous unions all the time in RHEL6, and make\n> > > > assignments to them frequently, and the compiler doesn't complain (see the\n> > > > dropcount variable in sk_buff for an example). Not saying that this is a big\n> > > > deal, but can you explain a little more about what you're seeing when this error\n> > > > occurs, before we just paper over it?\n> > > > \n> > > \n> > > Originally reported on RHEL6 as a build failure. When I use gcc4.4 on Fedora 20, I get the following without this change:\n> > > \n> > > CC ixgbe_rxtx_vec.o\n> > > == Build lib/librte_table\n> > > /home/bruce/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c: In function 'ixgbe_rxq_vec_setup':\n> > > /home/bruce/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c:726: error: unknown field 'refcnt' specified in initializer\n> > > compilation terminated due to -Wfatal-errors.\n> > > make[5]: *** [ixgbe_rxtx_vec.o] Error 1\n> > > make[4]: *** [librte_pmd_ixgbe] Error 2\n> > > make[4]: *** Waiting for unfinished jobs....\n> > > make[3]: *** [lib] Error 2\n> > > make[2]: *** [all] Error 2\n> > > make[1]: *** [x86_64-native-linuxapp-gcc_install] Error 2\n> > > make: *** [install] Error 2\n> > > \n> > > Regards,\n> > > /Bruce\n> > > \n> > \n> > \n> > Thank you, I've reproduced the problem here, and you're right, it is a compiler\n> > bug, but I really hate the idea of just throwing braces around something to shut\n> > the compiler up, especially when the compiler has since been fixed. Looking at\n> > it a bit more closely it seems like the the thing to do is actually just\n> > consistently use rte_mbuf_refcnt_set, since thats what the rte_mbuf header\n> > documentation says to do, and protects you if the internal structure changes, as\n> > well as prevents you from having to spread ifdef RTE_MBUF_REFCNT all over the\n> > place.\n> > \n> > This patch does a bit more than that too. With a little creative typedef-ing we\n> > don't need the anonymous union at all, and lets us just use a single refcnt\n> > variable, and I think eliminate that odd refcnt_reserved member of the union,\n> > that, as far as I can see, does nothing.\n> > \n> > Thoughts?\n> > \n> \n> I like the idea of using a typedef, though are we not adjusting the mbuf \n> layout if the refcount is disabled?\nWe are, theres still an RTE_MBUF_REFCNT ifdef around the definition of the\nrefcnt variable. The only change is that its consistently the same type\n(rte_refcnt), its just the typedef that changes based on the build configuration\n(which is in keeping with how the api implementation is toggled).\n\n A follow-on question would also be - do \n> we really need an option to disable the reference count, do we not always \n> just leave it on?\n> \nI would strongly agree with this, I find it hard to fathom a situation in which\nyou don't want or need to refcount buffers. I might be mistaken, but that seems\nlike folly. Though I left that issue alone in this patch.\n\n> As for using refcnt_set - that would be a solution that would work too, but \n> that approach needs to be checked for performance impacts as we are going \n> from a constant initializer value to having an extra assignment instruction \n> in the fast-path.\n> \nIts what you're documentation requires though (at least currently). Certainly\ncheck, but I would say the impact is zero, as the set routine is static inline,\nand the assignment value is a constant, which means the compiler should be able\nto optimize it to occur in parallel with the stack adjustment in the preamble.\n\n> The simplest fix is probably just to add the braces, which isn't really much \n> of a big deal.\n> \n\nIts duct tape. Its a simple fix that seems like a great idea today,\nspecifically becuase its simple, but it will cause maintenence headaches in the\nfuture, because you will constantly have to watch out in subsequent patch\nsubmissions for someone out of hand just fixing what by all rights is a silly\nextra set of braces that really shouldn't need to be there.\n\nAt the very least, consider this change because it means that you don't need to\nsprinkle code if ifdef RTE_MBUF_REFCNT. If you really feel like you need a\nstatic initalizer, lets make something akin to linux's ATOMIC_INIT, in which you\ncan package the brace magic, or define to nothing if you don't compile\nRTE_MBUF_REFCNT on.\n\nNeil", "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id BB997B3B0;\n\tThu, 18 Sep 2014 17:40:47 +0200 (CEST)", "from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58])\n\tby dpdk.org (Postfix) with ESMTP id 70115B3AB\n\tfor <dev@dpdk.org>; Thu, 18 Sep 2014 17:40:46 +0200 (CEST)", "from hmsreliant.think-freely.org\n\t([2001:470:8:a08:7aac:c0ff:fec2:933b] helo=localhost)\n\tby smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63)\n\t(envelope-from <nhorman@tuxdriver.com>)\n\tid 1XUduZ-0002eH-FG; Thu, 18 Sep 2014 11:46:29 -0400" ], "Date": "Thu, 18 Sep 2014 11:46:22 -0400", "From": "Neil Horman <nhorman@tuxdriver.com>", "To": "Bruce Richardson <bruce.richardson@intel.com>", "Message-ID": "<20140918154622.GH20389@hmsreliant.think-freely.org>", "References": "<1411037752-8000-1-git-send-email-bruce.richardson@intel.com>\n\t<5076244.KSjCyF24zI@xps13>\n\t<20140918122527.GE20389@hmsreliant.think-freely.org>\n\t<59AF69C657FD0841A61C55336867B5B0343F3406@IRSMSX103.ger.corp.intel.com>\n\t<20140918150312.GF20389@hmsreliant.think-freely.org>\n\t<20140918151600.GA12120@BRICHA3-MOBL>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=us-ascii", "Content-Disposition": "inline", "In-Reply-To": "<20140918151600.GA12120@BRICHA3-MOBL>", "User-Agent": "Mutt/1.5.23 (2014-03-12)", "X-Spam-Score": "-2.9 (--)", "X-Spam-Status": "No", "Cc": "\"dev@dpdk.org\" <dev@dpdk.org>", "Subject": "Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used\n\tRHEL 6)", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "addressed": null }, { "id": 865, "web_url": "http://patches.dpdk.org/comment/865/", "msgid": "<20140918155044.GI20389@hmsreliant.think-freely.org>", "list_archive_url": "https://inbox.dpdk.org/dev/20140918155044.GI20389@hmsreliant.think-freely.org", "date": "2014-09-18T15:50:44", "subject": "Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used\n\tRHEL 6)", "submitter": { "id": 32, "url": "http://patches.dpdk.org/api/people/32/?format=api", "name": "Neil Horman", "email": "nhorman@tuxdriver.com" }, "content": "On Thu, Sep 18, 2014 at 05:38:58PM +0200, Thomas Monjalon wrote:\n> Hi Neil,\n> \n> 2014-09-18 11:03, Neil Horman:\n> > Thank you, I've reproduced the problem here, and you're right, it is a\n> > compiler bug, but I really hate the idea of just throwing braces around\n> > something to shut the compiler up, especially when the compiler has since\n> > been fixed. Looking at it a bit more closely it seems like the the thing\n> > to do is actually just consistently use rte_mbuf_refcnt_set, since thats\n> > what the rte_mbuf header documentation says to do, and protects you if the\n> > internal structure changes, as well as prevents you from having to spread\n> > ifdef RTE_MBUF_REFCNT all over the place.\n> > \n> > This patch does a bit more than that too. With a little creative\n> > typedef-ing we don't need the anonymous union at all, and lets us just use\n> > a single refcnt variable, and I think eliminate that odd refcnt_reserved\n> > member of the union, that, as far as I can see, does nothing.\n> \n> > --- a/config/common_linuxapp\n> > +++ b/config/common_linuxapp\n> > @@ -123,7 +123,7 @@ CONFIG_RTE_LOG_HISTORY=256\n> > CONFIG_RTE_LIBEAL_USE_HPET=n\n> > CONFIG_RTE_EAL_ALLOW_INV_SOCKET_ID=n\n> > CONFIG_RTE_EAL_ALWAYS_PANIC_ON_ERROR=n\n> > -CONFIG_RTE_EAL_IGB_UIO=y\n> > +CONFIG_RTE_EAL_IGB_UIO=n\n> > CONFIG_RTE_EAL_VFIO=y\n> \n> Hum, indeed this patch does a bit more ;)\n> \nSorry, meant to clip that out, I was building without kernel headers, so I had\nto disable igb_uio and kni. I'll propose this patch properly if theres\nconsensus on the valid bits.\nNeil", "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id 758F4B3BB;\n\tThu, 18 Sep 2014 17:45:05 +0200 (CEST)", "from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58])\n\tby dpdk.org (Postfix) with ESMTP id 21ED0B3BA\n\tfor <dev@dpdk.org>; Thu, 18 Sep 2014 17:45:04 +0200 (CEST)", "from hmsreliant.think-freely.org\n\t([2001:470:8:a08:7aac:c0ff:fec2:933b] helo=localhost)\n\tby smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63)\n\t(envelope-from <nhorman@tuxdriver.com>)\n\tid 1XUdyn-0002gw-Bj; Thu, 18 Sep 2014 11:50:47 -0400" ], "Date": "Thu, 18 Sep 2014 11:50:44 -0400", "From": "Neil Horman <nhorman@tuxdriver.com>", "To": "Thomas Monjalon <thomas.monjalon@6wind.com>", "Message-ID": "<20140918155044.GI20389@hmsreliant.think-freely.org>", "References": "<1411037752-8000-1-git-send-email-bruce.richardson@intel.com>\n\t<59AF69C657FD0841A61C55336867B5B0343F3406@IRSMSX103.ger.corp.intel.com>\n\t<20140918150312.GF20389@hmsreliant.think-freely.org>\n\t<2042613.z76ONrFlF0@xps13>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=us-ascii", "Content-Disposition": "inline", "In-Reply-To": "<2042613.z76ONrFlF0@xps13>", "User-Agent": "Mutt/1.5.23 (2014-03-12)", "X-Spam-Score": "-2.9 (--)", "X-Spam-Status": "No", "Cc": "dev@dpdk.org", "Subject": "Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used\n\tRHEL 6)", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "addressed": null }, { "id": 867, "web_url": "http://patches.dpdk.org/comment/867/", "msgid": "<59AF69C657FD0841A61C55336867B5B0343F358D@IRSMSX103.ger.corp.intel.com>", "list_archive_url": "https://inbox.dpdk.org/dev/59AF69C657FD0841A61C55336867B5B0343F358D@IRSMSX103.ger.corp.intel.com", "date": "2014-09-18T16:01:32", "subject": "Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used\n\tRHEL 6)", "submitter": { "id": 20, "url": "http://patches.dpdk.org/api/people/20/?format=api", "name": "Bruce Richardson", "email": "bruce.richardson@intel.com" }, "content": "> -----Original Message-----\n> From: Neil Horman [mailto:nhorman@tuxdriver.com]\n> Sent: Thursday, September 18, 2014 4:51 PM\n> To: Thomas Monjalon\n> Cc: Richardson, Bruce; dev@dpdk.org\n> Subject: Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used RHEL\n> 6)\n> \n> On Thu, Sep 18, 2014 at 05:38:58PM +0200, Thomas Monjalon wrote:\n> > Hi Neil,\n> >\n> > 2014-09-18 11:03, Neil Horman:\n> > > Thank you, I've reproduced the problem here, and you're right, it is a\n> > > compiler bug, but I really hate the idea of just throwing braces around\n> > > something to shut the compiler up, especially when the compiler has since\n> > > been fixed. Looking at it a bit more closely it seems like the the thing\n> > > to do is actually just consistently use rte_mbuf_refcnt_set, since thats\n> > > what the rte_mbuf header documentation says to do, and protects you if the\n> > > internal structure changes, as well as prevents you from having to spread\n> > > ifdef RTE_MBUF_REFCNT all over the place.\n> > >\n> > > This patch does a bit more than that too. With a little creative\n> > > typedef-ing we don't need the anonymous union at all, and lets us just use\n> > > a single refcnt variable, and I think eliminate that odd refcnt_reserved\n> > > member of the union, that, as far as I can see, does nothing.\n> >\n> > > --- a/config/common_linuxapp\n> > > +++ b/config/common_linuxapp\n> > > @@ -123,7 +123,7 @@ CONFIG_RTE_LOG_HISTORY=256\n> > > CONFIG_RTE_LIBEAL_USE_HPET=n\n> > > CONFIG_RTE_EAL_ALLOW_INV_SOCKET_ID=n\n> > > CONFIG_RTE_EAL_ALWAYS_PANIC_ON_ERROR=n\n> > > -CONFIG_RTE_EAL_IGB_UIO=y\n> > > +CONFIG_RTE_EAL_IGB_UIO=n\n> > > CONFIG_RTE_EAL_VFIO=y\n> >\n> > Hum, indeed this patch does a bit more ;)\n> >\n> Sorry, meant to clip that out, I was building without kernel headers, so I had\n> to disable igb_uio and kni. I'll propose this patch properly if theres\n> consensus on the valid bits.\n> Neil\n\nIf we get rid of the REFCNT compile-time option (but not the REFCNT_ATOMIC one), then we should be able to get rid of the union, as we can do the atomic/non-atomic entirely inside the refcnt manipulation routines (since a regular uint16_t can be typecast directly to an atomic form of the same). Once we get rid of the union we can get rid of the extra braces that go along with the union, and we can just have the static initialiser cleanly put in the code.\nWhaddaya think?\n\n/Bruce", "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id 4F33FB3C0;\n\tThu, 18 Sep 2014 17:56:51 +0200 (CEST)", "from mga09.intel.com (mga09.intel.com [134.134.136.24])\n\tby dpdk.org (Postfix) with ESMTP id 444EBB3BE\n\tfor <dev@dpdk.org>; Thu, 18 Sep 2014 17:56:49 +0200 (CEST)", "from orsmga001.jf.intel.com ([10.7.209.18])\n\tby orsmga102.jf.intel.com with ESMTP; 18 Sep 2014 08:55:53 -0700", "from irsmsx103.ger.corp.intel.com ([163.33.3.157])\n\tby orsmga001.jf.intel.com with ESMTP; 18 Sep 2014 09:01:38 -0700", "from irsmsx105.ger.corp.intel.com (163.33.3.28) by\n\tIRSMSX103.ger.corp.intel.com (163.33.3.157) with Microsoft SMTP\n\tServer (TLS) id 14.3.195.1; Thu, 18 Sep 2014 17:01:33 +0100", "from irsmsx103.ger.corp.intel.com ([169.254.3.112]) by\n\tIRSMSX105.ger.corp.intel.com ([169.254.7.158]) with mapi id\n\t14.03.0195.001; Thu, 18 Sep 2014 17:01:32 +0100" ], "X-ExtLoop1": "1", "X-IronPort-AV": "E=Sophos;i=\"5.04,548,1406617200\"; d=\"scan'208\";a=\"575159352\"", "From": "\"Richardson, Bruce\" <bruce.richardson@intel.com>", "To": "Neil Horman <nhorman@tuxdriver.com>, Thomas Monjalon\n\t<thomas.monjalon@6wind.com>", "Thread-Topic": "[dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used\n\tRHEL 6)", "Thread-Index": "AQHP0y8i6ESf/18xKkWRib04BdvGMpwGqtYAgAAVSYCAABLtoIAAGSYAgAAJ/wCAAANJAIAAEwjg", "Date": "Thu, 18 Sep 2014 16:01:32 +0000", "Message-ID": "<59AF69C657FD0841A61C55336867B5B0343F358D@IRSMSX103.ger.corp.intel.com>", "References": "<1411037752-8000-1-git-send-email-bruce.richardson@intel.com>\n\t<59AF69C657FD0841A61C55336867B5B0343F3406@IRSMSX103.ger.corp.intel.com>\n\t<20140918150312.GF20389@hmsreliant.think-freely.org>\n\t<2042613.z76ONrFlF0@xps13>\n\t<20140918155044.GI20389@hmsreliant.think-freely.org>", "In-Reply-To": "<20140918155044.GI20389@hmsreliant.think-freely.org>", "Accept-Language": "en-GB, en-US", "Content-Language": "en-US", "X-MS-Has-Attach": "", "X-MS-TNEF-Correlator": "", "x-originating-ip": "[163.33.239.180]", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "quoted-printable", "MIME-Version": "1.0", "Cc": "\"dev@dpdk.org\" <dev@dpdk.org>", "Subject": "Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used\n\tRHEL 6)", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "addressed": null }, { "id": 873, "web_url": "http://patches.dpdk.org/comment/873/", "msgid": "<20140918181224.GO20389@hmsreliant.think-freely.org>", "list_archive_url": "https://inbox.dpdk.org/dev/20140918181224.GO20389@hmsreliant.think-freely.org", "date": "2014-09-18T18:12:24", "subject": "Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used\n\tRHEL 6)", "submitter": { "id": 32, "url": "http://patches.dpdk.org/api/people/32/?format=api", "name": "Neil Horman", "email": "nhorman@tuxdriver.com" }, "content": "On Thu, Sep 18, 2014 at 04:01:32PM +0000, Richardson, Bruce wrote:\n> > -----Original Message-----\n> > From: Neil Horman [mailto:nhorman@tuxdriver.com]\n> > Sent: Thursday, September 18, 2014 4:51 PM\n> > To: Thomas Monjalon\n> > Cc: Richardson, Bruce; dev@dpdk.org\n> > Subject: Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used RHEL\n> > 6)\n> > \n> > On Thu, Sep 18, 2014 at 05:38:58PM +0200, Thomas Monjalon wrote:\n> > > Hi Neil,\n> > >\n> > > 2014-09-18 11:03, Neil Horman:\n> > > > Thank you, I've reproduced the problem here, and you're right, it is a\n> > > > compiler bug, but I really hate the idea of just throwing braces around\n> > > > something to shut the compiler up, especially when the compiler has since\n> > > > been fixed. Looking at it a bit more closely it seems like the the thing\n> > > > to do is actually just consistently use rte_mbuf_refcnt_set, since thats\n> > > > what the rte_mbuf header documentation says to do, and protects you if the\n> > > > internal structure changes, as well as prevents you from having to spread\n> > > > ifdef RTE_MBUF_REFCNT all over the place.\n> > > >\n> > > > This patch does a bit more than that too. With a little creative\n> > > > typedef-ing we don't need the anonymous union at all, and lets us just use\n> > > > a single refcnt variable, and I think eliminate that odd refcnt_reserved\n> > > > member of the union, that, as far as I can see, does nothing.\n> > >\n> > > > --- a/config/common_linuxapp\n> > > > +++ b/config/common_linuxapp\n> > > > @@ -123,7 +123,7 @@ CONFIG_RTE_LOG_HISTORY=256\n> > > > CONFIG_RTE_LIBEAL_USE_HPET=n\n> > > > CONFIG_RTE_EAL_ALLOW_INV_SOCKET_ID=n\n> > > > CONFIG_RTE_EAL_ALWAYS_PANIC_ON_ERROR=n\n> > > > -CONFIG_RTE_EAL_IGB_UIO=y\n> > > > +CONFIG_RTE_EAL_IGB_UIO=n\n> > > > CONFIG_RTE_EAL_VFIO=y\n> > >\n> > > Hum, indeed this patch does a bit more ;)\n> > >\n> > Sorry, meant to clip that out, I was building without kernel headers, so I had\n> > to disable igb_uio and kni. I'll propose this patch properly if theres\n> > consensus on the valid bits.\n> > Neil\n> \n> If we get rid of the REFCNT compile-time option (but not the REFCNT_ATOMIC one), then we should be able to get rid of the union, as we can do the atomic/non-atomic entirely inside the refcnt manipulation routines (since a regular uint16_t can be typecast directly to an atomic form of the same). Once we get rid of the union we can get rid of the extra braces that go along with the union, and we can just have the static initialiser cleanly put in the code.\n> Whaddaya think?\n> \nYeah, that sounds good. Reduces the code complexity by a good amount, makes\nconfiguration simpler and allows for easy static initalization.\n> /Bruce\n>", "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id 682A7B3CE;\n\tThu, 18 Sep 2014 20:07:06 +0200 (CEST)", "from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58])\n\tby dpdk.org (Postfix) with ESMTP id 37888B3CD\n\tfor <dev@dpdk.org>; Thu, 18 Sep 2014 20:07:04 +0200 (CEST)", "from hmsreliant.think-freely.org\n\t([2001:470:8:a08:7aac:c0ff:fec2:933b] helo=localhost)\n\tby smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63)\n\t(envelope-from <nhorman@tuxdriver.com>)\n\tid 1XUgBt-0003W7-8t; Thu, 18 Sep 2014 14:12:31 -0400" ], "Date": "Thu, 18 Sep 2014 14:12:24 -0400", "From": "Neil Horman <nhorman@tuxdriver.com>", "To": "\"Richardson, Bruce\" <bruce.richardson@intel.com>", "Message-ID": "<20140918181224.GO20389@hmsreliant.think-freely.org>", "References": "<1411037752-8000-1-git-send-email-bruce.richardson@intel.com>\n\t<59AF69C657FD0841A61C55336867B5B0343F3406@IRSMSX103.ger.corp.intel.com>\n\t<20140918150312.GF20389@hmsreliant.think-freely.org>\n\t<2042613.z76ONrFlF0@xps13>\n\t<20140918155044.GI20389@hmsreliant.think-freely.org>\n\t<59AF69C657FD0841A61C55336867B5B0343F358D@IRSMSX103.ger.corp.intel.com>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=us-ascii", "Content-Disposition": "inline", "In-Reply-To": "<59AF69C657FD0841A61C55336867B5B0343F358D@IRSMSX103.ger.corp.intel.com>", "User-Agent": "Mutt/1.5.23 (2014-03-12)", "X-Spam-Score": "-2.9 (--)", "X-Spam-Status": "No", "Cc": "\"dev@dpdk.org\" <dev@dpdk.org>", "Subject": "Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used\n\tRHEL 6)", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "addressed": null } ]