From: Simon Wunderlich <sw-2YrNx6rUIHYiY0qSoAWiAoQuADTiUCJX@public•gmane.org>
To: David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public•gmane.org>
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public•gmane.org,
b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r@public•gmane.org,
linux-lFZ/pmaqli7XmaaqVzeoHQ@public•gmane.org,
mareklindner-rVWd3aGhH2z5bpWLKbzFeg@public•gmane.org,
antonio-x4xJYDvStAgysxA8WJXlww@public•gmane.org
Subject: Re: [PATCH] Fix ARM BUILD_BUG_ON() errors with batman-adv
Date: Mon, 2 Dec 2013 18:58:47 +0100 [thread overview]
Message-ID: <201312021858.48074.sw@simonwunderlich.de> (raw)
In-Reply-To: <20131130.160547.837987320410619405.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
David,
> From: Russell King - ARM Linux <linux-lFZ/pmaqli7XmaaqVzeoHQ@public•gmane.org>
> Date: Sat, 30 Nov 2013 19:15:53 +0000
>
> > so there should be no undesired side effect from this packing.
>
> There is a huge side effect from ever using the packed attribute, in
> that the compiler can assume absolutely nothing about the alignment of
> any object or sub-object of the type you apply this attribute to.
>
> Even if it is "obvious" that some members will be aligned, the
> compiler cannot take advantage of this assumption because this
> attribute also means that an array of such elements might have
> arbitrary alignment. So you when you get a pointer to one of these
> objects, the compiler has to assume the worst possible case.
>
> This means using 4 byte loads to load a 32-bit quantity, always,
> unconditionally, no matter what.
>
> That's why we should do whatever is necessary to align things properly
> by hand, and use packed only as the last possible and least desirable
> resort.
so far we have two feasible options how to handle batadv_header:
1) Russells patch
2) unrolling these 3 bytes of header into every packet structure
I understand your concerns for __packed in generally, but in this case it
might not be that bad:
* batadv_header is only used by embedding it in other structs (like
batadv_unicast_packet, batadv_broadcast_packet ...) which are well aligned - I
guess there should be no performance problem when accessing through the parent
structure (like unicast_packet->header.version)
* there is one exception in batadv_interface_rx() where we use this
batadv_header directly, but we can easily replace that by using e.g.
batadv_bcast_packet instead.
* It's easier to read and maintain than copying these 3 bytes in every other
structure.
Therefore I'd suggest to pick Russels patch, I can send a fix for
batadv_interface_rx() later, too.
Thanks,
Simon
next prev parent reply other threads:[~2013-12-02 17:58 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-30 19:15 [PATCH] Fix ARM BUILD_BUG_ON() errors with batman-adv Russell King - ARM Linux
[not found] ` <20131130191553.GA16735-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>
2013-11-30 20:12 ` Antonio Quartulli
2013-11-30 23:05 ` [B.A.T.M.A.N.] " Russell King - ARM Linux
[not found] ` <20131130230518.GC16735-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>
2013-12-01 0:27 ` Antonio Quartulli
2013-12-01 14:28 ` [B.A.T.M.A.N.] " Antonio Quartulli
2013-12-01 17:13 ` Russell King - ARM Linux
2013-12-02 10:24 ` [B.A.T.M.A.N.] [PATCH] Fix ARM BUILD_BUG_ON() errors withbatman-adv David Laight
2013-12-02 12:50 ` Antonio Quartulli
2013-12-02 13:10 ` David Laight
2013-12-02 16:20 ` David Miller
[not found] ` <529B47AA.8060403-x4xJYDvStAgysxA8WJXlww@public.gmane.org>
2013-12-01 19:21 ` [PATCH] Fix ARM BUILD_BUG_ON() errors with batman-adv David Miller
2013-12-01 20:40 ` [B.A.T.M.A.N.] " Antonio Quartulli
2013-11-30 21:05 ` David Miller
2013-11-30 23:03 ` Russell King - ARM Linux
[not found] ` <20131130.160547.837987320410619405.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2013-12-02 17:58 ` Simon Wunderlich [this message]
[not found] ` <201312021858.48074.sw-2YrNx6rUIHYiY0qSoAWiAoQuADTiUCJX@public.gmane.org>
2013-12-02 18:38 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201312021858.48074.sw@simonwunderlich.de \
--to=sw-2yrnx6ruihyiy0qsoawiaoquadtiucjx@public$(echo .)gmane.org \
--cc=antonio-x4xJYDvStAgysxA8WJXlww@public$(echo .)gmane.org \
--cc=b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r@public$(echo .)gmane.org \
--cc=davem-fT/PcQaiUtIeIZ0/mPfg9Q@public$(echo .)gmane.org \
--cc=linux-lFZ/pmaqli7XmaaqVzeoHQ@public$(echo .)gmane.org \
--cc=mareklindner-rVWd3aGhH2z5bpWLKbzFeg@public$(echo .)gmane.org \
--cc=netdev-u79uwXL29TY76Z2rM5mHXA@public$(echo .)gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox