[PATCH] btrfs: don't wait for ordered extents in nowait buffered writes

[Joseph Qi <joseph.qi@linux.alibaba.com>]

lock_and_cleanup_extent() honours @nowait when taking the extent lock,
but then waits for an overlapping ordered extent regardless of it:

  btrfs_start_ordered_extent()
    wait_event(entry->wait, BTRFS_ORDERED_COMPLETE)

BTRFS_ORDERED_COMPLETE is only set by btrfs_remove_ordered_extent(),
which runs from the ordered extent completion work, so this sleeps
uninterruptibly until that work gets to run, and it starts a
transaction.  A pwritev2(RWF_NOWAIT) buffered write must not block like
that.

This used to be limited to writes starting below i_size, because the
ordered extent lookup was guarded by a start_pos < i_size test.  Commit
ec78575dde99 ("btrfs: always wait for ordered extents to avoid OE races")
dropped that guard so that appends also wait for the ordered extent left
behind by a short direct write whose i_size was reverted, which made the
wait, and the blocking with it, unconditional.

btrfs_write_check() rejects a NOWAIT write with -EAGAIN unless the inode
has BTRFS_INODE_NODATACOW or BTRFS_INODE_PREALLOC, so the inode has to
have one of those: a chattr +C file, a preallocated file, or a nodatacow
mount.  Those are the usual setups for database files and VM images,
which is also where NOWAIT writes get used.

Given such an inode, an append write does reach the wait.  With i_size S
and a direct write of L bytes of which only W get submitted:

  task A: O_DIRECT append, short write  task B: pwritev2(RWF_NOWAIT) append
  ------------------------------------  -----------------------------------
  btrfs_split_ordered_extent() gives
  the submitted part [S, S+W) its own
  OE, so dio_data->ordered is left
  covering [S+W, S+L)
  btrfs_dio_iomap_end():
    i_size_write(max(S, S+W))
      <- i_size reverted
    btrfs_mark_ordered_extent_truncated()
    btrfs_finish_ordered_extent()
      btrfs_queue_ordered_fn()
        -> endio_write_workers
      <- the OE is still in the tree
  iomap_dio_complete()
  btrfs_inode_unlock()
                                        btrfs_buffered_write() at S+W
                                          btrfs_inode_lock(BTRFS_ILOCK_TRY)
                                            <- succeeds, A dropped it
                                          btrfs_write_check()
                                            <- passes
                                          copy_one_range()
                                            lock_and_cleanup_extent()
                                              btrfs_try_lock_extent()
                                                <- succeeds
                                              btrfs_lookup_ordered_range()
                                                <- finds A's [S+W, S+L)
                                              btrfs_start_ordered_extent()
                                                wait_event()
                                                  <- blocks here
  endio_write_workers eventually runs
  finish_ordered_fn()
    btrfs_finish_ordered_io()
      btrfs_remove_ordered_extent()
        <- sets BTRFS_ORDERED_COMPLETE,
           B finally wakes up
                                        -EAGAIN propagates back through
                                        copy_one_range() to userspace

Because i_size was reverted to S+W, start_pos equals i_size, which is
exactly the case the dropped start_pos < i_size guard used to skip.

Return -EAGAIN without starting the ordered extent instead.
copy_one_range() passes that straight back and btrfs_buffered_write()
returns it to userspace when nothing was written yet, so a nowait caller
retries from a blocking context without IOCB_NOWAIT and takes the
waiting branch then.  btrfs_try_lock_ordered_range(), which
btrfs_check_nocow_lock() uses for the same purpose, already behaves this
way.

Fixes: ec78575dde99 ("btrfs: always wait for ordered extents to avoid OE races")
Cc: stable@vger.kernel.org
Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
---
 fs/btrfs/file.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index f978c6524aa03..e84e9e129169e 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -916,7 +916,14 @@ lock_and_cleanup_extent(struct btrfs_inode *inode, struct folio *folio,
 				    cached_state);
 		folio_unlock(folio);
 		folio_put(folio);
-		btrfs_start_ordered_extent(ordered);
+		/*
+		 * Waiting for the ordered extent blocks uninterruptibly, which
+		 * a nowait caller must not do.  Leave it alone and let the
+		 * caller retry where blocking is acceptable, like
+		 * btrfs_try_lock_ordered_range() does.
+		 */
+		if (!nowait)
+			btrfs_start_ordered_extent(ordered);
 		btrfs_put_ordered_extent(ordered);
 		return -EAGAIN;
 	}
-- 
2.39.3