fix-a-race-in-the-ext4-shutdown-path

   1 ext4: fix a race in the ext4 shutdown path
   2
   3 From: Harshad Shirwadkar <harshads@google.com>
   4
   5 This patch fixes a race between the shutdown path and bio completion
   6 handling. In the ext4 direct io path with async io, after submitting a
   7 bio to the block layer, if journal starting fails,
   8 ext4_direct_IO_write() would bail out pretending that the IO
   9 failed. The caller would have had no way of knowing whether or not the
  10 IO was successfully submitted. So instead, we return -EIOCBQUEUED in
  11 this case. Now, the caller knows that the IO was submitted.  The bio
  12 completion handler takes care of the error.
  13
  14 Tested: Ran the shutdown xfstest test 461 in loop for over 2 hours across
  15 4 machines resulting in over 400 runs. Verified that the race didn't
  16 occur. Usually the race was seen in about 20-30 iterations.
  17
  18 Signed-off-by: Harshad Shirwadkar <harshads@google.com>
  19 Signed-off-by: Theodore Ts'o <tytso@mit.edu>
  20 ---
  21  fs/ext4/inode.c | 16 ++++++++++++----
  22  1 file changed, 12 insertions(+), 4 deletions(-)
  23
  24 diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
  25 index 534a9130f625..4c2f8b57bdc7 100644
  26 --- a/fs/ext4/inode.c
  27 +++ b/fs/ext4/inode.c
  28 @@ -3767,10 +3767,18 @@ static ssize_t ext4_direct_IO_write(struct kiocb *iocb, struct iov_iter *iter)
  29                 /* Credits for sb + inode write */
  30                 handle = ext4_journal_start(inode, EXT4_HT_INODE, 2);
  31                 if (IS_ERR(handle)) {
  32 -                       /* This is really bad luck. We've written the data
  33 -                        * but cannot extend i_size. Bail out and pretend
  34 -                        * the write failed... */
  35 -                       ret = PTR_ERR(handle);
  36 +                       /*
  37 +                        * We wrote the data but cannot extend
  38 +                        * i_size. Bail out. In async io case, we do
  39 +                        * not return error here because we have
  40 +                        * already submmitted the corresponding
  41 +                        * bio. Returning error here makes the caller
  42 +                        * think that this IO is done and failed
  43 +                        * resulting in race with bio's completion
  44 +                        * handler.
  45 +                        */
  46 +                       if (!ret)
  47 +                               ret = PTR_ERR(handle);
  48                         if (inode->i_nlink)
  49                                 ext4_orphan_del(NULL, inode);
  50
  51 --
  52 2.16.0.rc1.238.g530d649a79-goog
  53
  54