fix-deadlock-during-page-writeback

   1 ext4: fix deadlock during page writeback
   2
   3 From: Jan Kara <jack@suse.cz>
   4
   5 Commit 06bd3c36a733 (ext4: fix data exposure after a crash) uncovered a
   6 deadlock in ext4_writepages() which was previously much harder to hit.
   7 After this commit xfstest generic/130 reproduces the deadlock on small
   8 filesystems.
   9
  10 The problem happens when ext4_do_update_inode() sets LARGE_FILE feature
  11 and marks current inode handle as synchronous. That subsequently results
  12 in ext4_journal_stop() called from ext4_writepages() to block waiting for
  13 transaction commit while still holding page locks, reference to io_end,
  14 and some prepared bio in mpd structure each of which can possibly block
  15 transaction commit from completing and thus results in deadlock.
  16
  17 Fix the problem by releasing page locks, io_end reference, and
  18 submitting prepared bio before calling ext4_journal_stop().
  19
  20 [ Changed to defer the call to ext4_journal_stop() only if the handle
  21   is synchronous.  --tytso ]
  22
  23 Reported-and-tested-by: Eryu Guan <eguan@redhat.com>
  24 Signed-off-by: Theodore Ts'o <tytso@mit.edu>
  25 CC: stable@vger.kernel.org
  26 Signed-off-by: Jan Kara <jack@suse.cz>
  27 ---
  28  fs/ext4/inode.c | 29 ++++++++++++++++++++++++++---
  29  1 file changed, 26 insertions(+), 3 deletions(-)
  30
  31 diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
  32 index 44ee5d9..321a31c 100644
  33 --- a/fs/ext4/inode.c
  34 +++ b/fs/ext4/inode.c
  35 @@ -2754,13 +2754,36 @@ retry:
  36                                 done = true;
  37                         }
  38                 }
  39 -               ext4_journal_stop(handle);
  40 +               /*
  41 +                * Caution: If the handle is synchronous,
  42 +                * ext4_journal_stop() can wait for transaction commit
  43 +                * to finish which may depend on writeback of pages to
  44 +                * complete or on page lock to be released.  In that
  45 +                * case, we have to wait until after after we have
  46 +                * submitted all the IO, released page locks we hold,
  47 +                * and dropped io_end reference (for extent conversion
  48 +                * to be able to complete) before stopping the handle.
  49 +                */
  50 +               if (!ext4_handle_valid(handle) || handle->h_sync == 0) {
  51 +                       ext4_journal_stop(handle);
  52 +                       handle = NULL;
  53 +               }
  54                 /* Submit prepared bio */
  55                 ext4_io_submit(&mpd.io_submit);
  56                 /* Unlock pages we didn't use */
  57                 mpage_release_unused_pages(&mpd, give_up_on_write);
  58 -               /* Drop our io_end reference we got from init */
  59 -               ext4_put_io_end(mpd.io_submit.io_end);
  60 +               /*
  61 +                * Drop our io_end reference we got from init. We have
  62 +                * to be careful and use deferred io_end finishing if
  63 +                * we are still holding the transaction as we can
  64 +                * release the last reference to io_end which may end
  65 +                * up doing unwritten extent conversion.
  66 +                */
  67 +               if (handle) {
  68 +                       ext4_put_io_end_defer(mpd.io_submit.io_end);
  69 +                       ext4_journal_stop(handle);
  70 +               } else
  71 +                       ext4_put_io_end(mpd.io_submit.io_end);
  72
  73                 if (ret == -ENOSPC && sbi->s_journal) {
  74                         /*