ext4: call ext4_forget() from ext4_free_blocks()

Add the facility for ext4_forget() to be called from
ext4_free_blocks().  This simplifies the code in a large number of
places, and centralizes most of the work of calling ext4_forget() into
a single place.

Also fix a bug in the extents migration code; it wasn't calling
ext4_forget() when releasing the indirect blocks during the
conversion.  As a result, if the system cashed during or shortly after
the extents migration, and the released indirect blocks get reused as
data blocks, the journal replay would corrupt the data blocks.  With
this new patch, fixing this bug was as simple as adding the
EXT4_FREE_BLOCKS_FORGET flags to the call to ext4_free_blocks().

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>

1 files changed, 26 insertions, 41 deletions

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 72c694323492..3b28e1fbfc90 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -669,7 +669,7 @@ allocated:
         return ret;
 failed_out:
         for (i = 0; i < index; i++)
-                ext4_free_blocks(handle, inode, new_blocks[i], 1, 0);
+                ext4_free_blocks(handle, inode, 0, new_blocks[i], 1, 0);
         return ret;
 }
 
@@ -765,20 +765,20 @@ static int ext4_alloc_branch(handle_t *handle, struct inode *inode,
         return err;
 failed:
         /* Allocation failed, free what we already allocated */
+        ext4_free_blocks(handle, inode, 0, new_blocks[0], 1, 0);
         for (i = 1; i <= n ; i++) {
-                BUFFER_TRACE(branch[i].bh, "call jbd2_journal_forget");
                 /* 
-                 * Note: is_metadata is 0 because branch[i].bh is
-                 * newly allocated, so there is no need to revoke the
-                 * block.  If we do, it's harmless, but not necessary.
+                 * branch[i].bh is newly allocated, so there is no
+                 * need to revoke the block, which is why we don't
+                 * need to set EXT4_FREE_BLOCKS_METADATA.
                  */
-                ext4_forget(handle, 0, inode, branch[i].bh,
-                            branch[i].bh->b_blocknr);
+                ext4_free_blocks(handle, inode, 0, new_blocks[i], 1,
+                                 EXT4_FREE_BLOCKS_FORGET);
         }
-        for (i = 0; i < indirect_blks; i++)
-                ext4_free_blocks(handle, inode, new_blocks[i], 1, 0);
+        for (i = n+1; i < indirect_blks; i++)
+                ext4_free_blocks(handle, inode, 0, new_blocks[i], 1, 0);
 
-        ext4_free_blocks(handle, inode, new_blocks[i], num, 0);
+        ext4_free_blocks(handle, inode, 0, new_blocks[i], num, 0);
 
         return err;
 }
@@ -857,18 +857,16 @@ static int ext4_splice_branch(handle_t *handle, struct inode *inode,
 
 err_out:
         for (i = 1; i <= num; i++) {
-                BUFFER_TRACE(where[i].bh, "call jbd2_journal_forget");
                 /* 
-                 * Note: is_metadata is 0 because branch[i].bh is
-                 * newly allocated, so there is no need to revoke the
-                 * block.  If we do, it's harmless, but not necessary.
+                 * branch[i].bh is newly allocated, so there is no
+                 * need to revoke the block, which is why we don't
+                 * need to set EXT4_FREE_BLOCKS_METADATA.
                  */
-                ext4_forget(handle, 0, inode, where[i].bh,
-                            where[i].bh->b_blocknr);
-                ext4_free_blocks(handle, inode,
-                                        le32_to_cpu(where[i-1].key), 1, 0);
+                ext4_free_blocks(handle, inode, where[i].bh, 0, 1,
+                                 EXT4_FREE_BLOCKS_FORGET);
         }
-        ext4_free_blocks(handle, inode, le32_to_cpu(where[num].key), blks, 0);
+        ext4_free_blocks(handle, inode, 0, le32_to_cpu(where[num].key),
+                         blks, 0);
 
         return err;
 }
@@ -4080,7 +4078,10 @@ static void ext4_clear_blocks(handle_t *handle, struct inode *inode,
                               __le32 *last)
 {
         __le32 *p;
-        int     is_metadata = S_ISDIR(inode->i_mode) || S_ISLNK(inode->i_mode);
+        int     flags = EXT4_FREE_BLOCKS_FORGET;
+
+        if (S_ISDIR(inode->i_mode) || S_ISLNK(inode->i_mode))
+                flags |= EXT4_FREE_BLOCKS_METADATA;
 
         if (try_to_extend_transaction(handle, inode)) {
                 if (bh) {
@@ -4096,27 +4097,10 @@ static void ext4_clear_blocks(handle_t *handle, struct inode *inode,
                 }
         }
 
-        /*
-         * Any buffers which are on the journal will be in memory. We
-         * find them on the hash table so jbd2_journal_revoke() will
-         * run jbd2_journal_forget() on them.  We've already detached
-         * each block from the file, so bforget() in
-         * jbd2_journal_forget() should be safe.
-         *
-         * AKPM: turn on bforget in jbd2_journal_forget()!!!
-         */
-        for (p = first; p < last; p++) {
-                u32 nr = le32_to_cpu(*p);
-                if (nr) {
-                        struct buffer_head *tbh;
-
-                        *p = 0;
-                        tbh = sb_find_get_block(inode->i_sb, nr);
-                        ext4_forget(handle, is_metadata, inode, tbh, nr);
-                }
-        }
+        for (p = first; p < last; p++)
+                *p = 0;
 
-        ext4_free_blocks(handle, inode, block_to_free, count, is_metadata);
+        ext4_free_blocks(handle, inode, 0, block_to_free, count, flags);
 }
 
 /**
@@ -4304,7 +4288,8 @@ static void ext4_free_branches(handle_t *handle, struct inode *inode,
                                             blocks_for_truncate(inode));
                         }
 
-                        ext4_free_blocks(handle, inode, nr, 1, 1);
+                        ext4_free_blocks(handle, inode, 0, nr, 1,
+                                         EXT4_FREE_BLOCKS_METADATA);
 
                         if (parent_bh) {
                                 /*