make-sure-to-revoke-all-the-freeable-blocks-in-ext4_free_blocks

   1 ext4: make sure to revoke all the freeable blocks in ext4_free_blocks
   2
   3 From: Daeho Jeong <daeho.jeong@samsung.com>
   4
   5 Now, ext4_free_blocks() doesn't revoke data blocks of per-file data
   6 journalled inode and it can cause file data inconsistency problems.
   7 Even though data blocks of per-file data journalled inode are already
   8 forgotten by jbd2_journal_invalidatepage() in advance of invoking
   9 ext4_free_blocks(), we still need to revoke the data blocks here.
  10 Moreover some of the metadata blocks, which are not found by
  11 sb_find_get_block(), are still needed to be revoked, but this is also
  12 missing here.
  13
  14 Signed-off-by: Daeho Jeong <daeho.jeong@samsung.com>
  15 Signed-off-by: Theodore Ts'o <tytso@mit.edu>
  16 Reviewed-by: Jan Kara <jack@suse.cz>
  17 ---
  18  fs/ext4/mballoc.c |   32 +++++++++++++-------------------
  19  1 file changed, 13 insertions(+), 19 deletions(-)
  20
  21 diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
  22 index 61eaf74..26419d6 100644
  23 --- a/fs/ext4/mballoc.c
  24 +++ b/fs/ext4/mballoc.c
  25 @@ -4695,16 +4695,6 @@ void ext4_free_blocks(handle_t *handle, struct inode *inode,
  26         }
  27
  28         /*
  29 -        * We need to make sure we don't reuse the freed block until
  30 -        * after the transaction is committed, which we can do by
  31 -        * treating the block as metadata, below.  We make an
  32 -        * exception if the inode is to be written in writeback mode
  33 -        * since writeback mode has weak data consistency guarantees.
  34 -        */
  35 -       if (!ext4_should_writeback_data(inode))
  36 -               flags |= EXT4_FREE_BLOCKS_METADATA;
  37 -
  38 -       /*
  39          * If the extent to be freed does not begin on a cluster
  40          * boundary, we need to deal with partial clusters at the
  41          * beginning and end of the extent.  Normally we will free
  42 @@ -4738,14 +4728,13 @@ void ext4_free_blocks(handle_t *handle, struct inode *inode,
  43
  44         if (!bh && (flags & EXT4_FREE_BLOCKS_FORGET)) {
  45                 int i;
  46 +               int is_metadata = flags & EXT4_FREE_BLOCKS_METADATA;
  47
  48                 for (i = 0; i < count; i++) {
  49                         cond_resched();
  50 -                       bh = sb_find_get_block(inode->i_sb, block + i);
  51 -                       if (!bh)
  52 -                               continue;
  53 -                       ext4_forget(handle, flags & EXT4_FREE_BLOCKS_METADATA,
  54 -                                   inode, bh, block + i);
  55 +                       if (is_metadata)
  56 +                               bh = sb_find_get_block(inode->i_sb, block + i);
  57 +                       ext4_forget(handle, is_metadata, inode, bh, block + i);
  58                 }
  59         }
  60
  61 @@ -4819,12 +4808,17 @@ do_more:
  62         if (err)
  63                 goto error_return;
  64
  65 -       if ((flags & EXT4_FREE_BLOCKS_METADATA) && ext4_handle_valid(handle)) {
  66 +       /*
  67 +        * We need to make sure we don't reuse the freed block until after the
  68 +        * transaction is committed. We make an exception if the inode is to be
  69 +        * written in writeback mode since writeback mode has weak data
  70 +        * consistency guarantees.
  71 +        */
  72 +       if (ext4_handle_valid(handle) &&
  73 +           ((flags & EXT4_FREE_BLOCKS_METADATA) ||
  74 +            !ext4_should_writeback_data(inode))) {
  75                 struct ext4_free_data *new_entry;
  76                 /*
  77 -                * blocks being freed are metadata. these blocks shouldn't
  78 -                * be used until this transaction is committed
  79 -                *
  80                  * We use __GFP_NOFAIL because ext4_free_blocks() is not allowed
  81                  * to fail.
  82                  */
  83 --
  84 1.7.9.5
  85
  86 --
  87 To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
  88 the body of a message to majordomo@vger.kernel.org
  89 More majordomo info at  http://vger.kernel.org/majordomo-info.html
  90