Commit 4750af3b authored by Qu Wenruo's avatar Qu Wenruo Committed by David Sterba

btrfs: prevent extent_clear_unlock_delalloc() to unlock page not locked by __process_pages_contig()

In cow_file_range(), after we have succeeded creating an inline extent,
we unlock the page with extent_clear_unlock_delalloc() by passing
locked_page == NULL.

For sectorsize == PAGE_SIZE case, this is just making the page lock and
unlock harder to grab.

But for incoming subpage case, it can be a big problem.

For incoming subpage case, page locking have two entry points:

- __process_pages_contig()
  In that case, we know exactly the range we want to lock (which only
  requires sector alignment).
  To handle the subpage requirement, we introduce btrfs_subpage::writers
  to page::private, and will update it in __process_pages_contig().

- Other directly lock/unlock_page() call sites
  Those won't touch btrfs_subpage::writers at all.

This means, page locked by __process_pages_contig() can only be unlocked
by __process_pages_contig().
Thankfully we already have the existing infrastructure in the form of
@locked_page in various call sites.

Unfortunately, extent_clear_unlock_delalloc() in cow_file_range() after
creating an inline extent is the exception.
It intentionally call extent_clear_unlock_delalloc() with locked_page ==
NULL, to also unlock current page (and clear its dirty/writeback bits).

To co-operate with incoming subpage modifications, and make the page
lock/unlock pair easier to understand, this patch will still call
extent_clear_unlock_delalloc() with locked_page, and only unlock the
page in __extent_writepage().

Tested-by: Ritesh Harjani <riteshh@linux.ibm.com> # [ppc64]
Tested-by: Anand Jain <anand.jain@oracle.com> # [aarch64]
Signed-off-by: default avatarQu Wenruo <wqu@suse.com>
Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
parent a33a8e9a
...@@ -1091,7 +1091,8 @@ static noinline int cow_file_range(struct btrfs_inode *inode, ...@@ -1091,7 +1091,8 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
* our outstanding extent for clearing delalloc for this * our outstanding extent for clearing delalloc for this
* range. * range.
*/ */
extent_clear_unlock_delalloc(inode, start, end, NULL, extent_clear_unlock_delalloc(inode, start, end,
locked_page,
EXTENT_LOCKED | EXTENT_DELALLOC | EXTENT_LOCKED | EXTENT_DELALLOC |
EXTENT_DELALLOC_NEW | EXTENT_DEFRAG | EXTENT_DELALLOC_NEW | EXTENT_DEFRAG |
EXTENT_DO_ACCOUNTING, PAGE_UNLOCK | EXTENT_DO_ACCOUNTING, PAGE_UNLOCK |
...@@ -1099,6 +1100,19 @@ static noinline int cow_file_range(struct btrfs_inode *inode, ...@@ -1099,6 +1100,19 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
*nr_written = *nr_written + *nr_written = *nr_written +
(end - start + PAGE_SIZE) / PAGE_SIZE; (end - start + PAGE_SIZE) / PAGE_SIZE;
*page_started = 1; *page_started = 1;
/*
* locked_page is locked by the caller of
* writepage_delalloc(), not locked by
* __process_pages_contig().
*
* We can't let __process_pages_contig() to unlock it,
* as it doesn't have any subpage::writers recorded.
*
* Here we manually unlock the page, since the caller
* can't use page_started to determine if it's an
* inline extent or a compressed extent.
*/
unlock_page(locked_page);
goto out; goto out;
} else if (ret < 0) { } else if (ret < 0) {
goto out_unlock; goto out_unlock;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment