Re: Oops with ext4 from 2.6.27-rc3

[Mingming Cao <cmm@us.ibm.com>]

在 2008-08-13三的 18:01 -0400，Theodore Tso写道：
> On Wed, Aug 13, 2008 at 11:07:06PM +0200, Christian Hesse wrote:
> > 
> > Please look at the bottom of my last two mails... That was with your patch 
> > applied.
> 
> Sorry, I missed it.  The new BUG s without checking there is notch weems to be a bug in the delayed
> allocation code, specifically here, in fs/ext4/inode.c:ext4_da_release_space():
> 
> 	   /* figure out how many metablocks to release */
> 	   BUG_ON(mdb > EXT4_I(inode)->i_reserved_meta_blocks);
> 	   mdb_free = EXT4_I(inode)->i_reserved_meta_blocks - mdb;
> 
> I've quickly looked at the code, and how i_reserved_meta_blocks gets
> updated, and nothing *obviously* wrong is jumping out at me.  Anyone
> else have time to investigate this a bit more deeply?
> 

I could reproduce it.  

This patch works for me on top of Ted's change.  Christian, could you
try it?


---------------------------------------------------

Ext4: Fix delalloc release block reservation for truncate

From: Mingming Cao <cmm@us.ibm.com>

Ext4 will release the reserved blocks for delalloc 
when inode is truncated/unlinked.  If there is no reserved block at all,
we  shouldn't  need to do so.  But current code still tries to release the
reserved blocks regardless whether the counters's value is 0. 
Continue doing that causes the later calculation went wrong and a kernel BUG_ON()
catched that. This doesn't happen for originally extent format file, as the calculation
for 0 reserved blocks was right for  extent based file.

This patch fixed the kernel BUG() due to above reason. It adds checks for 0 to
avoid unnecessary release and fix calculation for non extent files.

Signed-off-by: Mingming Cao <cmm@us.ibm.com>
Index: linux-2.6.27-rc1/fs/ext4/inode.c
===================================================================
--- linux-2.6.27-rc1.orig/fs/ext4/inode.c	2008-08-13 15:29:35.000000000 -0700
+++ linux-2.6.27-rc1/fs/ext4/inode.c	2008-08-13 16:22:14.000000000 -0700
@@ -1007,6 +1007,9 @@ static int ext4_indirect_calc_metadata_a
  */
 static int ext4_calc_metadata_amount(struct inode *inode, int blocks)
 {
+	if (!blocks)
+		return 0;
+
 	if (EXT4_I(inode)->i_flags & EXT4_EXTENTS_FL)
 		return ext4_ext_calc_metadata_amount(inode, blocks);
 
@@ -1553,8 +1556,27 @@ static void ext4_da_release_space(struct
 	struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
 	int total, mdb, mdb_free, release;
 
+	if (!to_free){
+		/* Nothing to release, exit */
+		return;
+	}
+
 	spin_lock(&EXT4_I(inode)->i_block_reservation_lock);
 
+	if (!EXT4_I(inode)->i_reserved_data_blocks){
+		/*
+		 * if there is no reserved blocks, but we try to free some
+		 * then the counter is messed up somewhere.
+		 * but since this function is called from invalidate
+		 * page, it's harmless to return without any action
+		 */
+		printk(KERN_INFO "ext4 delalloc try to release %d reserved"
+			    "blocks for inode %lu, but there is no reserved"
+			    "data blocks\n", inode->i_ino, to_free);
+		spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
+		return;
+	}
+
 	/* recalculate the number of metablocks still need to be reserved */
 	total = EXT4_I(inode)->i_reserved_data_blocks - to_free;
 	mdb = ext4_calc_metadata_amount(inode, total);
@@ -3592,7 +3614,7 @@ void ext4_truncate(struct inode *inode)
 	 * ext4 *really* writes onto the disk inode.
 	 */
 	ei->i_disksize = inode->i_size;
-	
+
 	if (n == 1) {		/* direct blocks */
 		ext4_free_data(handle, inode, NULL, i_data+offsets[0],
 			       i_data + EXT4_NDIR_BLOCKS);


--
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