Re: NFS structure allocation alignment patch

[Richard Curnow <Richard.Curnow@superh.com>]

On Thu, 10 Jul, 2003 at 12:05pm, David Woodhouse wrote:
> On Mon, 2003-06-30 at 14:52, Richard Curnow wrote:
> > Hi Trond, Marcelo,
> > 
> > Below is a patch against 2.4.21 to tidy up the allocation of two
> > structures in nfs3_proc_unlink_setup.  We need this change for NFS to
> > work on the sh64 architecture, which has just been merged into 2.4 in
> > the last couple of days.  Otherwise, 'res' is 4-byte aligned but not
> > necessarily 8-byte aligned, but struct nfs_attr contains fields that are
> > 8 bytes wide.  This leads to alignment exceptions on loads and stores
> > into that structure.
> 
> What's wrong with alignment exceptions? They get fixed up by your
> exception handler, surely?
> 
> If you assert that it's a performance-critical path and hence we
> shouldn't be relying on the exception fixup, that's fine -- but in that
> case it's not a correctness fix, it's just an optimisation.

Apart from this issue, we haven't seen any code- or compiler-related
problems due to misaligned loads and stores occurring.  Indeed, because
gcc takes care to lay out structures to honour load/store alignments, we
deliberately don't 'fix-up' misaligned accesses, rather an oops is
raised since they are almost certainly due to something having gone
wrong, e.g.  a pointer has been overwritten somehow.

I bet someone will wonder how the misalignment hadn't shown up before.
Recall there were 2 structures being allocated with 1 kmalloc call.  The
first (nfs3_diropargs) contains 2 pointers and an unsigned int.  The
second (nfs_fattr) contains amongst other things some __u64's.  On sh64,
the __u64's will be accessed with single 8-byte load/store operations.
Although the SHmedia instruction set fully supports 64-bit addressing,
the current generation implements 32-bit (with sign-extension to 64) so
the toolchains currently store pointers as 32-bit to save memory &
cache.  Hence the 1st structure is only compiled with 4-byte alignment
=> insufficiently aligned 2nd structure in the old code.  I presume all
the other 64-bit architectures already use 64-bit pointers so the
alignment problem didn't happen.  With the patch I sent, the required
alignments are assured for any architecture.

HTH
Richard

-- 
Richard \\\ SuperH Core+Debug Architect /// .. At home ..
  P.    /// richard.curnow@superh.com  ///  rc@rc0.org.uk
Curnow  \\\ http://www.superh.com/    ///  www.rc0.org.uk