----- Original Message ----- > 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 <[email protected]> > > --- > > 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 > Hi Andy,
Thanks for reviewing this. Good catches. I'll post a version 2 patch to replace it shortly. Regards, Bob Peterson Red Hat File Systems
