diff mbox

[RFC,1/3] ext4: Add blocks added during resize to bitmap

Message ID 1224830067-22963-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com
State Superseded, archived
Headers show

Commit Message

Aneesh Kumar K.V Oct. 24, 2008, 6:34 a.m. UTC 
With this change new blocks added during resize
are marked as free in the block bitmap and the
group is flagged with EXT4_GROUP_INFO_NEED_INIT_BIT
flag. This make sure when mballoc tries to allocate
blocks from the new group we would reload the
buddy information using the bitmap present in the disk.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/ext4/balloc.c |   52 +++++++++++++++-------------------------------------
 fs/ext4/ext4.h   |    2 +-
 fs/ext4/resize.c |   10 ++--------
 3 files changed, 18 insertions(+), 46 deletions(-)

Comments

Theodore Ts'o Nov. 3, 2008, 1:19 a.m. UTC | #1 
On Fri, Oct 24, 2008 at 12:04:25PM +0530, Aneesh Kumar K.V wrote:
> With this change new blocks added during resize
> are marked as free in the block bitmap and the
> group is flagged with EXT4_GROUP_INFO_NEED_INIT_BIT
> flag. This make sure when mballoc tries to allocate
> blocks from the new group we would reload the
> buddy information using the bitmap present in the disk.

Stupid question --- instead of reworking ext4_free_blocks_sb() and
renaming it be to called ext4_add_groupblocks(), and then forcing
mballoc() to reinitialize the block group, why not just have the
resize code call ext4_mb_free_blocks()?   

Also I had to make some minor changes to patches 2 and 3 to make them
apply against the latest tree (checked into the patch queue tree).  I
also changed calls to ext4_error() to use __func__ so the function
name in used in ext4_add_groupblocks() was correct in patch #1 --- but
I suspect that once you change resize to use ext4_mb_free_blocks(),
ext4_add_groupblocks()/ext4_free_blocks_sb() can go away entirely.

							- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index b9821be..b67f1ff 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -20,6 +20,7 @@ 
 #include "ext4.h"
 #include "ext4_jbd2.h"
 #include "group.h"
+#include "mballoc.h"
 
 /*
  * balloc.c contains the blocks allocation and deallocation routines
@@ -350,21 +351,18 @@  ext4_read_block_bitmap(struct super_block *sb, ext4_group_t block_group)
 }
 
 /**
- * ext4_free_blocks_sb() -- Free given blocks and update quota
+ * ext4_add_groupblocks() -- Add given blocks to an existing group and update quota
  * @handle:			handle to this transaction
  * @sb:				super block
  * @block:			start physcial block to free
  * @count:			number of blocks to free
  * @pdquot_freed_blocks:	pointer to quota
  *
- * XXX This function is only used by the on-line resizing code, which
- * should probably be fixed up to call the mballoc variant.  There
- * this needs to be cleaned up later; in fact, I'm not convinced this
- * is 100% correct in the face of the mballoc code.  The online resizing
- * code needs to be fixed up to more tightly (and correctly) interlock
- * with the mballoc code.
+ * This marks the blocks as free in the bitmap. We ask the
+ * mballoc to reload the buddy after this by setting group
+ * EXT4_GROUP_INFO_NEED_INIT_BIT flag
  */
