Re: [PATCH v2 02/16] thunderbolt: Add support for XDomain properties

From	Mika Westerberg <mika.westerberg@linux.intel.com>
Newsgroups	linux.kernel
Subject	Re: [PATCH v2 02/16] thunderbolt: Add support for XDomain properties
Date	2017-09-27 13:40 +0200
Message-ID	<uuiR4-1LQ-19@gated-at.bofh.it> (permalink)
References	<utzqV-5xX-3@gated-at.bofh.it> <utzAD-5BH-23@gated-at.bofh.it> <uuciB-5Yf-7@gated-at.bofh.it>
Organization	Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo

Show all headers | View raw

On Tue, Sep 26, 2017 at 09:33:54PM -0700, David Miller wrote:
> From: Mika Westerberg <mika.westerberg@linux.intel.com>
> Date: Mon, 25 Sep 2017 14:07:24 +0300
> 
> > +struct tb_property_entry {
> > +	u32 key_hi;
> > +	u32 key_lo;
> > +	u16 length;
> > +	u8 reserved;
> > +	u8 type;
> > +	u32 value;
> > +} __packed;
> > +
> > +struct tb_property_rootdir_entry {
> > +	u32 magic;
> > +	u32 length;
> > +	struct tb_property_entry entries[];
> > +} __packed;
> > +
> > +struct tb_property_dir_entry {
> > +	u32 uuid[4];
> > +	struct tb_property_entry entries[];
> > +} __packed;
> 
> There is no apparent need for __packed here, and __packed should be
> avoided unless absolutely necessary as it pessimizes the code
> significantly on some architectures.
> 
> Please remove __packed from these datastructures unless you can
> prove it is absolutely needed and, in such case, please document
> in a comment why that requirement exists.  Because from the layout
> of these types, everything will be packed in just fine without
> __packed.

I will thanks.

Just for my education, is there some rule which tells when __packed is
to be used? For example the above structures are all 32-bit aligned but
how about something like:

struct foo {
	u32 value1;
	u8 value2;
};

If the on-wire format requires such structures I assume __packed
is needed here?

Thanks!

Thread

Re: [PATCH v2 02/16] thunderbolt: Add support for XDomain  properties David Miller <davem@davemloft.net> - 2017-09-27 06:40 +0200
  Re: [PATCH v2 02/16] thunderbolt: Add support for XDomain properties Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-09-27 13:40 +0200
    RE: [PATCH v2 02/16] thunderbolt: Add support for XDomain properties David Laight <David.Laight@ACULAB.COM> - 2017-09-27 18:20 +0200
    Re: [PATCH v2 02/16] thunderbolt: Add support for XDomain  properties David Miller <davem@davemloft.net> - 2017-09-27 18:30 +0200

csiph-web