linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Qu Wenruo <wqu@suse.com>
To: linux-btrfs@vger.kernel.org
Subject: [PATCH v6 0/4] Introduce per-profile available space array to avoid over-confident can_overcommit()
Date: Thu, 16 Jan 2020 14:03:59 +0800	[thread overview]
Message-ID: <20200116060404.95200-1-wqu@suse.com> (raw)

There are several bug reports of ENOSPC error in
btrfs_run_delalloc_range().

With some extra info from one reporter, it turns out that
can_overcommit() is using a wrong way to calculate allocatable metadata
space.

The most typical case would look like:
  devid 1 unallocated:	1G
  devid 2 unallocated:  10G
  metadata profile:	RAID1

In above case, we can at most allocate 1G chunk for metadata, due to
unbalanced disk free space.
But current can_overcommit() uses factor based calculation, which never
consider the disk free space balance.


To address this problem, here comes the per-profile available space
array, which gets updated every time a chunk get allocated/removed or a
device get grown or shrunk.

This provides a quick way for hotter place like can_overcommit() to grab
an estimation on how many bytes it can over-commit.

The per-profile available space calculation tries to keep the behavior
of chunk allocator, thus it can handle uneven disks pretty well.

And statfs() can also grab that pre-calculated value for instance usage.
For metadata over-commit, statfs() falls back to factor based educated
guess method.
Since over-commit can only happen when we have unallocated space, the
problem caused by over-commit should only be a first world problem.

Since this patch introduced a new failure pattern, some new error
handling are introduced:
- __btrfs_alloc_chunk()
  At the end of that function where calc_per_profile_avail() get called,
  if it failed due to -ENOMEM, we will revert device used space, and
  remove the allocated chunk.
  This is the only new error handling added by patch 5.

- btrfs_remove_chunk()
  There is no good way to revert the change. So here we abort
  transaction, just like what the old error handling does.

- btrfs_grow_device()
  This function has its problem by not reverting device used space from
  the very beginning.
  This patchset will enhance it in patch 4.

- btrfs_shrink_device()
  This function already has good error handling, reuse it.

- btrfs_verify_dev_extents()
  Mount time error will lead to mount failure, nothing to worry about.

Contents of the patchset:
Patch 1:	Core facility, with basic (not perfect) error handling
Patch 2:	Fix for over-confident can_overcommit()
Patch 3:	Make statfs() more accurate
Patch 4:	Better error handling for btrfs_grow_device()
Patch 5:	Better error handling for __btrfs_alloc_chunk()

If needed, patch 4 and patch 5 can be merged into patch 1.

Changelog:
v1:
- Fix a bug where we forgot to update per-profile array after allocating
  a chunk.
  To avoid ABBA deadlock, this introduce a small windows at the end
  __btrfs_alloc_chunk(), it's not elegant but should be good enough
  before we rework chunk and device list mutex.
  
- Make statfs() to use virtual chunk allocator to do better estimation
  Now statfs() can report not only more accurate result, but can also
  handle RAID5/6 better.

v2:
- Fix a deadlock caused by acquiring device_list_mutex under
  __btrfs_alloc_chunk()
  There is no need to acquire device_list_mutex when holding
  chunk_mutex.
  Fix it and remove the lockdep assert.

v3:
- Use proper chunk_mutex instead of device_list_mutex
  Since they are protecting two different things, and we only care about
  alloc_list, we should only use chunk_mutex.
  With improved lock situation, it's easier to fold
  calc_per_profile_available() calls into the first patch.

- Add performance benchmark for statfs() modification
  As Facebook seems to run into some problems with statfs() calls, add
  some basic ftrace results.

v4:
- Keep the lock-free design for statfs()
  As extra sleeping in statfs() may not be a good idea, keep the old
  lock-free design, and use factor based calculation as fall back.

v5:
- Enhance btrfs_update_device() error handling in btrfs_grow_device()
- Ensure all failure caused by calc_per_profile_available() is the same
  with existing error handling
- Fix a bug where chunk_mutex is not released in btrfs_shrink_device()

v6:
- Don't update the array if we hit any error.
  To avoid calling calc_per_profile_avail() in error handling path.

- Re-order the patchset
  Make the core facility the first patch.
  Error handling improvement in later patches.

- Add better error handling
  Improve one existing bad error handling, and provide a better solution
  for __btrfs_alloc_chunk()

Qu Wenruo (5):
  btrfs: Introduce per-profile available space facility
  btrfs: space-info: Use per-profile available space in can_overcommit()
  btrfs: statfs: Use pre-calculated per-profile available space
  btrfs: Reset device size when btrfs_update_device() failed in
    btrfs_grow_device()
  btrfs: volumes: Revert device used bytes when calc_per_profile_avail()
    failed

 fs/btrfs/space-info.c |  15 ++-
 fs/btrfs/super.c      | 182 +++++++++----------------------
 fs/btrfs/volumes.c    | 245 ++++++++++++++++++++++++++++++++++++++----
 fs/btrfs/volumes.h    |  11 ++
 4 files changed, 290 insertions(+), 163 deletions(-)

-- 
2.24.1


             reply	other threads:[~2020-01-16  6:04 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-16  6:03 Qu Wenruo [this message]
2020-01-16  6:04 ` [PATCH v6 1/5] btrfs: Introduce per-profile available space facility Qu Wenruo
2020-01-16 16:14   ` Josef Bacik
2020-01-17  0:55     ` Qu Wenruo
2020-01-17  1:50       ` Josef Bacik
2020-01-17  1:54         ` Qu Wenruo
2020-01-17  2:00           ` Josef Bacik
2020-01-16  6:04 ` [PATCH v6 2/5] btrfs: space-info: Use per-profile available space in can_overcommit() Qu Wenruo
2020-01-16  6:04 ` [PATCH v6 3/5] btrfs: statfs: Use pre-calculated per-profile available space Qu Wenruo
2020-01-16 16:21   ` Josef Bacik
2020-01-16  6:04 ` [PATCH v6 4/5] btrfs: Reset device size when btrfs_update_device() failed in btrfs_grow_device() Qu Wenruo
2020-01-16  6:04 ` [PATCH v6 5/5] btrfs: volumes: Revert device used bytes when calc_per_profile_avail() failed Qu Wenruo
2020-01-29  5:19 ` [PATCH v6 0/4] Introduce per-profile available space array to avoid over-confident can_overcommit() Qu WenRuo
2020-01-29  9:23   ` Nikolay Borisov
2020-01-29 10:51     ` Qu Wenruo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200116060404.95200-1-wqu@suse.com \
    --to=wqu@suse.com \
    --cc=linux-btrfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).