Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: [PATCH 2.4] jffs2 aligment problems
Original-Message-ID: <Pine.LNX.4.58.0406070900010.6162@ppc970.osdl.org>
Date: Mon, 7 Jun 2004 16:05:11 GMT
Message-ID: <fa.gtu1ue5.8i8rpj@ifi.uio.no>

On Mon, 7 Jun 2004, Thomas Gleixner wrote:
>
> On Monday 07 June 2004 17:08, Greg Weeks wrote:
> > This fixed some jffs2 alignment problems we saw on an IXP425 based
> > XScale board. I just got pinged that I was supposed to post this patch
> > in case anyone else finds it usefull. This was against a modified 2.4.19
> > kernel.
>
> Enable CONFIG_ALIGNMENT_TRAP instead of tweaking the code.
> JFFS2 / MTD must be allowed to do unaligned access

Wrong.

Pleas fix jffs2 to use the proper "get_unaligned()"/"put_unaligned()"
instead.

Emulating unaligned accesses with traps (even even the architecture
supports it, which isn't universally true) is _stupid_ when we have
perfectly well-defined macros for them that do it faster and are
_designed_ for this.

On architectures where it doesn't matter, the macros just do the access,
so it's not like you're slowing anything down.

		Linus

Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: [PATCH 2.4] jffs2 aligment problems
Original-Message-ID: <Pine.LNX.4.58.0406071218240.1637@ppc970.osdl.org>
Date: Mon, 7 Jun 2004 19:26:02 GMT
Message-ID: <fa.gsu3tui.aiqqpu@ifi.uio.no>

On Mon, 7 Jun 2004, David Woodhouse wrote:
> > > Pleas fix jffs2 to use the proper "get_unaligned()"/"put_unaligned()"
> > > instead.
>
> > I'll let you have a bun fight with dwmw2 and networking people over
> > this.  I'm standing well clear. 8)
>
> S'fine by me. I already added get_unaligned() to the flash drivers years
> ago -- it's actually those, not JFFS2 itself which needs it. Alan
> insisted I remove it on the basis that much of our network code is
> similarly broken anyway; an arch without fixups is dead in the water
> whatever happens.

I don't see it as a correctness issue, I see it as a performance issue.

Yes, the old ARM chips that can't do unaligned accesses and can't even
trap on them probably have a number of cases where they literally do the
wrong thing, and I think most people will say "tough luck, don't do that
then".

But get_unaligned() makes sense quite apart from that usage too. Notably,
many architectures can cheaply do unaligned accesses when they are known
to be unaligned, but take thousands of cycles to fix up traps. Alpha comes
to mind, and this was actually what "get_unaligned()" was really designed
for.

> Anything which _could_ be unaligned should be marked as such, even if we
> do have two levels ('possibly unaligned', 'probably unaligned') where
> the latter behaves purely as an optimisation on most arches, just like
> our current get_unaligned() does.

Right now we might as well consider the "get_unaligned()" to be a "quite
possibly unaligned" as opposed to "this just _might_ be unaligned".

That's the current usage pattern anyway. Nothing really _guarantees_ that
unaligned accesses are marked with "get_unaligned()", but the unaligned
fixup is at least supposed to be _rare_.

Put another way: imagine if the unaligned handler did a printk(). That
shouldn't flood the logs under normal load.

			Linus

Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: [PATCH 2.4] jffs2 aligment problems
Original-Message-ID: <Pine.LNX.4.58.0406071351450.1637@ppc970.osdl.org>
Date: Mon, 7 Jun 2004 20:56:08 GMT
Message-ID: <fa.gtu7uub.9iqrpn@ifi.uio.no>

On Mon, 7 Jun 2004, David Woodhouse wrote:
>
> On Mon, 2004-06-07 at 12:22 -0700, Linus Torvalds wrote:
> > I don't see it as a correctness issue, I see it as a performance issue.
>
> In the case in question it's very much _not_ a performance issue. We're
> writing a buffer to FLASH memory. The time it takes to read the word
> from RAM is entirely lost in the noise compared with the time it takes
> to write it to the flash.

Not if you have to take an alignment fault, which is easily several
thousand cycles.

Think of "get_unaligned()" as a worst-case limiter. It can make the best
case be worse on architectures where it matters, but it can make the worst
case go from thousands of cycles to just single cycles.

And your flash isn't _that_ slow. Thousands of cycles that can't even
overlap with any flash IO _does_ show up.

Now, whether the unaligned case is common enough for people to even worry,
I don't know.

			Linus