FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Redhat > Cluster Development

 
 
LinkBack Thread Tools
 
Old 11-16-2011, 09:46 AM
Steven Whitehouse
 
Default GFS2: combine gfs2_alloc_block and gfs2_alloc_di

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);
>
 
Old 11-16-2011, 12:17 PM
Steven Whitehouse
 
Default GFS2: combine gfs2_alloc_block and gfs2_alloc_di

Hi,

As a follow up to this patch, I'd quite like to see rgblk_search split
into two functions. One to do the search and another to actually make
changes to the bitmap when a block is found. That should help
development of further alloc changes on top.

Also, if someone requests allocation of an extent > 1 block with an
inode type, then only the first block should be set to being an inode,
the others should be set to "used" and the various stats should be
updated accordingly. Otherwise it will never be possible to ask for more
than a single block when an inode is being allocated,

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);
>
 
Old 11-16-2011, 01:30 PM
Steven Whitehouse
 
Default GFS2: combine gfs2_alloc_block and gfs2_alloc_di

Hi,

On Wed, 2011-11-16 at 09:18 -0500, Bob Peterson wrote:
> ----- Original Message -----
> | Hi,
> |
> | As a follow up to this patch, I'd quite like to see rgblk_search
> | split
> | into two functions. One to do the search and another to actually make
> | changes to the bitmap when a block is found. That should help
> | development of further alloc changes on top.
> |
> | Also, if someone requests allocation of an extent > 1 block with an
> | inode type, then only the first block should be set to being an
> | inode,
> | the others should be set to "used" and the various stats should be
> | updated accordingly. Otherwise it will never be possible to ask for
> | more
> | than a single block when an inode is being allocated,
> |
> | Steve.
>
> Hi,
>
> Good suggestions. Hopefully I'll post a patch or two soon.
> In the meantime, here's a clean-up for one of the patches.
> I should have done this in the first place.
>
> Regards,
>
> Bob Peterson
> Red Hat File Systems
> --
> commit e5091d2b20b98424cb6924ec499bdf5c5bc5827d
> Author: Bob Peterson <rpeterso@redhat.com>
> Date: Wed Nov 16 08:17:33 2011 -0600
>
> GFS2: Clean up gfs2_block_alloc by passing in block type
>
> This patch simplifies function gfs2_block_alloc by making its
> callers pass in the requested block type rather than a flag to be
> translated as a block type.
>
> diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
> index b69235b..491e1f2 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, 0, NULL);
> + error = gfs2_alloc_block(ip, &block, &n, GFS2_BLKST_USED, 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, 0, NULL);
> + error = gfs2_alloc_block(ip, &bn, &n, GFS2_BLKST_USED, NULL);
> if (error)
> return error;
> alloced += n;
> diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
> index ae75319..c38b32c 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, 0, NULL);
> + error = gfs2_alloc_block(ip, &bn, &n, GFS2_BLKST_USED, 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 de2668f..25b6657 100644
> --- a/fs/gfs2/inode.c
> +++ b/fs/gfs2/inode.c
> @@ -402,7 +402,8 @@ static int alloc_dinode(struct gfs2_inode *dip, u64 *no_addr, u64 *generation)
> if (error)
> goto out_ipreserv;
>
> - error = gfs2_alloc_block(dip, no_addr, NULL, 1, generation);
> + error = gfs2_alloc_block(dip, no_addr, NULL, GFS2_BLKST_DINODE,
> + generation);
>
> gfs2_trans_end(sdp);
>
> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> index 995f4e6..abac4bd 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -1311,7 +1311,7 @@ static void gfs2_rgrp_error(struct gfs2_rgrpd *rgd)
> */
>
> int gfs2_alloc_block(struct gfs2_inode *ip, u64 *bn, unsigned int *n,
> - int dinode, u64 *generation)
> + unsigned char blk_type, u64 *generation)
> {
> struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
> struct buffer_head *dibh;
> @@ -1321,7 +1321,6 @@ int gfs2_alloc_block(struct gfs2_inode *ip, u64 *bn, unsigned int *n,
> 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.
> @@ -1333,7 +1332,7 @@ int gfs2_alloc_block(struct gfs2_inode *ip, u64 *bn, unsigned int *n,
> n = &extn;
> rgd = ip->i_rgd;
>
> - if (!dinode && rgrp_contains_block(rgd, ip->i_goal))
> + if (blk_type == GFS2_BLKST_USED && rgrp_contains_block(rgd, ip->i_goal))
> goal = ip->i_goal - rgd->rd_data0;
> else
> goal = rgd->rd_last_alloc;
> @@ -1346,7 +1345,7 @@ int gfs2_alloc_block(struct gfs2_inode *ip, u64 *bn, unsigned int *n,
>
> rgd->rd_last_alloc = blk;
> block = rgd->rd_data0 + blk;
> - if (!dinode) {
> + if (blk_type == GFS2_BLKST_USED) {
> ip->i_goal = block + *n - 1;
> error = gfs2_meta_inode_buffer(ip, &dibh);
> if (error == 0) {
> @@ -1362,7 +1361,7 @@ int gfs2_alloc_block(struct gfs2_inode *ip, u64 *bn, unsigned int *n,
> goto rgrp_error;
>
> rgd->rd_free -= *n;
> - if (dinode) {
> + if (blk_type == GFS2_BLKST_DINODE) {
> rgd->rd_dinodes++;
> *generation = rgd->rd_igeneration++;
> if (*generation == 0)
> @@ -1374,8 +1373,8 @@ int gfs2_alloc_block(struct gfs2_inode *ip, u64 *bn, unsigned int *n,
>
> al->al_alloced += *n;
>
> - gfs2_statfs_change(sdp, 0, -(s64)*n, dinode ? 1 : 0);
> - if (dinode)
> + gfs2_statfs_change(sdp, 0, -(s64)*n, (blk_type == GFS2_BLKST_DINODE));
It is not a good plan to rely on this to give us 0 and 1 inode counts.
Using the ?: as before would be better. I suspect that if you maintain
separate counts of inodes (which can be 0 or 1 only) and other blocks
(any number) calculated at the start of the function then a number of
the tests you need to do will just become 0 or non-zero on one or other
other of these.

Also, it would be a good plan to always pass the extent size in, even if
the caller is requesting only a single block, then there is no need for
a NULL test,

Steve.


> + if (blk_type == GFS2_BLKST_DINODE)
> gfs2_trans_add_unrevoke(sdp, block, 1);
> else
> gfs2_quota_change(ip, *n, ip->i_inode.i_uid,
> diff --git a/fs/gfs2/rgrp.h b/fs/gfs2/rgrp.h
> index 4cb5608..6a81011 100644
> --- a/fs/gfs2/rgrp.h
> +++ b/fs/gfs2/rgrp.h
> @@ -40,7 +40,7 @@ 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,
> - int dinode, u64 *generation);
> + unsigned char blk_type, 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 e4794a5..f2b77b4 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, 0, NULL);
> + error = gfs2_alloc_block(ip, &block, &n, GFS2_BLKST_USED, NULL);
> if (error)
> return error;
> gfs2_trans_add_unrevoke(sdp, block, 1);
> @@ -672,7 +672,8 @@ 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, 0, NULL);
> + error = gfs2_alloc_block(ip, &block, &n,
> + GFS2_BLKST_USED, NULL);
> if (error)
> return error;
> gfs2_trans_add_unrevoke(sdp, block, 1);
> @@ -992,7 +993,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, 0, NULL);
> + error = gfs2_alloc_block(ip, &blk, &n, GFS2_BLKST_USED, NULL);
> if (error)
> return error;
> gfs2_trans_add_unrevoke(sdp, blk, 1);
 
Old 11-18-2011, 08:54 AM
Steven Whitehouse
 
Default GFS2: combine gfs2_alloc_block and gfs2_alloc_di

Hi,

On Wed, 2011-11-16 at 14:07 -0500, Bob Peterson wrote:
> ----- Original Message -----
> | It is not a good plan to rely on this to give us 0 and 1 inode
> | counts.
> | Using the ?: as before would be better. I suspect that if you
> | maintain
So the new patch is still doing bool to int conversions in various
places, I think.

> | separate counts of inodes (which can be 0 or 1 only) and other blocks
> | (any number) calculated at the start of the function then a number of
> | the tests you need to do will just become 0 or non-zero on one or
> | other
> | other of these.
> |
> | Also, it would be a good plan to always pass the extent size in, even
> | if
> | the caller is requesting only a single block, then there is no need
> | for
> | a NULL test,
> |
> | Steve.
>
> Hi,
>
> Okay, scratch that then. How about something like the following patch?
>
> This patch moves more toward a generic multi-block allocator that
> takes a pointer to the number of data blocks to allocate, plus whether
> or not to allocate a dinode. In theory, it could be called to allocate
> (1) a single dinode block, (2) a group of one or more data blocks, or
> (3) a dinode plus several data blocks.
>
> Regards,
>
> Bob Peterson
> Red Hat File Systems
> --
> fs/gfs2/bmap.c | 4 +-
> fs/gfs2/dir.c | 2 +-
> fs/gfs2/inode.c | 3 +-
> fs/gfs2/rgrp.c | 58 +++++++++++++++++++++++++++++-------------------------
> fs/gfs2/rgrp.h | 4 +-
> fs/gfs2/xattr.c | 6 ++--
> 6 files changed, 41 insertions(+), 36 deletions(-)
>
> diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
> index b69235b..cb74312 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, 0, NULL);
> + error = gfs2_alloc_blocks(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, 0, NULL);
> + error = gfs2_alloc_blocks(ip, &bn, &n, 0, NULL);
> if (error)
> return error;
> alloced += n;
> diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
> index ae75319..f8485da 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, 0, NULL);
> + error = gfs2_alloc_blocks(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 de2668f..3ab192b 100644
> --- a/fs/gfs2/inode.c
> +++ b/fs/gfs2/inode.c
> @@ -389,6 +389,7 @@ static int alloc_dinode(struct gfs2_inode *dip, u64 *no_addr, u64 *generation)
> {
> struct gfs2_sbd *sdp = GFS2_SB(&dip->i_inode);
> int error;
> + int dblocks = 0;
>
> if (gfs2_alloc_get(dip) == NULL)
> return -ENOMEM;
> @@ -402,7 +403,7 @@ static int alloc_dinode(struct gfs2_inode *dip, u64 *no_addr, u64 *generation)
> if (error)
> goto out_ipreserv;
>
> - error = gfs2_alloc_block(dip, no_addr, NULL, 1, generation);
> + error = gfs2_alloc_blocks(dip, no_addr, &dblocks, 1, generation);
>
> gfs2_trans_end(sdp);
>
> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> index 995f4e6..131289b 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -65,8 +65,8 @@ static const char valid_change[16] = {
> };
>
> static u32 rgblk_search(struct gfs2_rgrpd *rgd, u32 goal,
> - unsigned char old_state, unsigned char new_state,
> - unsigned int *n);
> + unsigned char old_state, bool dinode,
> + unsigned int *ndata);
>
> /**
> * gfs2_setbit - Set a bit in the bitmaps
> @@ -921,8 +921,7 @@ static void try_rgrp_unlink(struct gfs2_rgrpd *rgd, u64 *last_unlinked, u64 skip
> while (goal < rgd->rd_data) {
> down_write(&sdp->sd_log_flush_lock);
> n = 1;
> - block = rgblk_search(rgd, goal, GFS2_BLKST_UNLINKED,
> - GFS2_BLKST_UNLINKED, &n);
> + block = rgblk_search(rgd, goal, GFS2_BLKST_UNLINKED, 0, &n);
> up_write(&sdp->sd_log_flush_lock);
> if (block == BFITNOENT)
> break;
> @@ -1115,7 +1114,7 @@ static unsigned char gfs2_get_block_type(struct gfs2_rgrpd *rgd, u64 block)
> * @rgd: the resource group descriptor
> * @goal: the goal block within the RG (start here to search for avail block)
> * @old_state: GFS2_BLKST_XXX the before-allocation state to find
> - * @new_state: GFS2_BLKST_XXX the after-allocation block state
> + * dinode: TRUE if the first block we allocate is for a dinode
> * @n: The extent length
> *
> * Walk rgrp's bitmap to find bits that represent a block in @old_state.
> @@ -1132,8 +1131,7 @@ static unsigned char gfs2_get_block_type(struct gfs2_rgrpd *rgd, u64 block)
> */
>
> static u32 rgblk_search(struct gfs2_rgrpd *rgd, u32 goal,
> - unsigned char old_state, unsigned char new_state,
> - unsigned int *n)
> + unsigned char old_state, bool dinode, unsigned int *n)
> {
> struct gfs2_bitmap *bi = NULL;
> const u32 length = rgd->rd_length;
> @@ -1141,7 +1139,14 @@ static u32 rgblk_search(struct gfs2_rgrpd *rgd, u32 goal,
> unsigned int buf, x;
> const unsigned int elen = *n;
> const u8 *buffer = NULL;
> + unsigned char new_state;
>
> + if (old_state == GFS2_BLKST_UNLINKED)
> + new_state = GFS2_BLKST_UNLINKED;
> + else if (dinode)
> + new_state = GFS2_BLKST_DINODE;
> + else
> + new_state = GFS2_BLKST_USED;

I know this vanishes in the next patch, but the code is a lot clearer
when it states explicitly what is being looked for and what changes are
going to be made.

> *n = 0;
> /* Find bitmap block that contains bits for goal block */
> for (buf = 0; buf < length; buf++) {
> @@ -1192,13 +1197,15 @@ skip:
> if (blk == BFITNOENT)
> return blk;
>
> - *n = 1;
> if (old_state == new_state)
> goto out;
>
> gfs2_trans_add_bh(rgd->rd_gl, bi->bi_bh, 1);
> gfs2_setbit(rgd, bi->bi_bh->b_data, bi->bi_clone, bi->bi_offset,
> bi, blk, new_state);
> + if (new_state == GFS2_BLKST_USED)
> + *n = 1;
> + new_state = GFS2_BLKST_USED; /* for extents, we need data blocks */
If the extent length includes the inode, then we don't need to special
case this.

> goal = blk;
> while (*n < elen) {
> goal++;
> @@ -1300,18 +1307,18 @@ static void gfs2_rgrp_error(struct gfs2_rgrpd *rgd)
> }
>
> /**
> - * gfs2_alloc_block - Allocate one or more blocks
> + * gfs2_alloc_blocks - Allocate one or more blocks of data and/or a dinode
> * @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
> + * @ndata: requested number of data blocks/extent length (value/result)
> + * dinode: 1 if we're allocating a dinode block, else 0
A missing @

> * @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 dinode, u64 *generation)
> +int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *ndata,
> + bool dinode, u64 *generation)
> {
> struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
> struct buffer_head *dibh;
> @@ -1319,9 +1326,7 @@ int gfs2_alloc_block(struct gfs2_inode *ip, u64 *bn, unsigned int *n,
> struct gfs2_rgrpd *rgd;
> 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.
> @@ -1329,8 +1334,6 @@ 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 (!dinode && rgrp_contains_block(rgd, ip->i_goal))
> @@ -1338,7 +1341,7 @@ int gfs2_alloc_block(struct gfs2_inode *ip, u64 *bn, unsigned int *n,
> else
> goal = rgd->rd_last_alloc;
>
> - blk = rgblk_search(rgd, goal, GFS2_BLKST_FREE, blk_type, n);
> + blk = rgblk_search(rgd, goal, GFS2_BLKST_FREE, dinode, ndata);
>
> /* Since all blocks are reserved in advance, this shouldn't happen */
> if (blk == BFITNOENT)
> @@ -1347,7 +1350,7 @@ int gfs2_alloc_block(struct gfs2_inode *ip, u64 *bn, unsigned int *n,
> rgd->rd_last_alloc = blk;
> block = rgd->rd_data0 + blk;
> if (!dinode) {
> - ip->i_goal = block + *n - 1;
> + ip->i_goal = block + *ndata - 1;
> error = gfs2_meta_inode_buffer(ip, &dibh);
> if (error == 0) {
> struct gfs2_dinode *di =
> @@ -1358,11 +1361,12 @@ int gfs2_alloc_block(struct gfs2_inode *ip, u64 *bn, unsigned int *n,
> brelse(dibh);
> }
> }
> - if (rgd->rd_free < *n)
> + if (rgd->rd_free < (*ndata) + dinode)

This (*ndata) + dinode expression appears a lot, can we just have an
extent length variable, so it doesn't have to be calculated separately
each time?

> goto rgrp_error;
>
> - rgd->rd_free -= *n;
> + rgd->rd_free -= *ndata;
> if (dinode) {
> + rgd->rd_free--;
> rgd->rd_dinodes++;
> *generation = rgd->rd_igeneration++;
> if (*generation == 0)
> @@ -1372,17 +1376,17 @@ int gfs2_alloc_block(struct gfs2_inode *ip, u64 *bn, unsigned int *n,
> 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;
> + al->al_alloced += (*ndata) + dinode;
>
> - gfs2_statfs_change(sdp, 0, -(s64)*n, dinode ? 1 : 0);
> + gfs2_statfs_change(sdp, 0, (-(s64)((*ndata) + dinode)), dinode);
> if (dinode)
> gfs2_trans_add_unrevoke(sdp, block, 1);
> - else
> - gfs2_quota_change(ip, *n, ip->i_inode.i_uid,
> + if (*ndata)
> + gfs2_quota_change(ip, *ndata, ip->i_inode.i_uid,
> ip->i_inode.i_gid);
>
Looks like we need to take a look at the inode allocation and see why we
don't do the quota change for that via this function. That can be a
separate patch though.

> - rgd->rd_free_clone -= *n;
> - trace_gfs2_block_alloc(ip, block, *n, blk_type);
> + rgd->rd_free_clone -= (*ndata) + dinode;
> + trace_gfs2_block_alloc(ip, block, *ndata, dinode);
The call to the tracepoint is wrong, the final argument is supposed to
be the type of the block that is being changed. That makes it a bit
awkward in terms of tracing with the combined functions Maybe resolve
this in the splitting rgblk_search patch, where it might be possible to
move the tracing to the gfs2_alloc_extent() function.

Otherwise, it looks good,

Steve.

> *bn = block;
> return 0;
>
> diff --git a/fs/gfs2/rgrp.h b/fs/gfs2/rgrp.h
> index 4cb5608..b3b61b8 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,
> - int dinode, u64 *generation);
> +extern int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *n,
> + bool 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 e4794a5..ef74e159 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, 0, NULL);
> + error = gfs2_alloc_blocks(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, 0, NULL);
> + error = gfs2_alloc_blocks(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, 0, NULL);
> + error = gfs2_alloc_blocks(ip, &blk, &n, 0, NULL);
> if (error)
> return error;
> gfs2_trans_add_unrevoke(sdp, blk, 1);
 
Old 11-18-2011, 08:57 AM
Steven Whitehouse
 
Default GFS2: combine gfs2_alloc_block and gfs2_alloc_di

Hi,

On Wed, 2011-11-16 at 14:59 -0500, Bob Peterson wrote:
> ----- Original Message -----
> | Hi,
> |
> | As a follow up to this patch, I'd quite like to see rgblk_search
> | split
> | into two functions. One to do the search and another to actually make
> | changes to the bitmap when a block is found. That should help
> | development of further alloc changes on top.
>
> Hi,
>
> So something like the patch that follows?
>
> This patch splits function rgblk_search into two functions:
> the finding and the bit setting function, now called gfs2_alloc_extent.
>
Yes, but lets make gfs2_alloc_extent() a totally separate function and
not call it from rgblk_search, that way when looking for unlinked inodes
we don't need to care about gfs2_alloc_extent at all. The only issue is
passing a couple of things from rgblk_search to gfs2_alloc_extent (the
bi (either by index or by pointer) and the current position in the
bitmap.

However my thought was to separate these two completely rather than
leaving one calling the other,

Steve.

> Regards,
>
> Bob Peterson
> Red Hat File Systems
> --
> fs/gfs2/rgrp.c | 69 ++++++++++++++++++++++++++++++++-----------------------
> 1 files changed, 40 insertions(+), 29 deletions(-)
>
> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> index 131289b..f67610e 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -1109,6 +1109,44 @@ static unsigned char gfs2_get_block_type(struct gfs2_rgrpd *rgd, u64 block)
> }
>
> /**
> + * gfs2_alloc_extent - allocate an extent from a given bitmap
> + *
> + * @rgd: the resource group descriptor
> + * @bi: the bitmap within the rgrp
> + * blk: the block within the RG
> + * dinode: TRUE if the first block we allocate is for a dinode
> + * @n: The extent length
> + */
> +static void gfs2_alloc_extent(struct gfs2_rgrpd *rgd, struct gfs2_bitmap *bi,
> + u32 blk, bool dinode, unsigned int *n)
> +{
> + unsigned char new_state = dinode ? GFS2_BLKST_DINODE : GFS2_BLKST_USED;
> + const unsigned int elen = *n;
> + u32 goal;
> + const u8 *buffer = NULL;
> +
> + buffer = bi->bi_bh->b_data + bi->bi_offset;
> + gfs2_trans_add_bh(rgd->rd_gl, bi->bi_bh, 1);
> + gfs2_setbit(rgd, bi->bi_bh->b_data, bi->bi_clone, bi->bi_offset,
> + bi, blk, new_state);
> + if (new_state == GFS2_BLKST_USED)
> + *n = 1;
> + new_state = GFS2_BLKST_USED; /* for extents, we need data blocks */
> + goal = blk;
> + while (*n < elen) {
> + goal++;
> + if (goal >= (bi->bi_len * GFS2_NBBY))
> + break;
> + if (gfs2_testbit(rgd, buffer, bi->bi_len, goal) !=
> + GFS2_BLKST_FREE)
> + break;
> + gfs2_setbit(rgd, bi->bi_bh->b_data, bi->bi_clone, bi->bi_offset,
> + bi, goal, new_state);
> + (*n)++;
> + }
> +}
> +
> +/**
> * rgblk_search - find a block in @old_state, change allocation
> * state to @new_state
> * @rgd: the resource group descriptor
> @@ -1137,16 +1175,8 @@ static u32 rgblk_search(struct gfs2_rgrpd *rgd, u32 goal,
> const u32 length = rgd->rd_length;
> u32 blk = BFITNOENT;
> unsigned int buf, x;
> - const unsigned int elen = *n;
> const u8 *buffer = NULL;
> - unsigned char new_state;
>
> - if (old_state == GFS2_BLKST_UNLINKED)
> - new_state = GFS2_BLKST_UNLINKED;
> - else if (dinode)
> - new_state = GFS2_BLKST_DINODE;
> - else
> - new_state = GFS2_BLKST_USED;
> *n = 0;
> /* Find bitmap block that contains bits for goal block */
> for (buf = 0; buf < length; buf++) {
> @@ -1197,28 +1227,9 @@ skip:
> if (blk == BFITNOENT)
> return blk;
>
> - if (old_state == new_state)
> - goto out;
> + if (old_state != GFS2_BLKST_UNLINKED)
> + gfs2_alloc_extent(rgd, bi, blk, dinode, n);
>
> - gfs2_trans_add_bh(rgd->rd_gl, bi->bi_bh, 1);
> - gfs2_setbit(rgd, bi->bi_bh->b_data, bi->bi_clone, bi->bi_offset,
> - bi, blk, new_state);
> - if (new_state == GFS2_BLKST_USED)
> - *n = 1;
> - new_state = GFS2_BLKST_USED; /* for extents, we need data blocks */
> - goal = blk;
> - while (*n < elen) {
> - goal++;
> - if (goal >= (bi->bi_len * GFS2_NBBY))
> - break;
> - if (gfs2_testbit(rgd, buffer, bi->bi_len, goal) !=
> - GFS2_BLKST_FREE)
> - break;
> - gfs2_setbit(rgd, bi->bi_bh->b_data, bi->bi_clone, bi->bi_offset,
> - bi, goal, new_state);
> - (*n)++;
> - }
> -out:
> return (bi->bi_start * GFS2_NBBY) + blk;
> }
>
 

Thread Tools




All times are GMT. The time now is 06:28 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org