Attachment 587836 Details for Bug 236099 – GFS2: combine functions alloc_dinode and make_dinode

[patch] GFS2: combine functions alloc_dinode and make_dinode

bz236099.experimental.patch (text/plain), 10.16 KB, created by Robert Peterson on 2012-05-30 19:45:51 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Robert Peterson

Created: 2012-05-30 19:45:51 UTC

Size: 10.16 KB

patch

obsolete

>commit a2998cb45de854ff3cfb6f8f78a4779ad635bf42
>Author: Bob Peterson <rpeterso@redhat.com>
>Date:   Fri May 25 14:39:14 2012 -0500
>
>    GFS2: combine functions alloc_dinode and make_dinode
>    
>    This patch combines functions alloc_dinode and make_dinode so that
>    their functions run under a single transaction. So effectively,
>    we have reduced the gfs2_createi by one transaction.
>
>diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
>index fad3ec4..3741c64 100644
>--- a/fs/gfs2/bmap.c
>+++ b/fs/gfs2/bmap.c
>@@ -135,7 +135,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_blocks(ip, &block, &n, 0, NULL);
>+		error = gfs2_alloc_blocks(ip, &block, &n, 0, NULL, NULL);
> 		if (error)
> 			goto out_brelse;
> 		if (isdir) {
>@@ -482,7 +482,7 @@ static int gfs2_bmap_alloc(struct inode *inode, const sector_t lblock,
> 	do {
> 		int error;
> 		n = blks - alloced;
>-		error = gfs2_alloc_blocks(ip, &bn, &n, 0, NULL);
>+		error = gfs2_alloc_blocks(ip, &bn, &n, 0, NULL, NULL);
> 		if (error)
> 			return error;
> 		alloced += n;
>diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
>index bcd1505..dec276f 100644
>--- a/fs/gfs2/dir.c
>+++ b/fs/gfs2/dir.c
>@@ -868,7 +868,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_blocks(ip, &bn, &n, 0, NULL);
>+	error = gfs2_alloc_blocks(ip, &bn, &n, 0, NULL, 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 923ab17..da1c208 100644
>--- a/fs/gfs2/inode.c
>+++ b/fs/gfs2/inode.c
>@@ -542,30 +542,6 @@ static void munge_mode_uid_gid(struct gfs2_inode *dip, unsigned int *mode,
> 		*gid = current_fsgid();
> }
> 
>-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 = 1;
>-
>-	error = gfs2_inplace_reserve(dip, RES_DINODE);
>-	if (error)
>-		goto out;
>-
>-	error = gfs2_trans_begin(sdp, RES_RG_BIT + RES_STATFS, 0);
>-	if (error)
>-		goto out_ipreserv;
>-
>-	error = gfs2_alloc_blocks(dip, no_addr, &dblocks, 1, generation);
>-
>-	gfs2_trans_end(sdp);
>-
>-out_ipreserv:
>-	gfs2_inplace_release(dip);
>-out:
>-	return error;
>-}
>-
> /**
>  * init_dinode - Fill in a new dinode structure
>  * @dip: the directory this inode is being created in
>@@ -636,15 +612,16 @@ static void init_dinode(struct gfs2_inode *dip, struct gfs2_glock *gl,
> 	*bhp = dibh;
> }
> 
>-static int make_dinode(struct gfs2_inode *dip, struct gfs2_glock *gl,
>-		       unsigned int mode, const struct gfs2_inum_host *inum,
>-		       const u64 *generation, dev_t dev, struct buffer_head **bhp)
>+static int alloc_make_dinode(struct gfs2_holder *gh, struct gfs2_inode *dip,
>+			     unsigned int mode,
>+			     struct gfs2_inum_host *inum, dev_t dev,
>+			     struct buffer_head **bhp)
> {
> 	struct gfs2_sbd *sdp = GFS2_SB(&dip->i_inode);
> 	unsigned int uid, gid;
> 	int error;
>+	int dblocks = 1;
> 
>-	munge_mode_uid_gid(dip, &mode, &uid, &gid);
> 	error = gfs2_rindex_update(sdp);
> 	if (error)
> 		return error;
>@@ -657,13 +634,27 @@ static int make_dinode(struct gfs2_inode *dip, struct gfs2_glock *gl,
> 	if (error)
> 		goto out_quota;
> 
>-	error = gfs2_trans_begin(sdp, RES_DINODE + RES_QUOTA, 0);
>+	munge_mode_uid_gid(dip, &mode, &uid, &gid);
>+	error = gfs2_inplace_reserve(dip, RES_DINODE);
> 	if (error)
> 		goto out_quota;
> 
>-	init_dinode(dip, gl, inum, mode, uid, gid, generation, dev, bhp);
>-	gfs2_quota_change(dip, +1, uid, gid);
>-	gfs2_trans_end(sdp);
>+	/* This call to gfs2_alloc_blocks should lock the new dinode glock and
>+	   start a transaction for us, unless there's an error. */
>+	error = gfs2_alloc_blocks(dip, &inum->no_addr, &dblocks, 1,
>+				  &inum->no_formal_ino, gh);
>+	if (!error) {
>+		init_dinode(dip, gh->gh_gl, inum, mode, uid, gid,
>+			    &inum->no_formal_ino, dev, bhp);
>+		gfs2_quota_change(dip, +1, uid, gid);
>+	}
>+
>+out_trans:
>+	if (!error)
>+		gfs2_trans_end(sdp);
>+
>+out_release:
>+	gfs2_inplace_release(dip);
> 
> out_quota:
> 	gfs2_quota_unlock(dip);
>@@ -782,10 +773,8 @@ struct inode *gfs2_createi(struct gfs2_holder *ghs, const struct qstr *name,
> 	struct inode *inode = NULL;
> 	struct gfs2_inode *dip = ghs->gh_gl->gl_object;
> 	struct inode *dir = &dip->i_inode;
>-	struct gfs2_sbd *sdp = GFS2_SB(&dip->i_inode);
> 	struct gfs2_inum_host inum = { .no_addr = 0, .no_formal_ino = 0 };
> 	int error;
>-	u64 generation;
> 	struct buffer_head *bh = NULL;
> 
> 	if (!name->len || name->len > GFS2_FNAMESIZE)
>@@ -804,17 +793,7 @@ struct inode *gfs2_createi(struct gfs2_holder *ghs, const struct qstr *name,
> 	if (error)
> 		goto fail_gunlock;
> 
>-	error = alloc_dinode(dip, &inum.no_addr, &generation);
>-	if (error)
>-		goto fail_gunlock;
>-	inum.no_formal_ino = generation;
>-
>-	error = gfs2_glock_nq_num(sdp, inum.no_addr, &gfs2_inode_glops,
>-				  LM_ST_EXCLUSIVE, GL_SKIP, ghs + 1);
>-	if (error)
>-		goto fail_gunlock;
>-
>-	error = make_dinode(dip, ghs[1].gh_gl, mode, &inum, &generation, dev, &bh);
>+	error = alloc_make_dinode(&ghs[1], dip, mode, &inum, dev, &bh);
> 	if (error)
> 		goto fail_gunlock2;
> 
>diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
>index e470722..cf8e360 100644
>--- a/fs/gfs2/rgrp.c
>+++ b/fs/gfs2/rgrp.c
>@@ -1987,15 +1987,13 @@ static void gfs2_rgrp_error(struct gfs2_rgrpd *rgd)
> /**
>  * claim_reserved_blks - Claim previously reserved blocks
>  * @ip: the inode that's claiming the reservation
>- * @dinode: 1 if this block is a dinode block, otherwise data block
>  * @nblocks: desired extent length
>  *
>  * Lay claim to previously allocated block reservation blocks.
>  * Returns: Starting block number of the blocks claimed.
>  * Sets *nblocks to the actual extent length allocated.
>  */
>-static u64 claim_reserved_blks(struct gfs2_inode *ip, bool dinode,
>-			       unsigned int *nblocks)
>+static u64 claim_reserved_blks(struct gfs2_inode *ip, unsigned int *nblocks)
> {
> 	struct gfs2_blkreserv *rs = ip->i_res;
> 	struct gfs2_rgrpd *rgd = rs->rs_rgd;
>@@ -2042,16 +2040,13 @@ static u64 claim_reserved_blks(struct gfs2_inode *ip, bool dinode,
> 		}
> */
> 		/* Make sure the bitmap hasn't changed */
>-		gfs2_setbit(rgd, bi->bi_clone, bi, biblk,
>-			    dinode ? GFS2_BLKST_DINODE : GFS2_BLKST_USED);
>+		gfs2_setbit(rgd, bi->bi_clone, bi, biblk, GFS2_BLKST_USED);
> 
> 		BUG_ON(!rgd->rd_reserved);
> 		rgd->rd_reserved--;
> 		sanity_check(rgd, true);
>-		if (!start_block) {
>+		if (!start_block)
> 			start_block = fsblock;
>-			dinode = false;
>-		}
> 		trace_gfs2_rs(ip, rs, fsblock, TRACE_RS_CLAIM);
> 	}
> 
>@@ -2085,7 +2080,7 @@ static u64 claim_reserved_blks(struct gfs2_inode *ip, bool dinode,
>  */
> 
> int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks,
>-		      bool dinode, u64 *generation)
>+		      bool dinode, u64 *generation, struct gfs2_holder *gh)
> {
> 	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
> 	struct buffer_head *dibh;
>@@ -2106,8 +2101,9 @@ int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks,
> 	   next free block from it. */
> 	if (gfs2_rs_active(ip)) {
> 		BUG_ON(!ip->i_res->rs_free);
>+		BUG_ON(dinode);
> 		rgd = ip->i_res->rs_rgd;
>-		block = claim_reserved_blks(ip, dinode, nblocks);
>+		block = claim_reserved_blks(ip, nblocks);
> 	} else {
> 		rgd = ip->i_rgd;
> 
>@@ -2128,6 +2124,27 @@ int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks,
> 			goto rgrp_error;
> 		}
> 
>+		/* If we're allocating a dinode, we have to lock the new
>+		   dinode's glock and start a transaction after we find the
>+		   block, but before we allocate it. That's because function
>+		   gfs2_alloc_extent needs to operate within a transaction,
>+		   but otherwise we would need two transactions: one to write
>+		   the rgrp bitmap, and one to write the new inode. This
>+		   method preserves the lock ordering within a single one.
>+		   Ordinarily, we should lock the dinode glock BEFORE locking
>+		   the rgrp glock, but in this case, the dinode doesn't exist
>+		   so there should be no conflict. */
>+		if (dinode) {
>+			block = rgd->rd_data0 + gfs2_bi2rgd_blk(bi, blk);
>+			error = gfs2_glock_nq_num(sdp, block, &gfs2_inode_glops,
>+						  LM_ST_EXCLUSIVE, GL_SKIP, gh);
>+			if (error)
>+				goto rgrp_error;
>+			error = gfs2_trans_begin(sdp, RES_RG_BIT + RES_STATFS +
>+						 RES_DINODE + RES_QUOTA, 0);
>+			if (error)
>+				goto dinode_error;
>+		}
> 		block = gfs2_alloc_extent(rgd, bi, blk, dinode, nblocks);
> 	}
> 	ndata = *nblocks;
>@@ -2180,6 +2197,8 @@ int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks,
> 	*bn = block;
> 	return 0;
> 
>+dinode_error:
>+	gfs2_glock_dq(gh);
> rgrp_error:
> 	gfs2_rgrp_error(rgd);
> 	return -EIO;
>diff --git a/fs/gfs2/rgrp.h b/fs/gfs2/rgrp.h
>index 1b4a434..9e754d2 100644
>--- a/fs/gfs2/rgrp.h
>+++ b/fs/gfs2/rgrp.h
>@@ -43,7 +43,8 @@ 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,
>-			     bool dinode, u64 *generation);
>+			     bool dinode, u64 *generation,
>+			     struct gfs2_holder *gh);
> 
> extern int gfs2_rs_alloc(struct gfs2_inode *ip);
> extern void gfs2_rs_treedel(struct gfs2_inode *ip, bool lock);
>diff --git a/fs/gfs2/xattr.c b/fs/gfs2/xattr.c
>index af28ec8..7adfe06 100644
>--- a/fs/gfs2/xattr.c
>+++ b/fs/gfs2/xattr.c
>@@ -612,7 +612,7 @@ static int ea_alloc_blk(struct gfs2_inode *ip, struct buffer_head **bhp)
> 	u64 block;
> 	int error;
> 
>-	error = gfs2_alloc_blocks(ip, &block, &n, 0, NULL);
>+	error = gfs2_alloc_blocks(ip, &block, &n, 0, NULL, NULL);
> 	if (error)
> 		return error;
> 	gfs2_trans_add_unrevoke(sdp, block, 1);
>@@ -674,7 +674,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_blocks(ip, &block, &n, 0, NULL);
>+			error = gfs2_alloc_blocks(ip, &block, &n, 0, NULL, NULL);
> 			if (error)
> 				return error;
> 			gfs2_trans_add_unrevoke(sdp, block, 1);
>@@ -989,7 +989,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_blocks(ip, &blk, &n, 0, NULL);
>+		error = gfs2_alloc_blocks(ip, &blk, &n, 0, NULL, NULL);
> 		if (error)
> 			return error;
> 		gfs2_trans_add_unrevoke(sdp, blk, 1);

Actions: View | Diff

Attachments on bug 236099: 587836