-void ext4_free_blocks_sb(handle_t *handle, struct super_block *sb,
+void ext4_add_groupblocks(handle_t *handle, struct super_block *sb,
 			 ext4_fsblk_t block, unsigned long count,
 			 unsigned long *pdquot_freed_blocks)
 {
@@ -379,6 +377,7 @@  void ext4_free_blocks_sb(handle_t *handle, struct super_block *sb,
 	struct ext4_sb_info *sbi;
 	int err = 0, ret;
 	ext4_grpblk_t group_freed;
+	struct ext4_group_info *grp;
 
 	*pdquot_freed_blocks = 0;
 	sbi = EXT4_SB(sb);
@@ -419,7 +418,7 @@  void ext4_free_blocks_sb(handle_t *handle, struct super_block *sb,
 	    in_range(block + count - 1, ext4_inode_table(sb, desc),
 		     sbi->s_itb_per_group)) {
 		ext4_error(sb, "ext4_free_blocks",
-			   "Freeing blocks in system zones - "
+			   "Adding blocks in system zones - "
 			   "Block = %llu, count = %lu",
 			   block, count);
 		goto error_return;
@@ -472,35 +471,7 @@  void ext4_free_blocks_sb(handle_t *handle, struct super_block *sb,
 			cond_resched();
 			jbd_lock_bh_state(bitmap_bh);
 		}
-		/* @@@ This prevents newly-allocated data from being
-		 * freed and then reallocated within the same
-		 * transaction.
-		 *
-		 * Ideally we would want to allow that to happen, but to
-		 * do so requires making jbd2_journal_forget() capable of
-		 * revoking the queued write of a data block, which
-		 * implies blocking on the journal lock.  *forget()
-		 * cannot block due to truncate races.
-		 *
-		 * Eventually we can fix this by making jbd2_journal_forget()
-		 * return a status indicating whether or not it was able
-		 * to revoke the buffer.  On successful revoke, it is
-		 * safe not to set the allocation bit in the committed
-		 * bitmap, because we know that there is no outstanding
-		 * activity on the buffer any more and so it is safe to
-		 * reallocate it.
-		 */
-		BUFFER_TRACE(bitmap_bh, "set in b_committed_data");
-		J_ASSERT_BH(bitmap_bh,
-				bh2jh(bitmap_bh)->b_committed_data != NULL);
-		ext4_set_bit_atomic(sb_bgl_lock(sbi, block_group), bit + i,
-				bh2jh(bitmap_bh)->b_committed_data);
 
-		/*
-		 * We clear the bit in the bitmap after setting the committed
-		 * data bit, because this is the reverse order to that which
-		 * the allocator uses.
-		 */
 		BUFFER_TRACE(bitmap_bh, "clear bit");
 		if (!ext4_clear_bit_atomic(sb_bgl_lock(sbi, block_group),
 						bit + i, bitmap_bh->b_data)) {
@@ -545,6 +516,13 @@  void ext4_free_blocks_sb(handle_t *handle, struct super_block *sb,
 		goto do_more;
 	}
 	sb->s_dirt = 1;
+	/*
+	 * request to reload the buddy with the
+	 * new bitmap information
+	 */
+	grp = ext4_get_group_info(sb, block_group);
+	set_bit(EXT4_GROUP_INFO_NEED_INIT_BIT, &(grp->bb_state));
+	ext4_mb_update_group_info(grp, count);
 error_return:
 	brelse(bitmap_bh);
 	ext4_std_error(sb, err);
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 3882a6c..ca915d5 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1024,7 +1024,7 @@  extern ext4_fsblk_t ext4_has_free_blocks(struct ext4_sb_info *sbi,
 					 s64 nblocks);
 extern void ext4_free_blocks(handle_t *handle, struct inode *inode,
 			ext4_fsblk_t block, unsigned long count, int metadata);
-extern void ext4_free_blocks_sb(handle_t *handle, struct super_block *sb,
+extern void ext4_add_groupblocks(handle_t *handle, struct super_block *sb,
 				ext4_fsblk_t block, unsigned long count,
 				unsigned long *pdquot_freed_blocks);
 extern ext4_fsblk_t ext4_count_free_blocks(struct super_block *);
diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index b6ec184..a8403a8 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -977,7 +977,6 @@  int ext4_group_extend(struct super_block *sb, struct ext4_super_block *es,
 	int err;
 	unsigned long freed_blocks;
 	ext4_group_t group;
-	struct ext4_group_info *grp;
 
 	/* We don't need to worry about locking wrt other resizers just
 	 * yet: we're going to revalidate es->s_blocks_count after
@@ -1076,7 +1075,8 @@  int ext4_group_extend(struct super_block *sb, struct ext4_super_block *es,
 	unlock_super(sb);
 	ext4_debug("freeing blocks %llu through %llu\n", o_blocks_count,
 		   o_blocks_count + add);
-	ext4_free_blocks_sb(handle, sb, o_blocks_count, add, &freed_blocks);
+	/* We add the blocks to the bitmap and set the group need init bit */
+	ext4_add_groupblocks(handle, sb, o_blocks_count, add, &freed_blocks);
 	ext4_debug("freed blocks %llu through %llu\n", o_blocks_count,
 		   o_blocks_count + add);
 	if ((err = ext4_journal_stop(handle)))
@@ -1119,12 +1119,6 @@  int ext4_group_extend(struct super_block *sb, struct ext4_super_block *es,
 			ClearPageUptodate(page);
 			page_cache_release(page);
 		}
-
-		/* Get the info on the last group */
-		grp = ext4_get_group_info(sb, group);
-
-		/* Update free blocks in group info */
-		ext4_mb_update_group_info(grp, add);
 	}
 
 	if (test_opt(sb, DEBUG))