*[PATCH v2 00/12] btrfs: add zstd compression level support@ 2019-02-04 20:19 Dennis Zhou
2019-02-04 20:19 ` [PATCH 01/12] btrfs: add helpers for compression type and level Dennis Zhou
` (13 more replies)0 siblings, 14 replies; 34+ messages in thread
From: Dennis Zhou @ 2019-02-04 20:19 UTC (permalink / raw)
To: David Sterba, Josef Bacik, Chris Mason, Omar Sandoval,
Nick Terrell, Nikolay Borisov
Cc: kernel-team, linux-btrfs, linux-kernel, Dennis Zhou
Hi everyone,
V2 had only a handful of changes outside of minor feedback.
0001:
- use functions over macros
0003:
- BTRFS_NR_WORKSPACE_MANAGERS is added instead of overriding
BTRFS_COMPRESS_TYPES
0011 (new):
- address monotonic memory requirement for zstd workspaces
0012:
- increase reclaim timer to 307s from 67s
- move from keeping track of time in ns to jiffies
- remove timer in cleanup code
- use min_t() instead of if statements in .set_level()
- add header text to describe how workspaces are managed
- nofs_flag type -> unsigned long to unsigned int
From v1:
This is a respin of [1] which aims to add zstd compression level
support. V3 moves away from the using set_level() to resize workspaces
in favor of just allocating a workspace of the appropriate level and
using a timer to reclaim unused workspaces.
Zstd compression requires different amounts of memory for each level of
compression. The prior patches implemented indirection to allow for each
compression type to manage their workspaces independently. This patch
uses this indirection to implement compression level support for zstd.
As mentioned above, a requirement that differs zstd from zlib is that
higher levels of compression require more memory. To manage this, each
compression level has its own queue of workspaces. A global LRU is used
to help with reclaim. To guarantee forward progress, a max level
workspace is preallocated and hidden from the LRU.
When getting a workspace, it uses a bitmap to identify the levels that
are populated and scans up. If it finds a workspace that is greater than
it, it uses it, but does not update the last_used time and the
corresponding place in the LRU. This provides a mechanism to decrease
memory utilization as we only keep around workspaces that are sized
appropriately for the in use compression levels.
By knowing which compression levels have available workspaces, we can
recycle rather than always create new workspaces as well as take
advantage of the preallocated max level for forward progress. If we hit
memory pressure, we sleep on the max level workspace. We continue to
rescan in case we can use a smaller workspace, but eventually should be
able to obtain the max level workspace or allocate one again should
memory pressure subside. The memory requirement for decompression is the
same as level 1, and therefore can use any of available workspace.
The number of workspaces is bound by an upper limit of the workqueue's
limit which currently is 2 (percpu) limit). Second, a reclaim timer is
used to free inactive/improperly sized workspaces. The reclaim timer is
set to 67s to avoid colliding with transaction commit (every 30s) and
attempts to reclaim any unused workspace older than 45s.
Repeating the experiment from v2 [1], the Silesia corpus was copied to a
btrfs filesystem 10 times and then read back after dropping the caches.
The btrfs filesystem was on an SSD.
Level Ratio Compression (MB/s) Decompression (MB/s)
1 2.658 438.47 910.51
2 2.744 364.86 886.55
3 2.801 336.33 828.41
4 2.858 286.71 886.55
5 2.916 212.77 556.84
6 2.363 119.82 990.85
7 3.000 154.06 849.30
8 3.011 159.54 875.03
9 3.025 100.51 940.15
10 3.033 118.97 616.26
11 3.036 94.19 802.11
12 3.037 73.45 931.49
13 3.041 55.17 835.26
14 3.087 44.70 716.78
15 3.126 37.30 878.84
[1] https://lore.kernel.org/linux-btrfs/20181031181108.289340-1-terrelln@fb.com/
This patchset contains the following 12 patches:
0001-btrfs-add-helpers-for-compression-type-and-level.patch
0002-btrfs-rename-workspaces_list-to-workspace_manager.patch
0003-btrfs-manage-heuristic-workspace-as-index-0.patch
0004-btrfs-unify-compression-ops-with-workspace_manager.patch
0005-btrfs-add-helper-methods-for-workspace-manager-init-.patch
0006-btrfs-add-compression-interface-in-get-put-_workspac.patch
0007-btrfs-move-to-fn-pointers-for-get-put-workspaces.patch
0008-btrfs-plumb-level-through-the-compression-interface.patch
0009-btrfs-change-set_level-to-bound-the-level-passed-in.patch
0010-btrfs-zstd-use-the-passed-through-level-instead-of-d.patch
0011-btrfs-make-zstd-memory-requirements-monotonic.patch
0012-btrfs-add-zstd-compression-level-support.patch
0001 adds helper functions for type_level conversion. 0002 renames
workspaces_list to workspace_manager. 0003 moves back to managing the
heuristic workspaces as the index 0 compression level. 0004-0007 unify
operations with the workspace_manager with 0007 moving to compression
types owning their workspace_manager. 0008-0010 plumbs level throughout
the compression level getting interface and converts set_level() to be a
bounding function rather than setting level on a workspace. 0011
makes zstd compression level memory monotonic. 00012 adds zstd
compression level support.
This patchset is on top of kdave#master d73aba1115cf.
diffstats below:
Dennis Zhou (12):
btrfs: add helpers for compression type and level
btrfs: rename workspaces_list to workspace_manager
btrfs: manage heuristic workspace as index 0
btrfs: unify compression ops with workspace_manager
btrfs: add helper methods for workspace manager init and cleanup
btrfs: add compression interface in (get/put)_workspace()
btrfs: move to fn pointers for get/put workspaces
btrfs: plumb level through the compression interface
btrfs: change set_level() to bound the level passed in
btrfs: zstd use the passed through level instead of default
btrfs: make zstd memory requirements monotonic
btrfs: add zstd compression level support
fs/btrfs/compression.c | 249 +++++++++++++++------------------
fs/btrfs/compression.h | 53 ++++++-
fs/btrfs/lzo.c | 31 +++-
fs/btrfs/super.c | 10 +-
fs/btrfs/zlib.c | 45 ++++--
fs/btrfs/zstd.c | 311 +++++++++++++++++++++++++++++++++++++++--
6 files changed, 539 insertions(+), 160 deletions(-)
Thanks,
Dennis
^permalinkrawreply [flat|nested] 34+ messages in thread

