[NFS] Increase size of struct fid raw buffer - Kernel

This is a discussion on [NFS] Increase size of struct fid raw buffer - Kernel ; >From ba713d661333d790db5106c54d1f0ee94a876944 Mon Sep 17 00:00:00 2001 From: Steven Whitehouse Date: Mon, 7 Apr 2008 09:53:24 +0100 Subject: [PATCH] [NFS] Increase size of struct fid raw buffer GFS2 requires the NFS filehandle buffer to be larger than the minimum size ...

+ Reply to Thread
Results 1 to 7 of 7

Thread: [NFS] Increase size of struct fid raw buffer

  1. [NFS] Increase size of struct fid raw buffer

    >From ba713d661333d790db5106c54d1f0ee94a876944 Mon Sep 17 00:00:00 2001
    From: Steven Whitehouse
    Date: Mon, 7 Apr 2008 09:53:24 +0100
    Subject: [PATCH] [NFS] Increase size of struct fid raw buffer

    GFS2 requires the NFS filehandle buffer to be larger than the
    minimum size as per the bug report: http://lkml.org/lkml/2007/10/24/374
    Its a pretty trivial fix for now and I've done a test which shows
    that it works ok.

    Signed-off-by: Steven Whitehouse
    Cc: Christoph Hellwig
    Cc: Neil Brown
    Cc: J. Bruce Fields
    Cc: Adrian Bunk

    diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
    index adcbb05..a6e928d 100644
    --- a/include/linux/exportfs.h
    +++ b/include/linux/exportfs.h
    @@ -43,7 +43,7 @@ struct fid {
    u32 parent_ino;
    u32 parent_gen;
    } i32;
    - __u32 raw[6];
    + __u32 raw[8];
    };
    };

    --
    1.5.1.2



    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [NFS] Increase size of struct fid raw buffer



    On Mon, 7 Apr 2008, Steven Whitehouse wrote:
    >
    > GFS2 requires the NFS filehandle buffer to be larger than the
    > minimum size as per the bug report: http://lkml.org/lkml/2007/10/24/374
    > Its a pretty trivial fix for now and I've done a test which shows
    > that it works ok.


    I'm not seeing the point of this.

    Every single instance of "struct fid" that I saw in a quick grep was
    created not as a "struct fid", but as some other data structure that was
    then cast to a "struct fid *".

    So the _underlying_ size of "struct fid" seems to be pretty random, and
    totally unrelated to this declaration.

    But admittedly that really was just a quick grep, and maybe I missed
    something. But it seems like this patch doesn't really change anything,
    just largely makes a change in a structure that is apparently used as an
    opaque pointer.

    Is there anything that actually uses "struct fid" as an _allocation_
    entity?

    And if not, then that "_u32 raw[6]" should probably be a un-sized "_u32
    raw[]" instead, no?

    Linus
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [NFS] Increase size of struct fid raw buffer

    On Tue, Apr 08, 2008 at 08:50:05AM +0100, Steven Whitehouse wrote:
    > I'm happy with that solution, although I'd assumed that the reason this
    > field had a size in the first place was that the NFS people had a plan
    > to use the structure as an allocation entity in the future. Can an NFS
    > developer please confirm/deny this?
    >
    > If everybody is happy with the plan, then I'll send a patch to make the
    > change as you suggest shortly,


    I've introduce it and I don't plan to use it as allocation entity. I
    don't quite remember why I sized it either, so it's conceptually fine
    to make it a VLA. Just do some testing with that variant please.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [NFS] Increase size of struct fid raw buffer

    Hi,

    On Mon, 2008-04-07 at 08:54 -0700, Linus Torvalds wrote:
    >
    > On Mon, 7 Apr 2008, Steven Whitehouse wrote:
    > >
    > > GFS2 requires the NFS filehandle buffer to be larger than the
    > > minimum size as per the bug report: http://lkml.org/lkml/2007/10/24/374
    > > Its a pretty trivial fix for now and I've done a test which shows
    > > that it works ok.

    >
    > I'm not seeing the point of this.
    >
    > Every single instance of "struct fid" that I saw in a quick grep was
    > created not as a "struct fid", but as some other data structure that was
    > then cast to a "struct fid *".
    >
    > So the _underlying_ size of "struct fid" seems to be pretty random, and
    > totally unrelated to this declaration.
    >
    > But admittedly that really was just a quick grep, and maybe I missed
    > something. But it seems like this patch doesn't really change anything,
    > just largely makes a change in a structure that is apparently used as an
    > opaque pointer.
    >
    > Is there anything that actually uses "struct fid" as an _allocation_
    > entity?
    >
    > And if not, then that "_u32 raw[6]" should probably be a un-sized "_u32
    > raw[]" instead, no?
    >
    > Linus


    I'm happy with that solution, although I'd assumed that the reason this
    field had a size in the first place was that the NFS people had a plan
    to use the structure as an allocation entity in the future. Can an NFS
    developer please confirm/deny this?

    If everybody is happy with the plan, then I'll send a patch to make the
    change as you suggest shortly,

    Steve.


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [NFS] Increase size of struct fid raw buffer

    Hi,

    On Tue, 2008-04-08 at 03:58 -0400, Christoph Hellwig wrote:
    > On Tue, Apr 08, 2008 at 08:50:05AM +0100, Steven Whitehouse wrote:
    > > I'm happy with that solution, although I'd assumed that the reason this
    > > field had a size in the first place was that the NFS people had a plan
    > > to use the structure as an allocation entity in the future. Can an NFS
    > > developer please confirm/deny this?
    > >
    > > If everybody is happy with the plan, then I'll send a patch to make the
    > > change as you suggest shortly,

    >
    > I've introduce it and I don't plan to use it as allocation entity. I
    > don't quite remember why I sized it either, so it's conceptually fine
    > to make it a VLA. Just do some testing with that variant please.


    Then we get this:

    In file included from mm/shmem.c:30:
    include/linux/exportfs.h:46: error: flexible array member in union
    make[1]: *** [mm/shmem.o] Error 1
    make: *** [mm] Error 2

    but we can have a zero length array though, so I'll test that and send a
    patch in due course,

    Steve.


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. [NFS] Use a zero sized array for raw field in struct fid

    >From b8732f8bea4e8abc331b8fa58a4047c91e2e7d02 Mon Sep 17 00:00:00 2001
    From: Steven Whitehouse
    Date: Tue, 8 Apr 2008 13:12:52 +0100
    Subject: [PATCH] [NFS] Use a zero sized array for raw field in struct fid

    The raw field's size can vary so we use a zero sized array since
    gcc will not allow a variable sized array inside a union. This
    has been tested with ext3 and gfs2 and relates to the bug
    report: http://lkml.org/lkml/2007/10/24/374 and discussion
    thread: http://lkml.org/lkml/2008/4/7/65

    Signed-off-by: Steven Whitehouse
    Cc: Christoph Hellwig
    Cc: Neil Brown
    Cc: J. Bruce Fields
    Cc: Adrian Bunk

    diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
    index adcbb05..de8387b 100644
    --- a/include/linux/exportfs.h
    +++ b/include/linux/exportfs.h
    @@ -43,7 +43,7 @@ struct fid {
    u32 parent_ino;
    u32 parent_gen;
    } i32;
    - __u32 raw[6];
    + __u32 raw[0];
    };
    };

    --
    1.5.1.2



    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [NFS] Use a zero sized array for raw field in struct fid

    On Tue, Apr 08, 2008 at 02:01:26PM +0100, Steven Whitehouse wrote:
    > >From b8732f8bea4e8abc331b8fa58a4047c91e2e7d02 Mon Sep 17 00:00:00 2001

    > From: Steven Whitehouse
    > Date: Tue, 8 Apr 2008 13:12:52 +0100
    > Subject: [PATCH] [NFS] Use a zero sized array for raw field in struct fid
    >
    > The raw field's size can vary so we use a zero sized array since
    > gcc will not allow a variable sized array inside a union. This
    > has been tested with ext3 and gfs2 and relates to the bug
    > report: http://lkml.org/lkml/2007/10/24/374 and discussion
    > thread: http://lkml.org/lkml/2008/4/7/65


    OK; queued for 2.6.26.--b.

    >
    > Signed-off-by: Steven Whitehouse
    > Cc: Christoph Hellwig
    > Cc: Neil Brown
    > Cc: J. Bruce Fields
    > Cc: Adrian Bunk
    >
    > diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
    > index adcbb05..de8387b 100644
    > --- a/include/linux/exportfs.h
    > +++ b/include/linux/exportfs.h
    > @@ -43,7 +43,7 @@ struct fid {
    > u32 parent_ino;
    > u32 parent_gen;
    > } i32;
    > - __u32 raw[6];
    > + __u32 raw[0];
    > };
    > };
    >
    > --
    > 1.5.1.2
    >
    >
    >

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread