btrfs: remove force_page_uptodate variable from btrfs_buffered_write()

Commit c87c299776e4 ("btrfs: make buffered write to copy one page a
time") changed how the variable @force_page_uptodate was updated.

Before that commit the variable was only initialized to false at the
beginning of the function, and after hitting a short copy, the next
retry on the same folio would force the folio to be read from the disk.

But after the commit, the variable is always initialized to false at the
beginning of the loop's scope, causing prepare_one_folio() never to get a
true value passed in.

The change in behavior is not a huge deal, it only makes a difference
on how we handle short copies:

Old: Allow the buffer to be split

     The first short copy will be rejected, that's the same for both
     cases.

     But for the next retry, we require the folio to be read from disk.

     Then even if we hit a short copy again, since the folio is already
     uptodate, we do not need to handle partial uptodate range, and can
     continue, marking the short copied range as dirty and continue.

     This will split the buffer write into the folio as two buffered
     writes.

New: Do not allow the buffer to be split

     The first short copy will be rejected, that's the same for both
     cases.

     For the next retry, we do nothing special, thus if the short copy
     happened again, we reject it again, until either the short copy is
     gone, or we failed to fault in the buffer.

     This will mean the buffer write into the folio will either fail or
     succeed, no splitting will happen.

To me, either solution is fine, but the new one makes it simpler and
requires no special handling, so I prefer that solution.

And since @force_page_uptodate is always false when passed into
prepare_one_folio(), we can just remove the variable.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 71b8a825c4479a13df38c5e3f8abeba75d885b0a..53270c93b17a7e163c898e0b4e9246bc1f07bfb8 100644 (file)
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -800,7 +800,7 @@ out:
  * On success return a locked folio and 0
  */
 static int prepare_uptodate_folio(struct inode *inode, struct folio *folio, u64 pos,
-                                 u64 len, bool force_uptodate)
+                                 u64 len)
 {
        u64 clamp_start = max_t(u64, pos, folio_pos(folio));
        u64 clamp_end = min_t(u64, pos + len, folio_pos(folio) + folio_size(folio));
@@ -810,8 +810,7 @@ static int prepare_uptodate_folio(struct inode *inode, struct folio *folio, u64
        if (folio_test_uptodate(folio))
                return 0;
 
-       if (!force_uptodate &&
-           IS_ALIGNED(clamp_start, blocksize) &&
+       if (IS_ALIGNED(clamp_start, blocksize) &&
            IS_ALIGNED(clamp_end, blocksize))
                return 0;
 
@@ -858,7 +857,7 @@ static gfp_t get_prepare_gfp_flags(struct inode *inode, bool nowait)
  */
 static noinline int prepare_one_folio(struct inode *inode, struct folio **folio_ret,
                                      loff_t pos, size_t write_bytes,
-                                     bool force_uptodate, bool nowait)
+                                     bool nowait)
 {
        unsigned long index = pos >> PAGE_SHIFT;
        gfp_t mask = get_prepare_gfp_flags(inode, nowait);
@@ -883,7 +882,7 @@ again:
                folio_put(folio);
                return ret;
        }
-       ret = prepare_uptodate_folio(inode, folio, pos, write_bytes, force_uptodate);
+       ret = prepare_uptodate_folio(inode, folio, pos, write_bytes);
        if (ret) {
                /* The folio is already unlocked. */
                folio_put(folio);
@@ -1129,7 +1128,6 @@ ssize_t btrfs_buffered_write(struct kiocb *iocb, struct iov_iter *i)
                size_t num_sectors;
                struct folio *folio = NULL;
                int extents_locked;
-               bool force_page_uptodate = false;
 
                /*
                 * Fault pages before locking them in prepare_one_folio()
@@ -1198,8 +1196,7 @@ again:
                        break;
                }
 
-               ret = prepare_one_folio(inode, &folio, pos, write_bytes,
-                                       force_page_uptodate, false);
+               ret = prepare_one_folio(inode, &folio, pos, write_bytes, false);
                if (ret) {
                        btrfs_delalloc_release_extents(BTRFS_I(inode),
                                                       reserve_bytes);
@@ -1242,12 +1239,8 @@ again:
                                        fs_info->sectorsize);
                dirty_sectors = BTRFS_BYTES_TO_BLKS(fs_info, dirty_sectors);
 
-               if (copied == 0) {
-                       force_page_uptodate = true;
+               if (copied == 0)
                        dirty_sectors = 0;
-               } else {
-                       force_page_uptodate = false;
-               }
 
                if (num_sectors > dirty_sectors) {
                        /* release everything except the sectors we dirtied */