]> www.infradead.org Git - users/dwmw2/linux.git/commitdiff
btrfs: wait for writeback if sector size is smaller than page size
authorQu Wenruo <wqu@suse.com>
Sun, 8 Sep 2024 22:52:06 +0000 (08:22 +0930)
committerDavid Sterba <dsterba@suse.com>
Mon, 11 Nov 2024 13:34:12 +0000 (14:34 +0100)
[PROBLEM]
If sector perfect compression is enabled for sector size < page size
case, the following case can lead dirty ranges not being written back:

     0     32K     64K     96K     128K
     |     |///////||//////|     |/|
                                 124K

In above example, the page size is 64K, and we need to write back above
two pages.

- Submit for page 0 (main thread)
  We found delalloc range [32K, 96K), which can be compressed.
  So we queue an async range for [32K, 96K).
  This means, the page unlock/clearing dirty/setting writeback will
  all happen in a workqueue context.

- The compression is done, and compressed range is submitted (workqueue)
  Since the compression is done in asynchronously, the compression can
  be done before the main thread to submit for page 64K.

  Now the whole range [32K, 96K), involving two pages, will be marked
  writeback.

- Submit for page 64K (main thread)
  extent_write_cache_pages() got its wbc->sync_mode is WB_SYNC_NONE,
  so it skips the writeback wait.

  And unlock the page and exit. This means the dirty range [124K, 128K)
  will never be submitted, until next writeback happens for page 64K.

This will never happen for previous kernels because:

- For sector size == page size case
  Since one page is one sector, if a page is marked writeback it will
  not have dirty flags.
  So this corner case will never hit.

- For sector size < page size case
  We never do subpage compression, a range can only be submitted for
  compression if the range is fully page aligned.
  This change makes the subpage behavior mostly the same as non-subpage
  cases.

[ENHANCEMENT]
Instead of relying WB_SYNC_NONE check only, if it's a subpage case, then
always wait for writeback flags.

Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
fs/btrfs/extent_io.c

index 872cca54cc6ce41bd0f6f5809b1a5671e6187239..7620d335f1ecfd95e94325a4fe8380e537fa9630 100644 (file)
@@ -2116,7 +2116,27 @@ retry:
                                continue;
                        }
 
-                       if (wbc->sync_mode != WB_SYNC_NONE) {
+                       /*
+                        * For subpage case, compression can lead to mixed
+                        * writeback and dirty flags, e.g:
+                        * 0     32K    64K    96K    128K
+                        * |     |//////||/////|   |//|
+                        *
+                        * In above case, [32K, 96K) is asynchronously submitted
+                        * for compression, and [124K, 128K) needs to be written back.
+                        *
+                        * If we didn't wait wrtiteback for page 64K, [128K, 128K)
+                        * won't be submitted as the page still has writeback flag
+                        * and will be skipped in the next check.
+                        *
+                        * This mixed writeback and dirty case is only possible for
+                        * subpage case.
+                        *
+                        * TODO: Remove this check after migrating compression to
+                        * regular submission.
+                        */
+                       if (wbc->sync_mode != WB_SYNC_NONE ||
+                           btrfs_is_subpage(inode_to_fs_info(inode), mapping)) {
                                if (folio_test_writeback(folio))
                                        submit_write_bio(bio_ctrl, 0);
                                folio_wait_writeback(folio);