[Cluster-devel] [PATCH 2/4] GFS2: Reduce inode size by using 32-bit i_generation
Andrew Price
anprice at redhat.com
Thu Oct 29 17:21:24 UTC 2015
Hi Bob,
On 28/10/15 14:20, Bob Peterson wrote:
> This patch removes variable i_generation from gfs2_inode. In its
> place, we use inode->i_generation which is pretty much the same thing,
> in 32-bits. The loss of 32 bits should not be a problem.
>
> Signed-off-by: Bob Peterson <rpeterso at redhat.com>
> ---
> fs/gfs2/glops.c | 2 +-
> fs/gfs2/incore.h | 9 +++++++--
> fs/gfs2/inode.c | 5 +++--
> fs/gfs2/rgrp.c | 16 ++++++++--------
> fs/gfs2/rgrp.h | 2 +-
> fs/gfs2/super.c | 2 +-
> include/uapi/linux/gfs2_ondisk.h | 25 +++++++++++++++++++++----
> 7 files changed, 42 insertions(+), 19 deletions(-)
>
> diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
> index 1f6c9c3..79cd4a9 100644
> --- a/fs/gfs2/glops.c
> +++ b/fs/gfs2/glops.c
> @@ -358,7 +358,7 @@ static int gfs2_dinode_in(struct gfs2_inode *ip, const void *buf)
> ip->i_inode.i_ctime.tv_nsec = be32_to_cpu(str->di_ctime_nsec);
>
> ip->i_goal = be64_to_cpu(str->di_goal_meta);
> - ip->i_generation = be64_to_cpu(str->di_generation);
> + ip->i_inode.i_generation = be32_to_cpu(str->di_gen2);
>
> ip->i_diskflags = be32_to_cpu(str->di_flags);
> ip->i_eattr = be64_to_cpu(str->di_eattr);
> diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
> index 76c9c77..8ca88b9 100644
> --- a/fs/gfs2/incore.h
> +++ b/fs/gfs2/incore.h
> @@ -88,7 +88,13 @@ struct gfs2_rgrpd {
> u32 rd_reserved; /* number of blocks reserved */
> u32 rd_free_clone;
> u32 rd_dinodes;
> - u64 rd_igeneration;
> + union {
> + u64 rd_igeneration;
> + struct {
> + u32 rd_igen1;
> + u32 rd_igen2;
> + };
> + };
Is this union necessary given that it's not an on-disk structure?
> struct gfs2_bitmap *rd_bits;
> struct gfs2_sbd *rd_sbd;
> struct gfs2_rgrp_lvb *rd_rgl;
> @@ -385,7 +391,6 @@ struct gfs2_inode {
> struct inode i_inode;
> u64 i_no_addr;
> u64 i_no_formal_ino;
> - u64 i_generation;
> u64 i_eattr;
> unsigned long i_flags; /* GIF_... */
> struct gfs2_glock *i_gl; /* Move into i_gh? */
> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> index 063fdfc..cc815ab 100644
> --- a/fs/gfs2/inode.c
> +++ b/fs/gfs2/inode.c
> @@ -394,8 +394,9 @@ static int alloc_dinode(struct gfs2_inode *ip, u32 flags, unsigned *dblocks)
> if (error)
> goto out_ipreserv;
>
> - error = gfs2_alloc_blocks(ip, &ip->i_no_addr, dblocks, 1, &ip->i_generation);
> - ip->i_no_formal_ino = ip->i_generation;
> + error = gfs2_alloc_blocks(ip, &ip->i_no_addr, dblocks, 1,
> + &ip->i_inode.i_generation);
> + ip->i_no_formal_ino = ip->i_inode.i_generation;
> ip->i_inode.i_ino = ip->i_no_addr;
> ip->i_goal = ip->i_no_addr;
>
> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> index 475985d..34c42a8 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -1062,7 +1062,7 @@ static void gfs2_rgrp_in(struct gfs2_rgrpd *rgd, const void *buf)
> rgd->rd_flags |= rg_flags;
> rgd->rd_free = be32_to_cpu(str->rg_free);
> rgd->rd_dinodes = be32_to_cpu(str->rg_dinodes);
> - rgd->rd_igeneration = be64_to_cpu(str->rg_igeneration);
> + rgd->rd_igen2 = be32_to_cpu(str->rg_igen2);
> }
>
> static void gfs2_rgrp_out(struct gfs2_rgrpd *rgd, void *buf)
> @@ -1073,7 +1073,7 @@ static void gfs2_rgrp_out(struct gfs2_rgrpd *rgd, void *buf)
> str->rg_free = cpu_to_be32(rgd->rd_free);
> str->rg_dinodes = cpu_to_be32(rgd->rd_dinodes);
> str->__pad = cpu_to_be32(0);
> - str->rg_igeneration = cpu_to_be64(rgd->rd_igeneration);
> + str->rg_igen2 = cpu_to_be64(rgd->rd_igen2);
Probably should be using cpu_to_be32()
> memset(&str->rg_reserved, 0, sizeof(str->rg_reserved));
> }
>
> @@ -1084,7 +1084,7 @@ static int gfs2_rgrp_lvb_valid(struct gfs2_rgrpd *rgd)
>
> if (rgl->rl_flags != str->rg_flags || rgl->rl_free != str->rg_free ||
> rgl->rl_dinodes != str->rg_dinodes ||
> - rgl->rl_igeneration != str->rg_igeneration)
> + rgl->rl_igen2 != str->rg_igen2)
> return 0;
> return 1;
> }
> @@ -1097,7 +1097,7 @@ static void gfs2_rgrp_ondisk2lvb(struct gfs2_rgrp_lvb *rgl, const void *buf)
> rgl->rl_flags = str->rg_flags;
> rgl->rl_free = str->rg_free;
> rgl->rl_dinodes = str->rg_dinodes;
> - rgl->rl_igeneration = str->rg_igeneration;
> + rgl->rl_igen2 = str->rg_igen2;
> rgl->__pad = 0UL;
> }
>
> @@ -1229,7 +1229,7 @@ static int update_rgrp_lvb(struct gfs2_rgrpd *rgd)
> rgd->rd_free = be32_to_cpu(rgd->rd_rgl->rl_free);
> rgd->rd_free_clone = rgd->rd_free;
> rgd->rd_dinodes = be32_to_cpu(rgd->rd_rgl->rl_dinodes);
> - rgd->rd_igeneration = be64_to_cpu(rgd->rd_rgl->rl_igeneration);
> + rgd->rd_igen2 = be64_to_cpu(rgd->rd_rgl->rl_igen2);
Probably should be using be32_to_cpu()
> return 0;
> }
>
> @@ -2334,7 +2334,7 @@ static void gfs2_set_alloc_start(struct gfs2_rbm *rbm,
> */
>
> int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks,
> - bool dinode, u64 *generation)
> + bool dinode, u32 *generation)
> {
> struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
> struct buffer_head *dibh;
> @@ -2390,9 +2390,9 @@ int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks,
> rbm.rgd->rd_free -= *nblocks;
> if (dinode) {
> rbm.rgd->rd_dinodes++;
> - *generation = rbm.rgd->rd_igeneration++;
> + *generation = rbm.rgd->rd_igen2++;
> if (*generation == 0)
> - *generation = rbm.rgd->rd_igeneration++;
> + *generation = rbm.rgd->rd_igen2++;
> }
>
> gfs2_trans_add_meta(rbm.rgd->rd_gl, rbm.rgd->rd_bits[0].bi_bh);
> diff --git a/fs/gfs2/rgrp.h b/fs/gfs2/rgrp.h
> index c0ab33f..71c1a8e 100644
> --- a/fs/gfs2/rgrp.h
> +++ b/fs/gfs2/rgrp.h
> @@ -47,7 +47,7 @@ extern int gfs2_inplace_reserve(struct gfs2_inode *ip,
> extern void gfs2_inplace_release(struct gfs2_inode *ip);
>
> extern int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *n,
> - bool dinode, u64 *generation);
> + bool dinode, u32 *generation);
>
> extern int gfs2_rs_alloc(struct gfs2_inode *ip);
> extern void gfs2_rs_deltree(struct gfs2_blkreserv *rs);
> diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
> index 894fb01..928be2f 100644
> --- a/fs/gfs2/super.c
> +++ b/fs/gfs2/super.c
> @@ -714,7 +714,7 @@ void gfs2_dinode_out(const struct gfs2_inode *ip, void *buf)
>
> str->di_goal_meta = cpu_to_be64(ip->i_goal);
> str->di_goal_data = cpu_to_be64(ip->i_goal);
> - str->di_generation = cpu_to_be64(ip->i_generation);
> + str->di_gen2 = cpu_to_be64(ip->i_inode.i_generation);
Again here.
>
> str->di_flags = cpu_to_be32(ip->i_diskflags);
> str->di_height = cpu_to_be16(ip->i_height);
> diff --git a/include/uapi/linux/gfs2_ondisk.h b/include/uapi/linux/gfs2_ondisk.h
> index 1a763ea..bef244e 100644
> --- a/include/uapi/linux/gfs2_ondisk.h
> +++ b/include/uapi/linux/gfs2_ondisk.h
> @@ -175,7 +175,13 @@ struct gfs2_rgrp_lvb {
> __be32 rl_flags;
> __be32 rl_free;
> __be32 rl_dinodes;
> - __be64 rl_igeneration;
> + union {
> + __be64 rl_igeneration;
> + struct {
> + __be32 rl_igen1;
> + __be32 rl_igen2;
> + };
> + };
> __be32 rl_unlinked;
> __be32 __pad;
> };
> @@ -187,7 +193,13 @@ struct gfs2_rgrp {
> __be32 rg_free;
> __be32 rg_dinodes;
> __be32 __pad;
> - __be64 rg_igeneration;
> + union {
> + __be64 rg_igeneration;
> + struct {
> + __be32 rg_igen1;
> + __be32 rg_igen2;
> + };
> + };
>
> __u8 rg_reserved[80]; /* Several fields from gfs1 now reserved */
> };
> @@ -268,8 +280,13 @@ struct gfs2_dinode {
> */
> __be64 di_goal_meta; /* rgrp to alloc from next */
> __be64 di_goal_data; /* data block goal */
> - __be64 di_generation; /* generation number for NFS */
> -
> + union {
> + __be64 di_generation; /* generation number for NFS */
> + struct {
> + __be32 di_gen1;
> + __be32 di_gen2;
> + };
> + };
> __be32 di_flags; /* GFS2_DIF_... */
> __be32 di_payload_format; /* GFS2_FORMAT_... */
> __u16 __pad1; /* Was ditype in gfs1 */
>
It would be useful to have some description of how the two u32s are
meant to be used now that the u64 is split. Although, I notice that only
the *gen2 fields are being used now, so perhaps the *gen2 fields could
be given a more descriptive name and the *gen1 fields could follow the
__pad convention instead.
I assume there are no endianness problems here as the generation number
is always big-endian on-disk so it will always be found in *gen2.
I'm not too familiar with how the generation number is used so I hope
it's also safe to assume that inode->i_generation will never be bumped
up to 64 bits to avoid some kind of i_generation-Y2K bug :)
Cheers,
Andy
More information about the Cluster-devel
mailing list