Re: [PATCH 2.4] jffs2 aligment problems

[David Woodhouse <dwmw2@infradead.org>]

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.

Most of the time the buffer passed to the flash write routines will be
word-aligned. Occasionally it'll be unaligned but since there's a
distinct correlation between those arches on which we can't do fixups
and those machines on which flash is primary storage, putting
get_unaligned() in is a _correctness_ issue. And I don't care that it
might be slightly slower in the common case; as I said, it's in the
noise.

> 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". 

Not just old ARM chips. Some new chips too; especially MMU-less ones.

> 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.

Yes. That's why I suggested we should have two forms -- for 'possibly'
and 'probably' unaligned.

> > 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".

Yes. That's why I was told to remove our current get_unaligned() from
the flash drivers. I'm perfectly happy to put it back.


-- 
dwmw2