From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Whitehouse Date: Wed, 16 Nov 2011 10:46:13 +0000 Subject: [Cluster-devel] [GFS2 PATCH] GFS2: combine gfs2_alloc_block and gfs2_alloc_di In-Reply-To: <09ada988-3cfd-490b-8336-8e0584db75ba@zmail16.collab.prod.int.phx2.redhat.com> References: <09ada988-3cfd-490b-8336-8e0584db75ba@zmail16.collab.prod.int.phx2.redhat.com> Message-ID: <1321440373.2713.23.camel@menhir> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Hi, Both patches are now in the -nmw tree. Thanks, Steve. On Mon, 2011-11-14 at 11:17 -0500, Bob Peterson wrote: > Hi, > > GFS2 functions gfs2_alloc_block and gfs2_alloc_di do basically > the same things, with a few exceptions. This patch combines > the two functions into a slightly more generic gfs2_alloc_block. > Having one centralized block allocation function will reduce > code redundancy and make it easier to implement multi-block > reservations to reduce file fragmentation in the future. > > Regards, > > Bob Peterson > Red Hat File Systems > -- > diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c > index f6be14f..b69235b 100644 > --- a/fs/gfs2/bmap.c > +++ b/fs/gfs2/bmap.c > @@ -133,7 +133,7 @@ int gfs2_unstuff_dinode(struct gfs2_inode *ip, struct page *page) > and write it out to disk */ > > unsigned int n = 1; > - error = gfs2_alloc_block(ip, &block, &n); > + error = gfs2_alloc_block(ip, &block, &n, 0, NULL); > if (error) > goto out_brelse; > if (isdir) { > @@ -503,7 +503,7 @@ static int gfs2_bmap_alloc(struct inode *inode, const sector_t lblock, > do { > int error; > n = blks - alloced; > - error = gfs2_alloc_block(ip, &bn, &n); > + error = gfs2_alloc_block(ip, &bn, &n, 0, NULL); > if (error) > return error; > alloced += n; > diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c > index 946b6f8..ae75319 100644 > --- a/fs/gfs2/dir.c > +++ b/fs/gfs2/dir.c > @@ -823,7 +823,7 @@ static struct gfs2_leaf *new_leaf(struct inode *inode, struct buffer_head **pbh, > struct gfs2_dirent *dent; > struct qstr name = { .name = "", .len = 0, .hash = 0 }; > > - error = gfs2_alloc_block(ip, &bn, &n); > + error = gfs2_alloc_block(ip, &bn, &n, 0, NULL); > if (error) > return NULL; > bh = gfs2_meta_new(ip->i_gl, bn); > diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c > index 377920d..de2668f 100644 > --- a/fs/gfs2/inode.c > +++ b/fs/gfs2/inode.c > @@ -402,7 +402,7 @@ static int alloc_dinode(struct gfs2_inode *dip, u64 *no_addr, u64 *generation) > if (error) > goto out_ipreserv; > > - error = gfs2_alloc_di(dip, no_addr, generation); > + error = gfs2_alloc_block(dip, no_addr, NULL, 1, generation); > > gfs2_trans_end(sdp); > > diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c > index a1a815b..995f4e6 100644 > --- a/fs/gfs2/rgrp.c > +++ b/fs/gfs2/rgrp.c > @@ -1304,19 +1304,24 @@ static void gfs2_rgrp_error(struct gfs2_rgrpd *rgd) > * @ip: the inode to allocate the block for > * @bn: Used to return the starting block number > * @n: requested number of blocks/extent length (value/result) > + * dinode: 1 if we're allocating a dinode, 0 if it's a data block > + * @generation: the generation number of the inode > * > * Returns: 0 or error > */ > > -int gfs2_alloc_block(struct gfs2_inode *ip, u64 *bn, unsigned int *n) > +int gfs2_alloc_block(struct gfs2_inode *ip, u64 *bn, unsigned int *n, > + int dinode, u64 *generation) > { > struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode); > struct buffer_head *dibh; > struct gfs2_alloc *al = ip->i_alloc; > struct gfs2_rgrpd *rgd; > - u32 goal, blk; > - u64 block; > + u32 goal, blk; /* block, within the rgrp scope */ > + u64 block; /* block, within the file system scope */ > + unsigned int extn = 1; > int error; > + unsigned char blk_type = dinode ? GFS2_BLKST_DINODE : GFS2_BLKST_USED; > > /* Only happens if there is a bug in gfs2, return something distinctive > * to ensure that it is noticed. > @@ -1324,14 +1329,16 @@ int gfs2_alloc_block(struct gfs2_inode *ip, u64 *bn, unsigned int *n) > if (al == NULL) > return -ECANCELED; > > + if (n == NULL) > + n = &extn; > rgd = ip->i_rgd; > > - if (rgrp_contains_block(rgd, ip->i_goal)) > + if (!dinode && rgrp_contains_block(rgd, ip->i_goal)) > goal = ip->i_goal - rgd->rd_data0; > else > goal = rgd->rd_last_alloc; > > - blk = rgblk_search(rgd, goal, GFS2_BLKST_FREE, GFS2_BLKST_USED, n); > + blk = rgblk_search(rgd, goal, GFS2_BLKST_FREE, blk_type, n); > > /* Since all blocks are reserved in advance, this shouldn't happen */ > if (blk == BFITNOENT) > @@ -1339,82 +1346,43 @@ int gfs2_alloc_block(struct gfs2_inode *ip, u64 *bn, unsigned int *n) > > rgd->rd_last_alloc = blk; > block = rgd->rd_data0 + blk; > - ip->i_goal = block + *n - 1; > - error = gfs2_meta_inode_buffer(ip, &dibh); > - if (error == 0) { > - struct gfs2_dinode *di = (struct gfs2_dinode *)dibh->b_data; > - gfs2_trans_add_bh(ip->i_gl, dibh, 1); > - di->di_goal_meta = di->di_goal_data = cpu_to_be64(ip->i_goal); > - brelse(dibh); > + if (!dinode) { > + ip->i_goal = block + *n - 1; > + error = gfs2_meta_inode_buffer(ip, &dibh); > + if (error == 0) { > + struct gfs2_dinode *di = > + (struct gfs2_dinode *)dibh->b_data; > + gfs2_trans_add_bh(ip->i_gl, dibh, 1); > + di->di_goal_meta = di->di_goal_data = > + cpu_to_be64(ip->i_goal); > + brelse(dibh); > + } > } > if (rgd->rd_free < *n) > goto rgrp_error; > > rgd->rd_free -= *n; > + if (dinode) { > + rgd->rd_dinodes++; > + *generation = rgd->rd_igeneration++; > + if (*generation == 0) > + *generation = rgd->rd_igeneration++; > + } > > gfs2_trans_add_bh(rgd->rd_gl, rgd->rd_bits[0].bi_bh, 1); > gfs2_rgrp_out(rgd, rgd->rd_bits[0].bi_bh->b_data); > > al->al_alloced += *n; > > - gfs2_statfs_change(sdp, 0, -(s64)*n, 0); > - gfs2_quota_change(ip, *n, ip->i_inode.i_uid, ip->i_inode.i_gid); > + gfs2_statfs_change(sdp, 0, -(s64)*n, dinode ? 1 : 0); > + if (dinode) > + gfs2_trans_add_unrevoke(sdp, block, 1); > + else > + gfs2_quota_change(ip, *n, ip->i_inode.i_uid, > + ip->i_inode.i_gid); > > rgd->rd_free_clone -= *n; > - trace_gfs2_block_alloc(ip, block, *n, GFS2_BLKST_USED); > - *bn = block; > - return 0; > - > -rgrp_error: > - gfs2_rgrp_error(rgd); > - return -EIO; > -} > - > -/** > - * gfs2_alloc_di - Allocate a dinode > - * @dip: the directory that the inode is going in > - * @bn: the block number which is allocated > - * @generation: the generation number of the inode > - * > - * Returns: 0 on success or error > - */ > - > -int gfs2_alloc_di(struct gfs2_inode *dip, u64 *bn, u64 *generation) > -{ > - struct gfs2_sbd *sdp = GFS2_SB(&dip->i_inode); > - struct gfs2_alloc *al = dip->i_alloc; > - struct gfs2_rgrpd *rgd = dip->i_rgd; > - u32 blk; > - u64 block; > - unsigned int n = 1; > - > - blk = rgblk_search(rgd, rgd->rd_last_alloc, > - GFS2_BLKST_FREE, GFS2_BLKST_DINODE, &n); > - > - /* Since all blocks are reserved in advance, this shouldn't happen */ > - if (blk == BFITNOENT) > - goto rgrp_error; > - > - rgd->rd_last_alloc = blk; > - block = rgd->rd_data0 + blk; > - if (rgd->rd_free == 0) > - goto rgrp_error; > - > - rgd->rd_free--; > - rgd->rd_dinodes++; > - *generation = rgd->rd_igeneration++; > - if (*generation == 0) > - *generation = rgd->rd_igeneration++; > - gfs2_trans_add_bh(rgd->rd_gl, rgd->rd_bits[0].bi_bh, 1); > - gfs2_rgrp_out(rgd, rgd->rd_bits[0].bi_bh->b_data); > - > - al->al_alloced++; > - > - gfs2_statfs_change(sdp, 0, -1, +1); > - gfs2_trans_add_unrevoke(sdp, block, 1); > - > - rgd->rd_free_clone--; > - trace_gfs2_block_alloc(dip, block, 1, GFS2_BLKST_DINODE); > + trace_gfs2_block_alloc(ip, block, *n, blk_type); > *bn = block; > return 0; > > diff --git a/fs/gfs2/rgrp.h b/fs/gfs2/rgrp.h > index cf5c501..4cb5608 100644 > --- a/fs/gfs2/rgrp.h > +++ b/fs/gfs2/rgrp.h > @@ -39,8 +39,8 @@ static inline void gfs2_alloc_put(struct gfs2_inode *ip) > extern int gfs2_inplace_reserve(struct gfs2_inode *ip); > extern void gfs2_inplace_release(struct gfs2_inode *ip); > > -extern int gfs2_alloc_block(struct gfs2_inode *ip, u64 *bn, unsigned int *n); > -extern int gfs2_alloc_di(struct gfs2_inode *ip, u64 *bn, u64 *generation); > +extern int gfs2_alloc_block(struct gfs2_inode *ip, u64 *bn, unsigned int *n, > + int dinode, u64 *generation); > > extern void __gfs2_free_blocks(struct gfs2_inode *ip, u64 bstart, u32 blen, int meta); > extern void gfs2_free_meta(struct gfs2_inode *ip, u64 bstart, u32 blen); > diff --git a/fs/gfs2/xattr.c b/fs/gfs2/xattr.c > index a201a1d..e4794a5 100644 > --- a/fs/gfs2/xattr.c > +++ b/fs/gfs2/xattr.c > @@ -610,7 +610,7 @@ static int ea_alloc_blk(struct gfs2_inode *ip, struct buffer_head **bhp) > u64 block; > int error; > > - error = gfs2_alloc_block(ip, &block, &n); > + error = gfs2_alloc_block(ip, &block, &n, 0, NULL); > if (error) > return error; > gfs2_trans_add_unrevoke(sdp, block, 1); > @@ -672,7 +672,7 @@ static int ea_write(struct gfs2_inode *ip, struct gfs2_ea_header *ea, > int mh_size = sizeof(struct gfs2_meta_header); > unsigned int n = 1; > > - error = gfs2_alloc_block(ip, &block, &n); > + error = gfs2_alloc_block(ip, &block, &n, 0, NULL); > if (error) > return error; > gfs2_trans_add_unrevoke(sdp, block, 1); > @@ -992,7 +992,7 @@ static int ea_set_block(struct gfs2_inode *ip, struct gfs2_ea_request *er, > } else { > u64 blk; > unsigned int n = 1; > - error = gfs2_alloc_block(ip, &blk, &n); > + error = gfs2_alloc_block(ip, &blk, &n, 0, NULL); > if (error) > return error; > gfs2_trans_add_unrevoke(sdp, blk, 1); >