lkml.org 
[lkml]   [2009]   [Feb]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] fs: Add new pre-allocation ioctls to vfs for compatibility with legacy xfs ioctls
    On Sun, 1 Feb 2009, Boaz Harrosh wrote:
    > Arnd Bergmann wrote:
    > > On Saturday 31 January 2009, Andrew Morton wrote:
    > >> Is this written in a standard somewhere? Is it guaranteed?
    > >
    > > Alignment is defined in the architecture psABI documents.
    > > Unfortunately, many of them were written before the 'long long'
    > > type became part of the C standard, so it's not strictly guaranteed.
    > > AFAICT, the alignment of __u64 on x86 is the same as the alignment
    > > of 'double' by convention.
    > >
    > > However, the problem is well-understood: x86 is the only one
    > > that has a problem in 32/64 bit compat mode. m68k has similar
    > > issues with 16/32 bit integers, but those don't apply here.
    > >
    > >> If some (perhaps non-gcc) compiler were to lay this out differently
    > >> (perhaps with suitable command-line options) then that's liveable
    > >> with - as long as the kernel never changes the layout. Of course
    > >> it would be better to avoid this if poss.
    > >
    > > If a compiler was using irregular structure alignment, all sorts of
    > > library interfaces would break. The kernel ABI is only a small part
    > > of the problem then.
    > >
    > >> The other potential issue with a structure like this is that there's a
    > >> risk that it will lead us to copy four bytes of uninitialised kernel
    > >> memory out to userspace.
    > >>
    > >> IOW, it seems a generally bad idea to rely upon compiler-added padding
    > >> for this sort of thing.
    > >
    > > Agreed in general, but the whole point of this particular patch was to
    > > provide compatibility with an interface that has been part of XFS for
    > > many years.
    > > Linux already has a better interface for new users (sys_fallocate), so
    > > changing the patch would not be helpful and not provide any advantage.
    > >
    > > There is also no leak of uninitialized data here, because this structure
    > > is only read, never written.
    > >
    > > Arnd <><
    > Arnd Bergmann wrote:
    > > +struct space_resv {
    > > + __s16 l_type;
    > > + __s16 l_whence;
    > > + __s64 l_start;
    > > + __s64 l_len; /* len == 0 means until end of file */
    > > + __s32 l_sysid;
    > > + __u32 l_pid;
    > > + __s32 l_pad[4]; /* reserve area */
    > > +};
    >
    > What about telling the compiler exactly what you said above, just
    > to be sure we all mean the same thing. (And as documentation for new
    > comers):
    >
    > +struct space_resv_64 {
    > + __s16 l_type;
    > + __s16 l_whence;
    > + __u32 reserved;
    > + __s64 l_start;
    > + __s64 l_len; /* len == 0 means until end of file */
    > + __s32 l_sysid;
    > + __u32 l_pid;
    > + __s32 l_pad[4]; /* reserve area */
    > +} __packed;

    Because the compiler will assume all fields are always unaligned and will use very
    suboptimal code to access them?

    Gr{oetje,eeting}s,

    Geert

    --
    Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

    In personal conversations with technical people, I call myself a hacker. But
    when I'm talking to journalists I just say "programmer" or something like that.
    -- Linus Torvalds


    \
     
     \ /
      Last update: 2009-02-01 11:07    [W:0.025 / U:29.568 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site