[{"id":1776027,"web_url":"http://patchwork.ozlabs.org/comment/1776027/","msgid":"<20170926.213354.305351416790211828.davem@davemloft.net>","list_archive_url":null,"date":"2017-09-27T04:33:54","subject":"Re: [PATCH v2 02/16] thunderbolt: Add support for XDomain properties","submitter":{"id":15,"url":"http://patchwork.ozlabs.org/api/people/15/","name":"David Miller","email":"davem@davemloft.net"},"content":"From: Mika Westerberg \nDate: Mon, 25 Sep 2017 14:07:24 +0300\n\n> +struct tb_property_entry {\n> +\tu32 key_hi;\n> +\tu32 key_lo;\n> +\tu16 length;\n> +\tu8 reserved;\n> +\tu8 type;\n> +\tu32 value;\n> +} __packed;\n> +\n> +struct tb_property_rootdir_entry {\n> +\tu32 magic;\n> +\tu32 length;\n> +\tstruct tb_property_entry entries[];\n> +} __packed;\n> +\n> +struct tb_property_dir_entry {\n> +\tu32 uuid[4];\n> +\tstruct tb_property_entry entries[];\n> +} __packed;\n\nThere is no apparent need for __packed here, and __packed should be\navoided unless absolutely necessary as it pessimizes the code\nsignificantly on some architectures.\n\nPlease remove __packed from these datastructures unless you can\nprove it is absolutely needed and, in such case, please document\nin a comment why that requirement exists. Because from the layout\nof these types, everything will be packed in just fine without\n__packed.\n\nThank you.","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y24ht4wcYz9t30\n\tfor ;\n\tWed, 27 Sep 2017 14:34:06 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S970059AbdI0Ed5 (ORCPT );\n\tWed, 27 Sep 2017 00:33:57 -0400","from shards.monkeyblade.net ([184.105.139.130]:48436 \"EHLO\n\tshards.monkeyblade.net\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S965486AbdI0Ed4 (ORCPT\n\t); Wed, 27 Sep 2017 00:33:56 -0400","from localhost (74-93-104-102-Washington.hfc.comcastbusiness.net\n\t[74.93.104.102])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(Client did not present a certificate)\n\t(Authenticated sender: davem-davemloft)\n\tby shards.monkeyblade.net (Postfix) with ESMTPSA id 24C5D13401125;\n\tTue, 26 Sep 2017 21:33:55 -0700 (PDT)"],"Date":"Tue, 26 Sep 2017 21:33:54 -0700 (PDT)","Message-Id":"<20170926.213354.305351416790211828.davem@davemloft.net>","To":"mika.westerberg@linux.intel.com","Cc":"gregkh@linuxfoundation.org, andreas.noever@gmail.com,\n\tmichael.jamet@intel.com, yehezkel.bernat@intel.com,\n\tamir.jer.levy@intel.com, Mario.Limonciello@dell.com,\n\tlukas@wunner.de, andriy.shevchenko@linux.intel.com, andrew@lunn.ch,\n\tlinux-kernel@vger.kernel.org, netdev@vger.kernel.org","Subject":"Re: [PATCH v2 02/16] thunderbolt: Add support for XDomain properties","From":"David Miller ","In-Reply-To":"<20170925110738.68382-3-mika.westerberg@linux.intel.com>","References":"<20170925110738.68382-1-mika.westerberg@linux.intel.com>\n\t<20170925110738.68382-3-mika.westerberg@linux.intel.com>","X-Mailer":"Mew version 6.7 on Emacs 25.3 / Mule 6.0 (HANACHIRUSATO)","Mime-Version":"1.0","Content-Type":"Text/Plain; charset=us-ascii","Content-Transfer-Encoding":"7bit","X-Greylist":"Sender succeeded SMTP AUTH, not delayed by\n\tmilter-greylist-4.5.12 (shards.monkeyblade.net\n\t[149.20.54.216]); Tue, 26 Sep 2017 21:33:55 -0700 (PDT)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1776260,"web_url":"http://patchwork.ozlabs.org/comment/1776260/","msgid":"<20170927113241.GB4630@lahna.fi.intel.com>","list_archive_url":null,"date":"2017-09-27T11:32:41","subject":"Re: [PATCH v2 02/16] thunderbolt: Add support for XDomain properties","submitter":{"id":14534,"url":"http://patchwork.ozlabs.org/api/people/14534/","name":"Mika Westerberg","email":"mika.westerberg@linux.intel.com"},"content":"On Tue, Sep 26, 2017 at 09:33:54PM -0700, David Miller wrote:\n> From: Mika Westerberg \n> Date: Mon, 25 Sep 2017 14:07:24 +0300\n> \n> > +struct tb_property_entry {\n> > +\tu32 key_hi;\n> > +\tu32 key_lo;\n> > +\tu16 length;\n> > +\tu8 reserved;\n> > +\tu8 type;\n> > +\tu32 value;\n> > +} __packed;\n> > +\n> > +struct tb_property_rootdir_entry {\n> > +\tu32 magic;\n> > +\tu32 length;\n> > +\tstruct tb_property_entry entries[];\n> > +} __packed;\n> > +\n> > +struct tb_property_dir_entry {\n> > +\tu32 uuid[4];\n> > +\tstruct tb_property_entry entries[];\n> > +} __packed;\n> \n> There is no apparent need for __packed here, and __packed should be\n> avoided unless absolutely necessary as it pessimizes the code\n> significantly on some architectures.\n> \n> Please remove __packed from these datastructures unless you can\n> prove it is absolutely needed and, in such case, please document\n> in a comment why that requirement exists. Because from the layout\n> of these types, everything will be packed in just fine without\n> __packed.\n\nI will thanks.\n\nJust for my education, is there some rule which tells when __packed is\nto be used? For example the above structures are all 32-bit aligned but\nhow about something like:\n\nstruct foo {\n\tu32 value1;\n\tu8 value2;\n};\n\nIf the on-wire format requires such structures I assume __packed\nis needed here?\n\nThanks!","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2G0B6jRtz9t4b\n\tfor ;\n\tWed, 27 Sep 2017 21:32:58 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752439AbdI0Lcr (ORCPT );\n\tWed, 27 Sep 2017 07:32:47 -0400","from mga11.intel.com ([192.55.52.93]:43583 \"EHLO mga11.intel.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751521AbdI0Lcq (ORCPT );\n\tWed, 27 Sep 2017 07:32:46 -0400","from fmsmga003.fm.intel.com ([10.253.24.29])\n\tby fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t27 Sep 2017 04:32:46 -0700","from lahna.fi.intel.com (HELO lahna) ([10.237.72.157])\n\tby FMSMGA003.fm.intel.com with SMTP; 27 Sep 2017 04:32:42 -0700","by lahna (sSMTP sendmail emulation);\n\tWed, 27 Sep 2017 14:32:41 +0300"],"X-ExtLoop1":"1","X-IronPort-AV":"E=Sophos;i=\"5.42,445,1500966000\"; d=\"scan'208\";a=\"904349160\"","Date":"Wed, 27 Sep 2017 14:32:41 +0300","From":"Mika Westerberg ","To":"David Miller ","Cc":"gregkh@linuxfoundation.org, andreas.noever@gmail.com,\n\tmichael.jamet@intel.com, yehezkel.bernat@intel.com,\n\tamir.jer.levy@intel.com, Mario.Limonciello@dell.com,\n\tlukas@wunner.de, andriy.shevchenko@linux.intel.com, andrew@lunn.ch,\n\tlinux-kernel@vger.kernel.org, netdev@vger.kernel.org","Subject":"Re: [PATCH v2 02/16] thunderbolt: Add support for XDomain properties","Message-ID":"<20170927113241.GB4630@lahna.fi.intel.com>","References":"<20170925110738.68382-1-mika.westerberg@linux.intel.com>\n\t<20170925110738.68382-3-mika.westerberg@linux.intel.com>\n\t<20170926.213354.305351416790211828.davem@davemloft.net>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170926.213354.305351416790211828.davem@davemloft.net>","Organization":"Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo","User-Agent":"Mutt/1.8.3 (2017-05-23)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1776455,"web_url":"http://patchwork.ozlabs.org/comment/1776455/","msgid":"<063D6719AE5E284EB5DD2968C1650D6DD0082864@AcuExch.aculab.com>","list_archive_url":null,"date":"2017-09-27T16:06:50","subject":"RE: [PATCH v2 02/16] thunderbolt: Add support for XDomain properties","submitter":{"id":6689,"url":"http://patchwork.ozlabs.org/api/people/6689/","name":"David Laight","email":"David.Laight@ACULAB.COM"},"content":"From: Mika Westerberg\n> Sent: 27 September 2017 12:33\n...\n> Just for my education, is there some rule which tells when __packed is\n> to be used? For example the above structures are all 32-bit aligned but\n> how about something like:\n> \n> struct foo {\n> \tu32 value1;\n> \tu8 value2;\n> };\n> \n> If the on-wire format requires such structures I assume __packed\n> is needed here?\n\nYou've endianness considerations as well with on-wire formats.\n\n__packed indicates two things:\n1) There will be no padding bytes between fields.\n2) The structure itself might appear on any byte boundary.\n\nThe latter causes the compiler to do byte memory accesses and\nshifts to load/store the data on some architectures.\nSo only mark things __packed when they might be misaligned in\nmemory.\n\n\tDavid","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2N8S3CP7z9tY0\n\tfor ;\n\tThu, 28 Sep 2017 02:10:32 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751578AbdI0QKH convert rfc822-to-8bit (ORCPT\n\t);\n\tWed, 27 Sep 2017 12:10:07 -0400","from smtp-out4.electric.net ([192.162.216.185]:54393 \"EHLO\n\tsmtp-out4.electric.net\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751163AbdI0QKF (ORCPT\n\t); Wed, 27 Sep 2017 12:10:05 -0400","from 1dxErK-0000SU-TQ by out4d.electric.net with emc1-ok (Exim\n\t4.87) (envelope-from )\n\tid 1dxErL-0000cg-Vo; Wed, 27 Sep 2017 09:06:51 -0700","by emcmailer; Wed, 27 Sep 2017 09:06:51 -0700","from [156.67.243.126] (helo=AcuExch.aculab.com)\n\tby out4d.electric.net with esmtps (TLSv1:AES128-SHA:128)\n\t(Exim 4.87) (envelope-from )\n\tid 1dxErK-0000SU-TQ; Wed, 27 Sep 2017 09:06:50 -0700","from ACUEXCH.Aculab.com ([::1]) by AcuExch.aculab.com ([::1]) with\n\tmapi id 14.03.0123.003; Wed, 27 Sep 2017 17:06:51 +0100"],"From":"David Laight ","To":"'Mika Westerberg' ,\n\tDavid Miller ","CC":"\"gregkh@linuxfoundation.org\" ,\n\t\"andreas.noever@gmail.com\" ,\n\t\"michael.jamet@intel.com\" ,\n\t\"yehezkel.bernat@intel.com\" ,\n\t\"amir.jer.levy@intel.com\" ,\n\t\"Mario.Limonciello@dell.com\" ,\n\t\"lukas@wunner.de\" , \"andriy.shevchenko@linux.intel.com\" \n\t, \"andrew@lunn.ch\" ,\n\t\"linux-kernel@vger.kernel.org\" ,\n\t\"netdev@vger.kernel.org\" ","Subject":"RE: [PATCH v2 02/16] thunderbolt: Add support for XDomain properties","Thread-Topic":"[PATCH v2 02/16] thunderbolt: Add support for XDomain\n\tproperties","Thread-Index":"AQHTN4RdP3KjQqGQBUGiA7zogZcNgqLIylrw","Date":"Wed, 27 Sep 2017 16:06:50 +0000","Message-ID":"<063D6719AE5E284EB5DD2968C1650D6DD0082864@AcuExch.aculab.com>","References":"<20170925110738.68382-1-mika.westerberg@linux.intel.com>\n\t<20170925110738.68382-3-mika.westerberg@linux.intel.com>\n\t<20170926.213354.305351416790211828.davem@davemloft.net>\n\t<20170927113241.GB4630@lahna.fi.intel.com>","In-Reply-To":"<20170927113241.GB4630@lahna.fi.intel.com>","Accept-Language":"en-GB, en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","x-originating-ip":"[10.202.99.200]","Content-Type":"text/plain; charset=\"Windows-1252\"","Content-Transfer-Encoding":"8BIT","MIME-Version":"1.0","X-Outbound-IP":"156.67.243.126","X-Env-From":"David.Laight@ACULAB.COM","X-Proto":"esmtps","X-Revdns":"","X-HELO":"AcuExch.aculab.com","X-TLS":"TLSv1:AES128-SHA:128","X-Authenticated_ID":"","X-PolicySMART":"3396946, 3397078","X-Virus-Status":["Scanned by VirusSMART (c)","Scanned by VirusSMART (s)"],"Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1776466,"web_url":"http://patchwork.ozlabs.org/comment/1776466/","msgid":"<20170927.092216.1164977518037321262.davem@davemloft.net>","list_archive_url":null,"date":"2017-09-27T16:22:16","subject":"Re: [PATCH v2 02/16] thunderbolt: Add support for XDomain properties","submitter":{"id":15,"url":"http://patchwork.ozlabs.org/api/people/15/","name":"David Miller","email":"davem@davemloft.net"},"content":"From: Mika Westerberg \nDate: Wed, 27 Sep 2017 14:32:41 +0300\n\n> Just for my education, is there some rule which tells when __packed is\n> to be used? For example the above structures are all 32-bit aligned but\n> how about something like:\n> \n> struct foo {\n> \tu32 value1;\n> \tu8 value2;\n> };\n> \n> If the on-wire format requires such structures I assume __packed\n> is needed here?\n\nUsually header elements are 32-bit aligned in a protocol, so it wouldn't\nbe specified like that.\n\nThe only legitimate case I've seen is where things are purposefully\nmisaligned within the header, like this:\n\nstruct foo {\n\tu16\tx;\n\tu64\ty;\n\tu16\tz;\n};\n\nWhere the 'y' element is 2-byte aligned.\n\nFortunately, those situations are extremely rare.","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2NQH5N0Cz9tXy\n\tfor ;\n\tThu, 28 Sep 2017 02:22:31 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751740AbdI0QWT (ORCPT );\n\tWed, 27 Sep 2017 12:22:19 -0400","from shards.monkeyblade.net ([184.105.139.130]:39490 \"EHLO\n\tshards.monkeyblade.net\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751016AbdI0QWS (ORCPT\n\t); Wed, 27 Sep 2017 12:22:18 -0400","from localhost (74-93-104-102-Washington.hfc.comcastbusiness.net\n\t[74.93.104.102])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(Client did not present a certificate)\n\t(Authenticated sender: davem-davemloft)\n\tby shards.monkeyblade.net (Postfix) with ESMTPSA id 6B30413408BD7;\n\tWed, 27 Sep 2017 09:22:17 -0700 (PDT)"],"Date":"Wed, 27 Sep 2017 09:22:16 -0700 (PDT)","Message-Id":"<20170927.092216.1164977518037321262.davem@davemloft.net>","To":"mika.westerberg@linux.intel.com","Cc":"gregkh@linuxfoundation.org, andreas.noever@gmail.com,\n\tmichael.jamet@intel.com, yehezkel.bernat@intel.com,\n\tamir.jer.levy@intel.com, Mario.Limonciello@dell.com,\n\tlukas@wunner.de, andriy.shevchenko@linux.intel.com, andrew@lunn.ch,\n\tlinux-kernel@vger.kernel.org, netdev@vger.kernel.org","Subject":"Re: [PATCH v2 02/16] thunderbolt: Add support for XDomain properties","From":"David Miller ","In-Reply-To":"<20170927113241.GB4630@lahna.fi.intel.com>","References":"<20170925110738.68382-3-mika.westerberg@linux.intel.com>\n\t<20170926.213354.305351416790211828.davem@davemloft.net>\n\t<20170927113241.GB4630@lahna.fi.intel.com>","X-Mailer":"Mew version 6.7 on Emacs 25.3 / Mule 6.0 (HANACHIRUSATO)","Mime-Version":"1.0","Content-Type":"Text/Plain; charset=us-ascii","Content-Transfer-Encoding":"7bit","X-Greylist":"Sender succeeded SMTP AUTH, not delayed by\n\tmilter-greylist-4.5.12 (shards.monkeyblade.net\n\t[149.20.54.216]); Wed, 27 Sep 2017 09:22:17 -0700 (PDT)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}}]