All of lore.kernel.org
 help / color / mirror / Atom feed
From: Russell King - ARM Linux <linux@armlinux.org.uk>
To: Eric Nelson <eric@nelint.com>
Cc: David Laight <David.Laight@ACULAB.COM>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"andrew@lunn.ch" <andrew@lunn.ch>,
	"fugang.duan@nxp.com" <fugang.duan@nxp.com>,
	"otavio@ossystems.com.br" <otavio@ossystems.com.br>,
	"edumazet@google.com" <edumazet@google.com>,
	"troy.kisky@boundarydevices.com" <troy.kisky@boundarydevices.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"u.kleine-koenig@pengutronix.de" <u.kleine-koenig@pengutronix.de>
Subject: Re: [PATCH 3/3] net: fec: align IP header in hardware
Date: Sat, 1 Oct 2016 20:52:15 +0100	[thread overview]
Message-ID: <20161001195215.GP1041@n2100.armlinux.org.uk> (raw)
In-Reply-To: <638d2aa0-eb4a-dd4b-e516-d7ad89eca627@nelint.com>

On Fri, Sep 30, 2016 at 07:16:12AM -0700, Eric Nelson wrote:
> On ARM, the CPU can't handle misaligned memory cycles without
> taking an alignment fault and NET_IP_ALIGN is set to 2.

Let's get this right...  With Linux on MMU parts:

On ARMv6+, unaligned memory cycles using the LDR, LDRH and corresponding
store instructions are handled in hardware without any exception being
raised.

On pre-ARMv6, such instructions raise an alignment exception, and we fix
up the load/store manually.

Where things behave the same is with the LDM (load multiple) and STM
(store multiple) instructions.  Hardware does not fix these up if they
are unaligned: it is expected that the base address will always be
aligned to a 32-bit word.

For some reason, the compiler guys have decided it's okay to use these
instructions as an optimisation, and I see no way to disable this
behaviour.

> On ARM, we have CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y
> but I find it hard to believe that taking alignment faults is more
> efficient than adding two bytes to the start of the frame.

... see above, hence why CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is set.
It's only when the compiler decides to do silly things that things go
wrong.  However, net code does not care about that configuration
setting, so it's irrelevant to this discussion.

The issue with the networking layer is that it passes around structure
pointers which may not be "naturally aligned" - technically it goes
against the C standard specs.  However, you'll find it hard to argue
against this, so we have to accept that the networking people expect
it to work.

The optimisation that the C compiler uses (using LDM to access multiple
32-bit consecutive words) is legal and efficient when the structure
pointers are aligned as it expects, but that all breaks if the pointer
is not so aligned.  So, raising it as a bug against the C compiler isn't
going to work either.

What may work is to raise a feature request with compiler people to have
a mechanism to disable the LDM/STM optimisation for code where we know
that pointers may not be naturally aligned.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

  reply	other threads:[~2016-10-01 19:52 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-23 16:43 Alignment issues with freescale FEC driver Eric Nelson
2016-09-23 16:43 ` Eric Nelson
2016-09-23 16:54 ` Eric Dumazet
2016-09-23 16:54   ` Eric Dumazet
2016-09-23 17:19   ` Eric Nelson
2016-09-23 17:19     ` Eric Nelson
2016-09-23 17:33     ` Eric Nelson
2016-09-23 17:33       ` Eric Nelson
2016-09-23 18:13       ` Andrew Lunn
2016-09-23 18:13         ` Andrew Lunn
2016-09-23 18:30         ` Russell King - ARM Linux
2016-09-23 18:30           ` Russell King - ARM Linux
2016-09-23 18:39           ` Eric Nelson
2016-09-23 18:39             ` Eric Nelson
2016-09-23 18:35         ` Eric Nelson
2016-09-23 18:35           ` Eric Nelson
2016-09-24  2:45           ` David Miller
2016-09-24  2:45             ` David Miller
2016-09-24  5:13             ` Andy Duan
2016-09-24  5:13               ` Andy Duan
2016-09-24 14:42               ` [PATCH 0/3] net: fec: updates to align IP header Eric Nelson
2016-09-24 14:42                 ` [PATCH 1/3] net: fec: remove QUIRK_HAS_RACC from i.mx25 Eric Nelson
2016-09-24 14:42                 ` [PATCH 2/3] net: fec: remove QUIRK_HAS_RACC from i.mx27 Eric Nelson
2016-09-24 14:42                 ` [PATCH 3/3] net: fec: align IP header in hardware Eric Nelson
2016-09-26  9:26                   ` David Laight
2016-09-26 18:39                     ` Eric Nelson
2016-09-28 16:42                       ` David Laight
2016-09-28 17:14                         ` Eric Nelson
2016-09-28 17:25                           ` Russell King - ARM Linux
2016-09-28 18:01                             ` Eric Nelson
2016-09-29 11:07                           ` David Laight
2016-09-30 13:27                             ` Eric Nelson
2016-09-30 13:49                               ` David Laight
2016-09-30 14:16                                 ` Eric Nelson
2016-10-01 19:52                                   ` Russell King - ARM Linux [this message]
2016-10-03 16:42                                     ` David Laight
2016-10-03 18:48                                     ` Eric Nelson
2016-10-08  2:44                               ` Andy Duan
2016-09-24 15:09                 ` [PATCH 0/3] net: fec: updates to align IP header Andy Duan
2016-09-24 15:29                   ` Eric Nelson
2016-09-27 11:40                 ` David Miller
2016-09-24  2:43       ` Alignment issues with freescale FEC driver David Miller
2016-09-24  2:43         ` David Miller
2016-09-24 12:27         ` Eric Nelson
2016-09-24 12:27           ` Eric Nelson
2016-09-23 17:37     ` Russell King - ARM Linux
2016-09-23 17:37       ` Russell King - ARM Linux
2016-09-23 18:26       ` Eric Nelson
2016-09-23 18:26         ` Eric Nelson
2016-09-23 18:37         ` Russell King - ARM Linux
2016-09-23 18:37           ` Russell King - ARM Linux
2016-09-23 18:49           ` Eric Nelson
2016-09-23 18:49             ` Eric Nelson
2016-09-23 20:22           ` Uwe Kleine-König
2016-09-23 20:22             ` Uwe Kleine-König

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=20161001195215.GP1041@n2100.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=David.Laight@ACULAB.COM \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=eric@nelint.com \
    --cc=fugang.duan@nxp.com \
    --cc=netdev@vger.kernel.org \
    --cc=otavio@ossystems.com.br \
    --cc=troy.kisky@boundarydevices.com \
    --cc=u.kleine-koenig@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.