[Cluster-devel] [GFS2 PATCH] GFS2: move gfs2_blkrsv_get calls to function gfs2_inplace_reserve
Steven Whitehouse
swhiteho at redhat.com
Mon Nov 21 11:14:08 UTC 2011
Hi,
It looks like if you were to merge this patch and the one that precedes
it, then the resulting patch would be both smaller and easier to follow,
Steve.
On Fri, 2011-11-18 at 15:17 -0500, Bob Peterson wrote:
> Hi,
>
> Here is the simplification patch I wrote about earlier.
>
> Regards,
>
> Bob Peterson
> Red Hat File Systems
>
> Signed-off-by: Bob Peterson <rpeterso at redhat.com>
> --
> GFS2: move gfs2_blkrsv_get calls to function gfs2_inplace_reserve
>
> This patch moves all the block reservation structure allocations to
> function gfs2_inplace_reserve to simplify the code. It also moves
> the frees to function gfs2_inplace_release.
>
> diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
> index d201342..501e5cb 100644
> --- a/fs/gfs2/aops.c
> +++ b/fs/gfs2/aops.c
> @@ -616,7 +616,6 @@ static int gfs2_write_begin(struct file *file, struct address_space *mapping,
> int alloc_required;
> int error = 0;
> struct gfs2_qadata *qa = NULL;
> - struct gfs2_blkreserv *rs = NULL;
> pgoff_t index = pos >> PAGE_CACHE_SHIFT;
> unsigned from = pos & (PAGE_CACHE_SIZE - 1);
> struct page *page;
> @@ -650,16 +649,9 @@ static int gfs2_write_begin(struct file *file, struct address_space *mapping,
> if (error)
> goto out_alloc_put;
>
> - rs = gfs2_blkrsv_get(ip);
> - if (!rs) {
> - error = -ENOMEM;
> - goto out_qunlock;
> - }
> -
> - rs->rs_requested = data_blocks + ind_blocks;
> - error = gfs2_inplace_reserve(ip);
> + error = gfs2_inplace_reserve(ip, data_blocks + ind_blocks);
> if (error)
> - goto out_blkrsv_put;
> + goto out_qunlock;
> }
>
> rblocks = RES_DINODE + ind_blocks;
> @@ -715,8 +707,6 @@ out_endtrans:
> out_trans_fail:
> if (alloc_required) {
> gfs2_inplace_release(ip);
> -out_blkrsv_put:
> - gfs2_blkrsv_put(ip);
> out_qunlock:
> gfs2_quota_unlock(ip);
> out_alloc_put:
> @@ -858,7 +848,6 @@ static int gfs2_write_end(struct file *file, struct address_space *mapping,
> struct gfs2_inode *m_ip = GFS2_I(sdp->sd_statfs_inode);
> struct buffer_head *dibh;
> struct gfs2_qadata *qa = ip->i_qadata;
> - struct gfs2_blkreserv *rs = ip->i_res;
> unsigned int from = pos & (PAGE_CACHE_SIZE - 1);
> unsigned int to = from + len;
> int ret;
> @@ -890,10 +879,8 @@ static int gfs2_write_end(struct file *file, struct address_space *mapping,
> brelse(dibh);
> failed:
> gfs2_trans_end(sdp);
> - if (rs) {
> + if (ip->i_res)
> gfs2_inplace_release(ip);
> - gfs2_blkrsv_put(ip);
> - }
> if (qa) {
> gfs2_quota_unlock(ip);
> gfs2_qadata_put(ip);
> diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
> index dd6f3d1..14a7040 100644
> --- a/fs/gfs2/bmap.c
> +++ b/fs/gfs2/bmap.c
> @@ -1164,7 +1164,6 @@ static int do_grow(struct inode *inode, u64 size)
> struct gfs2_sbd *sdp = GFS2_SB(inode);
> struct buffer_head *dibh;
> struct gfs2_qadata *qa = NULL;
> - struct gfs2_blkreserv *rs = NULL;
> int error;
>
> if (gfs2_is_stuffed(ip) &&
> @@ -1177,15 +1176,9 @@ static int do_grow(struct inode *inode, u64 size)
> if (error)
> goto do_grow_alloc_put;
>
> - rs = gfs2_blkrsv_get(ip);
> - if (!rs) {
> - error = -ENOMEM;
> - goto do_grow_qunlock;
> - }
> - rs->rs_requested = 1;
> - error = gfs2_inplace_reserve(ip);
> + error = gfs2_inplace_reserve(ip, 1);
> if (error)
> - goto do_grow_rsput;
> + goto do_grow_qunlock;
> }
>
> error = gfs2_trans_begin(sdp, RES_DINODE + RES_STATFS + RES_RG_BIT, 0);
> @@ -1213,8 +1206,6 @@ do_end_trans:
> do_grow_release:
> if (qa) {
> gfs2_inplace_release(ip);
> -do_grow_rsput:
> - gfs2_blkrsv_put(ip);
> do_grow_qunlock:
> gfs2_quota_unlock(ip);
> do_grow_alloc_put:
> diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
> index a0ea143..5ad2681 100644
> --- a/fs/gfs2/file.c
> +++ b/fs/gfs2/file.c
> @@ -366,7 +366,6 @@ static int gfs2_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
> unsigned int data_blocks, ind_blocks, rblocks;
> struct gfs2_holder gh;
> struct gfs2_qadata *qa;
> - struct gfs2_blkreserv *rs;
> loff_t size;
> int ret;
>
> @@ -402,13 +401,7 @@ static int gfs2_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
> if (ret)
> goto out_alloc_put;
> gfs2_write_calc_reserv(ip, PAGE_CACHE_SIZE, &data_blocks, &ind_blocks);
> - rs = gfs2_blkrsv_get(ip);
> - if (!rs) {
> - ret = -ENOMEM;
> - goto out_quota_unlock;
> - }
> - rs->rs_requested = data_blocks + ind_blocks;
> - ret = gfs2_inplace_reserve(ip);
> + ret = gfs2_inplace_reserve(ip, data_blocks + ind_blocks);
> if (ret)
> goto out_rsrv_put;
>
> @@ -452,8 +445,6 @@ out_trans_end:
> out_trans_fail:
> gfs2_inplace_release(ip);
> out_rsrv_put:
> - gfs2_blkrsv_put(ip);
> -out_quota_unlock:
> gfs2_quota_unlock(ip);
> out_alloc_put:
> gfs2_qadata_put(ip);
> @@ -759,7 +750,6 @@ static long gfs2_fallocate(struct file *file, int mode, loff_t offset,
> unsigned int data_blocks = 0, ind_blocks = 0, rblocks;
> loff_t bytes, max_bytes;
> struct gfs2_qadata *qa;
> - struct gfs2_blkreserv *rs;
> int error;
> loff_t bsize_mask = ~((loff_t)sdp->sd_sb.sb_bsize - 1);
> loff_t next = (offset + len - 1) >> sdp->sd_sb.sb_bsize_shift;
> @@ -801,16 +791,10 @@ static long gfs2_fallocate(struct file *file, int mode, loff_t offset,
> if (error)
> goto out_alloc_put;
>
> - rs = gfs2_blkrsv_get(ip);
> - if (!rs) {
> - error = -ENOMEM;
> - goto out_qunlock;
> - }
> retry:
> gfs2_write_calc_reserv(ip, bytes, &data_blocks, &ind_blocks);
>
> - rs->rs_requested = data_blocks + ind_blocks;
> - error = gfs2_inplace_reserve(ip);
> + error = gfs2_inplace_reserve(ip, data_blocks + ind_blocks);
> if (error) {
> if (error == -ENOSPC && bytes > sdp->sd_sb.sb_bsize) {
> bytes >>= 1;
> @@ -819,12 +803,11 @@ retry:
> bytes = sdp->sd_sb.sb_bsize;
> goto retry;
> }
> - goto out_rsrv_put;
> + goto out_qunlock;
> }
> max_bytes = bytes;
> calc_max_reserv(ip, (len > max_chunk_size)? max_chunk_size: len,
> &max_bytes, &data_blocks, &ind_blocks);
> - rs->rs_requested = data_blocks + ind_blocks;
>
> rblocks = RES_DINODE + ind_blocks + RES_STATFS + RES_QUOTA +
> RES_RG_HDR + gfs2_rg_blocks(ip);
> @@ -852,8 +835,6 @@ retry:
>
> out_trans_fail:
> gfs2_inplace_release(ip);
> -out_rsrv_put:
> - gfs2_blkrsv_put(ip);
> out_qunlock:
> gfs2_quota_unlock(ip);
> out_alloc_put:
> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> index 81435fa..02df0b8 100644
> --- a/fs/gfs2/inode.c
> +++ b/fs/gfs2/inode.c
> @@ -394,11 +394,8 @@ static int alloc_dinode(struct gfs2_inode *dip, u64 *no_addr, u64 *generation)
> error = gfs2_rindex_update(sdp);
> if (error)
> fs_warn(sdp, "rindex update returns %d\n", error);
> - if (gfs2_blkrsv_get(dip) == NULL)
> - return -ENOMEM;
>
> - dip->i_res->rs_requested = RES_DINODE;
> - error = gfs2_inplace_reserve(dip);
> + error = gfs2_inplace_reserve(dip, RES_DINODE);
> if (error)
> goto out;
>
> @@ -413,7 +410,6 @@ static int alloc_dinode(struct gfs2_inode *dip, u64 *no_addr, u64 *generation)
> out_ipreserv:
> gfs2_inplace_release(dip);
> out:
> - gfs2_blkrsv_put(dip);
> return error;
> }
>
> @@ -560,7 +556,6 @@ static int link_dinode(struct gfs2_inode *dip, const struct qstr *name,
> {
> struct gfs2_sbd *sdp = GFS2_SB(&dip->i_inode);
> struct gfs2_qadata *qa;
> - struct gfs2_blkreserv *rs = NULL;
> int alloc_required;
> struct buffer_head *dibh;
> int error;
> @@ -581,16 +576,9 @@ static int link_dinode(struct gfs2_inode *dip, const struct qstr *name,
> if (error)
> goto fail_quota_locks;
>
> - rs = gfs2_blkrsv_get(dip);
> - if (!rs) {
> - error = -ENOMEM;
> - goto fail_quota_locks;
> - }
> - rs->rs_requested = sdp->sd_max_dirres;
> -
> - error = gfs2_inplace_reserve(dip);
> + error = gfs2_inplace_reserve(dip, sdp->sd_max_dirres);
> if (error)
> - goto fail_rsrv_put;
> + goto fail_quota_locks;
>
> error = gfs2_trans_begin(sdp, sdp->sd_max_dirres +
> dip->i_rgd->rd_length +
> @@ -625,10 +613,6 @@ fail_end_trans:
> fail_ipreserv:
> gfs2_inplace_release(dip);
>
> -fail_rsrv_put:
> - if (alloc_required)
> - gfs2_blkrsv_put(dip);
> -
> fail_quota_locks:
> gfs2_quota_unlock(dip);
>
> @@ -744,10 +728,8 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
> gfs2_trans_end(sdp);
> /* Check if we reserved space in the rgrp. Function link_dinode may
> not, depending on whether alloc is required. */
> - if (dip->i_res) {
> + if (dip->i_res)
> gfs2_inplace_release(dip);
> - gfs2_blkrsv_put(dip);
> - }
> gfs2_quota_unlock(dip);
> gfs2_qadata_put(dip);
> mark_inode_dirty(inode);
> @@ -895,7 +877,6 @@ static int gfs2_link(struct dentry *old_dentry, struct inode *dir,
>
> if (alloc_required) {
> struct gfs2_qadata *qa = gfs2_qadata_get(dip);
> - struct gfs2_blkreserv *rs = NULL;
>
> if (!qa) {
> error = -ENOMEM;
> @@ -906,16 +887,9 @@ static int gfs2_link(struct dentry *old_dentry, struct inode *dir,
> if (error)
> goto out_alloc;
>
> - rs = gfs2_blkrsv_get(dip);
> - if (!rs) {
> - error = -ENOMEM;
> - goto out_gunlock_q;
> - }
> - rs->rs_requested = sdp->sd_max_dirres;
> -
> - error = gfs2_inplace_reserve(dip);
> + error = gfs2_inplace_reserve(dip, sdp->sd_max_dirres);
> if (error)
> - goto out_rsrv_put;
> + goto out_qunlock;
>
> error = gfs2_trans_begin(sdp, sdp->sd_max_dirres +
> gfs2_rg_blocks(dip) +
> @@ -951,10 +925,7 @@ out_end_trans:
> out_ipres:
> if (alloc_required)
> gfs2_inplace_release(dip);
> -out_rsrv_put:
> - if (alloc_required)
> - gfs2_blkrsv_put(dip);
> -out_gunlock_q:
> +out_qunlock:
> if (alloc_required)
> gfs2_quota_unlock(dip);
> out_alloc:
> @@ -1383,7 +1354,7 @@ static int gfs2_rename(struct inode *odir, struct dentry *odentry,
>
> if (alloc_required) {
> struct gfs2_qadata *qa = gfs2_qadata_get(ndip);
> - struct gfs2_blkreserv *rs;
> +
> if (!qa) {
> error = -ENOMEM;
> goto out_gunlock;
> @@ -1393,16 +1364,9 @@ static int gfs2_rename(struct inode *odir, struct dentry *odentry,
> if (error)
> goto out_alloc;
>
> - rs = gfs2_blkrsv_get(ndip);
> - if (!rs) {
> - error = -ENOMEM;
> - goto out_gunlock_q;
> - }
> - rs->rs_requested = sdp->sd_max_dirres;
> -
> - error = gfs2_inplace_reserve(ndip);
> + error = gfs2_inplace_reserve(ndip, sdp->sd_max_dirres);
> if (error)
> - goto out_rsrv_put;
> + goto out_qunlock;
>
> error = gfs2_trans_begin(sdp, sdp->sd_max_dirres +
> gfs2_rg_blocks(ndip) +
> @@ -1456,10 +1420,7 @@ out_end_trans:
> out_ipreserv:
> if (alloc_required)
> gfs2_inplace_release(ndip);
> -out_rsrv_put:
> - if (alloc_required)
> - gfs2_blkrsv_put(ndip);
> -out_gunlock_q:
> +out_qunlock:
> if (alloc_required)
> gfs2_quota_unlock(ndip);
> out_alloc:
> diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> index d538f4d..98a01db 100644
> --- a/fs/gfs2/quota.c
> +++ b/fs/gfs2/quota.c
> @@ -762,7 +762,6 @@ static int do_sync(unsigned int num_qd, struct gfs2_quota_data **qda)
> struct gfs2_quota_data *qd;
> loff_t offset;
> unsigned int nalloc = 0, blocks;
> - struct gfs2_blkreserv *rs = NULL;
> int error;
>
> gfs2_write_calc_reserv(ip, sizeof(struct gfs2_quota),
> @@ -792,26 +791,19 @@ static int do_sync(unsigned int num_qd, struct gfs2_quota_data **qda)
> nalloc++;
> }
>
> - rs = gfs2_blkrsv_get(ip);
> - if (!rs) {
> - error = -ENOMEM;
> - goto out_gunlock;
> - }
> /*
> * 1 blk for unstuffing inode if stuffed. We add this extra
> * block to the reservation unconditionally. If the inode
> * doesn't need unstuffing, the block will be released to the
> * rgrp since it won't be allocated during the transaction
> */
> - rs->rs_requested = 1;
> /* +3 in the end for unstuffing block, inode size update block
> * and another block in case quota straddles page boundary and
> * two blocks need to be updated instead of 1 */
> blocks = num_qd * data_blocks + RES_DINODE + num_qd + 3;
>
> - if (nalloc)
> - rs->rs_requested += nalloc * (data_blocks + ind_blocks);
> - error = gfs2_inplace_reserve(ip);
> + error = gfs2_inplace_reserve(ip, 1 +
> + (nalloc * (data_blocks + ind_blocks)));
> if (error)
> goto out_alloc;
>
> @@ -840,8 +832,6 @@ out_end_trans:
> out_ipres:
> gfs2_inplace_release(ip);
> out_alloc:
> - gfs2_blkrsv_put(ip);
> -out_gunlock:
> gfs2_glock_dq_uninit(&i_gh);
> out:
> while (qx--)
> @@ -1529,7 +1519,6 @@ static int gfs2_set_dqblk(struct super_block *sb, int type, qid_t id,
> unsigned int data_blocks, ind_blocks;
> unsigned int blocks = 0;
> int alloc_required;
> - struct gfs2_blkreserv *rs;
> loff_t offset;
> int error;
>
> @@ -1594,15 +1583,12 @@ static int gfs2_set_dqblk(struct super_block *sb, int type, qid_t id,
> if (gfs2_is_stuffed(ip))
> alloc_required = 1;
> if (alloc_required) {
> - rs = gfs2_blkrsv_get(ip);
> - if (rs == NULL)
> - goto out_i;
> gfs2_write_calc_reserv(ip, sizeof(struct gfs2_quota),
> &data_blocks, &ind_blocks);
> - blocks = rs->rs_requested = 1 + data_blocks + ind_blocks;
> - error = gfs2_inplace_reserve(ip);
> + blocks = 1 + data_blocks + ind_blocks;
> + error = gfs2_inplace_reserve(ip, blocks);
> if (error)
> - goto out_alloc;
> + goto out_i;
> blocks += gfs2_rg_blocks(ip);
> }
>
> @@ -1617,11 +1603,8 @@ static int gfs2_set_dqblk(struct super_block *sb, int type, qid_t id,
>
> gfs2_trans_end(sdp);
> out_release:
> - if (alloc_required) {
> + if (alloc_required)
> gfs2_inplace_release(ip);
> -out_alloc:
> - gfs2_blkrsv_put(ip);
> - }
> out_i:
> gfs2_glock_dq_uninit(&i_gh);
> out_q:
> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> index a51f3cd..72bea39 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -885,7 +885,7 @@ struct gfs2_qadata *gfs2_qadata_get(struct gfs2_inode *ip)
> * Returns: the struct gfs2_qadata
> */
>
> -struct gfs2_blkreserv *gfs2_blkrsv_get(struct gfs2_inode *ip)
> +static struct gfs2_blkreserv *gfs2_blkrsv_get(struct gfs2_inode *ip)
> {
> BUG_ON(ip->i_res != NULL);
> ip->i_res = kzalloc(sizeof(struct gfs2_blkreserv), GFP_NOFS);
> @@ -1039,6 +1039,13 @@ static int get_local_rgrp(struct gfs2_inode *ip, u64 *last_unlinked)
> return -ENOSPC;
> }
>
> +static void gfs2_blkrsv_put(struct gfs2_inode *ip)
> +{
> + BUG_ON(ip->i_res == NULL);
> + kfree(ip->i_res);
> + ip->i_res = NULL;
> +}
> +
> /**
> * gfs2_inplace_reserve - Reserve space in the filesystem
> * @ip: the inode to reserve space for
> @@ -1046,16 +1053,23 @@ static int get_local_rgrp(struct gfs2_inode *ip, u64 *last_unlinked)
> * Returns: errno
> */
>
> -int gfs2_inplace_reserve(struct gfs2_inode *ip)
> +int gfs2_inplace_reserve(struct gfs2_inode *ip, u32 requested)
> {
> struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
> - struct gfs2_blkreserv *rs = ip->i_res;
> + struct gfs2_blkreserv *rs;
> int error = 0;
> u64 last_unlinked = NO_BLOCK;
> int tries = 0;
>
> - if (gfs2_assert_warn(sdp, rs->rs_requested))
> - return -EINVAL;
> + rs = gfs2_blkrsv_get(ip);
> + if (!rs)
> + return -ENOMEM;
> +
> + rs->rs_requested = requested;
> + if (gfs2_assert_warn(sdp, requested)) {
> + error = -EINVAL;
> + goto out;
> + }
>
> do {
> error = get_local_rgrp(ip, &last_unlinked);
> @@ -1072,6 +1086,9 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip)
> gfs2_log_flush(sdp, NULL);
> } while (tries++ < 3);
>
> +out:
> + if (error)
> + gfs2_blkrsv_put(ip);
> return error;
> }
>
> @@ -1086,6 +1103,7 @@ void gfs2_inplace_release(struct gfs2_inode *ip)
> {
> struct gfs2_blkreserv *rs = ip->i_res;
>
> + gfs2_blkrsv_put(ip);
> if (rs->rs_rgd_gh.gh_gl)
> gfs2_glock_dq_uninit(&rs->rs_rgd_gh);
> }
> diff --git a/fs/gfs2/rgrp.h b/fs/gfs2/rgrp.h
> index 57002a7..7ad2099 100644
> --- a/fs/gfs2/rgrp.h
> +++ b/fs/gfs2/rgrp.h
> @@ -38,15 +38,7 @@ static inline void gfs2_qadata_put(struct gfs2_inode *ip)
> ip->i_qadata = NULL;
> }
>
> -extern struct gfs2_blkreserv *gfs2_blkrsv_get(struct gfs2_inode *ip);
> -static inline void gfs2_blkrsv_put(struct gfs2_inode *ip)
> -{
> - BUG_ON(ip->i_res == NULL);
> - kfree(ip->i_res);
> - ip->i_res = NULL;
> -}
> -
> -extern int gfs2_inplace_reserve(struct gfs2_inode *ip);
> +extern int gfs2_inplace_reserve(struct gfs2_inode *ip, u32 requested);
> extern void gfs2_inplace_release(struct gfs2_inode *ip);
>
> extern int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *n,
> diff --git a/fs/gfs2/xattr.c b/fs/gfs2/xattr.c
> index b3642df..4d017b4 100644
> --- a/fs/gfs2/xattr.c
> +++ b/fs/gfs2/xattr.c
> @@ -710,7 +710,6 @@ static int ea_alloc_skeleton(struct gfs2_inode *ip, struct gfs2_ea_request *er,
> ea_skeleton_call_t skeleton_call, void *private)
> {
> struct gfs2_qadata *qa;
> - struct gfs2_blkreserv *rs;
> struct buffer_head *dibh;
> int error;
>
> @@ -722,16 +721,9 @@ static int ea_alloc_skeleton(struct gfs2_inode *ip, struct gfs2_ea_request *er,
> if (error)
> goto out;
>
> - rs = gfs2_blkrsv_get(ip);
> - if (!rs) {
> - error = -ENOMEM;
> - goto out_gunlock_q;
> - }
> - rs->rs_requested = blks;
> -
> - error = gfs2_inplace_reserve(ip);
> + error = gfs2_inplace_reserve(ip, blks);
> if (error)
> - goto out_rsrv_put;
> + goto out_qunlock;
>
> error = gfs2_trans_begin(GFS2_SB(&ip->i_inode),
> blks + gfs2_rg_blocks(ip) +
> @@ -755,9 +747,7 @@ out_end_trans:
> gfs2_trans_end(GFS2_SB(&ip->i_inode));
> out_ipres:
> gfs2_inplace_release(ip);
> -out_rsrv_put:
> - gfs2_blkrsv_put(ip);
> -out_gunlock_q:
> +out_qunlock:
> gfs2_quota_unlock(ip);
> out:
> gfs2_qadata_put(ip);
>
More information about the Cluster-devel
mailing list