dont-read-blocks-from-disk-after-extents-being-swapped

   1 ext4: don't read blocks from disk after extents being swapped
   2
   3 From: Eryu Guan <guaneryu@gmail.com>
   4
   5 I notice ext4/307 fails occasionally on ppc64 host, reporting md5
   6 checksum mismatch after moving data from original file to donor file.
   7
   8 The reason is that move_extent_per_page() calls __block_write_begin()
   9 and block_commit_write() to write saved data from original inode blocks
  10 to donor inode blocks, but __block_write_begin() not only maps buffer
  11 heads but also reads block content from disk if the size is not block
  12 size aligned.  At this time the physical block number in mapped buffer
  13 head is pointing to the donor file not the original file, and that
  14 results in reading wrong data to page, which get written to disk in
  15 following block_commit_write call.
  16
  17 This also can be reproduced by the following script on 1k block size ext4
  18 on x86_64 host:
  19
  20     mnt=/mnt/ext4
  21     donorfile=$mnt/donor
  22     testfile=$mnt/testfile
  23     e4compact=~/xfstests/src/e4compact
  24
  25     rm -f $donorfile $testfile
  26
  27     # reserve space for donor file, written by 0xaa and sync to disk to
  28     # avoid EBUSY on EXT4_IOC_MOVE_EXT
  29     xfs_io -fc "pwrite -S 0xaa 0 1m" -c "fsync" $donorfile
  30
  31     # create test file written by 0xbb
  32     xfs_io -fc "pwrite -S 0xbb 0 1023" -c "fsync" $testfile
  33
  34     # compute initial md5sum
  35     md5sum $testfile | tee md5sum.txt
  36     # drop cache, force e4compact to read data from disk
  37     echo 3 > /proc/sys/vm/drop_caches
  38
  39     # test defrag
  40     echo "$testfile" | $e4compact -i -v -f $donorfile
  41     # check md5sum
  42     md5sum -c md5sum.txt
  43
  44 Fix it by creating & mapping buffer heads only but not reading blocks
  45 from disk, because all the data in page is guaranteed to be up-to-date
  46 in mext_page_mkuptodate().
  47
  48 Cc: stable@vger.kernel.org
  49 Signed-off-by: Eryu Guan <guaneryu@gmail.com>
  50 Signed-off-by: Theodore Ts'o <tytso@mit.edu>
  51 ---
  52
  53 I tested this patch with ext4/307 and my reproducer more than 100 times on
  54 x86_64 and ppc64 hosts, also survived ext4 4k/2k/1k xfstests on both x86_64 and
  55 ppc64 host.
  56
  57  fs/ext4/move_extent.c | 15 ++++++++++++---
  58  1 file changed, 12 insertions(+), 3 deletions(-)
  59
  60 diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
  61 index fb6f117..e032a04 100644
  62 --- a/fs/ext4/move_extent.c
  63 +++ b/fs/ext4/move_extent.c
  64 @@ -265,11 +265,12 @@ move_extent_per_page(struct file *o_filp, struct inode *donor_inode,
  65         ext4_lblk_t orig_blk_offset, donor_blk_offset;
  66         unsigned long blocksize = orig_inode->i_sb->s_blocksize;
  67         unsigned int tmp_data_size, data_size, replaced_size;
  68 -       int err2, jblocks, retries = 0;
  69 +       int i, err2, jblocks, retries = 0;
  70         int replaced_count = 0;
  71         int from = data_offset_in_page << orig_inode->i_blkbits;
  72         int blocks_per_page = PAGE_CACHE_SIZE >> orig_inode->i_blkbits;
  73         struct super_block *sb = orig_inode->i_sb;
  74 +       struct buffer_head *bh = NULL;
  75
  76         /*
  77          * It needs twice the amount of ordinary journal buffers because
  78 @@ -380,8 +381,16 @@ data_copy:
  79         }
  80         /* Perform all necessary steps similar write_begin()/write_end()
  81          * but keeping in mind that i_size will not change */
  82 -       *err = __block_write_begin(pagep[0], from, replaced_size,
  83 -                                  ext4_get_block);
  84 +       if (!page_has_buffers(pagep[0]))
  85 +               create_empty_buffers(pagep[0], 1 << orig_inode->i_blkbits, 0);
  86 +       bh = page_buffers(pagep[0]);
  87 +       for (i = 0; i < data_offset_in_page; i++)
  88 +               bh = bh->b_this_page;
  89 +       for (i = 0; i < block_len_in_page; i++) {
  90 +               *err = ext4_get_block(orig_inode, orig_blk_offset + i, bh, 0);
  91 +               if (*err < 0)
  92 +                       break;
  93 +       }
  94         if (!*err)
  95                 *err = block_commit_write(pagep[0], from, from + replaced_size);
  96
  97 --
  98 2.5.0
  99
 100