*Re: [PATCH v2 00/12] btrfs: add zstd compression level support
2019-02-05 20:48 ` Dennis Zhou@ 2019-02-06 15:15 ` David Sterba
2019-02-06 16:51 ` Dennis Zhou0 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2019-02-06 15:15 UTC (permalink / raw)
To: Dennis Zhou
Cc: David Sterba, David Sterba, Josef Bacik, Chris Mason,
Omar Sandoval, Nick Terrell, Nikolay Borisov, kernel-team,
linux-btrfs, linux-kernel
On Tue, Feb 05, 2019 at 03:48:48PM -0500, Dennis Zhou wrote:
> > Ok great! I'm going to add a v2 for 0012 to add taking the spin_lock
> > just to be safe in cleanup. I should have that ready in a few minutes.
> >
>
> Before I run v3, are there any other concerns (pending the changes I
> made to address the mentioned make sense)?
You don't needs to do full v3 resend, either new version of the
individual patches or incremental changes that can be squashed to the
patch. I'm about to move the patchset from topic branch to misc-next so
incrementals work better now as there aren't big updates expected.
^permalinkrawreply [flat|nested] 34+ messages in thread

*Re: [PATCH v2 00/12] btrfs: add zstd compression level support
2019-02-06 15:15 ` David Sterba@ 2019-02-06 16:51 ` Dennis Zhou0 siblings, 0 replies; 34+ messages in thread
From: Dennis Zhou @ 2019-02-06 16:51 UTC (permalink / raw)
To: David Sterba
Cc: Dennis Zhou, David Sterba, Josef Bacik, Chris Mason,
Omar Sandoval, Nick Terrell, Nikolay Borisov, kernel-team,
linux-btrfs, linux-kernel
On Wed, Feb 06, 2019 at 04:15:52PM +0100, David Sterba wrote:
> On Tue, Feb 05, 2019 at 03:48:48PM -0500, Dennis Zhou wrote:
> > > Ok great! I'm going to add a v2 for 0012 to add taking the spin_lock
> > > just to be safe in cleanup. I should have that ready in a few minutes.
> > >
> >
> > Before I run v3, are there any other concerns (pending the changes I
> > made to address the mentioned make sense)?
>
> You don't needs to do full v3 resend, either new version of the
> individual patches or incremental changes that can be squashed to the
> patch. I'm about to move the patchset from topic branch to misc-next so
> incrementals work better now as there aren't big updates expected.
Ok great! I just sent a v2 of 0009 and v3 of 00012 which include the
changes for warning on incorrect input and level out of bounds. I did
forget to take out the "Re:" though.. whoops.
Thanks,
Dennis
^permalinkrawreply [flat|nested] 34+ messages in thread

*Re: [PATCH v2 00/12] btrfs: add zstd compression level support
2019-02-04 20:19 [PATCH v2 00/12] btrfs: add zstd compression level support Dennis Zhou
` (12 preceding siblings ...)
2019-02-05 14:57 ` [PATCH v2 00/12] " David Sterba
@ 2019-02-07 16:59 ` David Sterba
2019-02-07 17:36 ` Dennis Zhou13 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2019-02-07 16:59 UTC (permalink / raw)
To: Dennis Zhou
Cc: David Sterba, Josef Bacik, Chris Mason, Omar Sandoval,
Nick Terrell, Nikolay Borisov, kernel-team, linux-btrfs,
linux-kernel
On Mon, Feb 04, 2019 at 03:19:56PM -0500, Dennis Zhou wrote:
> Dennis Zhou (12):
> btrfs: add helpers for compression type and level
> btrfs: rename workspaces_list to workspace_manager
> btrfs: manage heuristic workspace as index 0
> btrfs: unify compression ops with workspace_manager
> btrfs: add helper methods for workspace manager init and cleanup
> btrfs: add compression interface in (get/put)_workspace()
> btrfs: move to fn pointers for get/put workspaces
> btrfs: plumb level through the compression interface
> btrfs: change set_level() to bound the level passed in
> btrfs: zstd use the passed through level instead of default
> btrfs: make zstd memory requirements monotonic
> btrfs: add zstd compression level support
The patchset has been added to msic-next, scheduled for 5.1. I've left
out the changes to warn on wrong level, so currently it falls back to
the default. I don't want to make a last minute change of that sort, so
this needs to go as a separate patch.
I also did a last pass through all patches and made some trivial changes
like reformatting comments or minor style updates, nothing functional.
The inter-branch diff can be found at
https://github.com/kdave/btrfs-devel/compare/ext/dzhou/compr-workspaces-v2..kdave:ext/dzhou/compr-workspaces
The feature should be ready to use, though as mentioned before some fine
tuning might be necessary to make the memory requirements more
efficient. The code could be simplified or cleaned up now that there are
3 compressors, at this point stabilization fixes are preferred.
Thanks.
^permalinkrawreply [flat|nested] 34+ messages in thread

*Re: [PATCH v2 00/12] btrfs: add zstd compression level support
2019-02-07 16:59 ` David Sterba@ 2019-02-07 17:36 ` Dennis Zhou
2019-02-08 13:22 ` David Sterba0 siblings, 1 reply; 34+ messages in thread
From: Dennis Zhou @ 2019-02-07 17:36 UTC (permalink / raw)
To: David Sterba
Cc: Dennis Zhou, David Sterba, Josef Bacik, Chris Mason,
Omar Sandoval, Nick Terrell, Nikolay Borisov, kernel-team,
linux-btrfs, linux-kernel
On Thu, Feb 07, 2019 at 05:59:26PM +0100, David Sterba wrote:
> On Mon, Feb 04, 2019 at 03:19:56PM -0500, Dennis Zhou wrote:
> > Dennis Zhou (12):
> > btrfs: add helpers for compression type and level
> > btrfs: rename workspaces_list to workspace_manager
> > btrfs: manage heuristic workspace as index 0
> > btrfs: unify compression ops with workspace_manager
> > btrfs: add helper methods for workspace manager init and cleanup
> > btrfs: add compression interface in (get/put)_workspace()
> > btrfs: move to fn pointers for get/put workspaces
> > btrfs: plumb level through the compression interface
> > btrfs: change set_level() to bound the level passed in
> > btrfs: zstd use the passed through level instead of default
> > btrfs: make zstd memory requirements monotonic
> > btrfs: add zstd compression level support
>
> The patchset has been added to msic-next, scheduled for 5.1. I've left
> out the changes to warn on wrong level, so currently it falls back to
> the default. I don't want to make a last minute change of that sort, so
> this needs to go as a separate patch.
Regarding the bug report from Dan, do you want me to send you a patch
for it on top of the topic branch or do you mind fixing the missing:
unsigned int level = 0;
>
> I also did a last pass through all patches and made some trivial changes
> like reformatting comments or minor style updates, nothing functional.
> The inter-branch diff can be found at
> https://github.com/kdave/btrfs-devel/compare/ext/dzhou/compr-workspaces-v2..kdave:ext/dzhou/compr-workspaces
>
> The feature should be ready to use, though as mentioned before some fine
> tuning might be necessary to make the memory requirements more
> efficient. The code could be simplified or cleaned up now that there are
> 3 compressors, at this point stabilization fixes are preferred.
>
> Thanks.
Thanks David!
Dennis
^permalinkrawreply [flat|nested] 34+ messages in thread