ZFS ignores ARC sizes below allmem/32

[Heitor Alves de Siqueira]

[Impact]
ZFS ignores tunable "zfs_arc_max" due to it being below allmem/32 threshold. This prevents users from properly restraining ARC sizes, and can cause increased memory contention in some systems.

[Test Plan]
1. Deploy test system with ZFS storage and 32GB RAM
2. Add ARC tunables to /etc/modprobe.d/99-zfs-arc.conf
   # cat /etc/modprobe.d/99-zfs-arc.conf
   options zfs zfs_arc_min=536870912
   options zfs zfs_arc_max=966367641
3. Reboot system
4. Verify ARC sizes through "arc_summary"
   # arc_summary | grep -A3 "ARC size"
   ARC size (current): < 0.1 % 1.3 MiB
           Target size (adaptive): 100.0 % 15.7 GiB
           Min size (hard limit): 3.2 % 512.0 MiB
           Max size (high water): 31:1 15.7 GiB

For a 32GB test system, we should be able to set max ARC sizes below 1GB.

[Fix]
This has been fixed by upstream commit:
 - 36a6e2335c45 "Don't ignore zfs_arc_max below allmem/32"
 - e945e8d7f4fc "Restore FreeBSD sysctl processing for arc.min and arc.max"

The commit has been introduced in upstream zfs-2.0.0, so it's needed for Bionic and Focal. Releases starting with Impish already have this commit by default:

$ git describe --contains 36a6e2335c45
zfs-2.0.0-rc1~332
$ rmadison zfs-linux
 zfs-linux | 0.7.5-1ubuntu15 | bionic | source
 zfs-linux | 0.7.5-1ubuntu16.12 | bionic-updates | source
 zfs-linux | 0.8.3-1ubuntu12 | focal | source
 zfs-linux | 0.8.3-1ubuntu12.9 | focal-security | source
 zfs-linux | 0.8.3-1ubuntu12.13 | focal-updates | source
 zfs-linux | 0.8.3-1ubuntu12.14 | focal-proposed | source
 zfs-linux | 2.0.6-1ubuntu2 | impish | source
 zfs-linux | 2.0.6-1ubuntu2.1 | impish-updates | source
 zfs-linux | 2.1.2-1ubuntu3 | jammy | source

[Regression Potential]
The introduced commit essentially removes the limitation of setting ARC tunables below allmem/32, and re-arranges the order of how some of the tunables are parsed. Regressions would possibly show up as other tunables being ignored or not being set correctly due to parsing errors. We should validate whether other ARC related tunables are still being set correctly, and whether ZFS is using the set values for the ARC memory thresholds.

[Heitor Alves de Siqueira (halves)]

This doesn't seem to be reproducible in Bionic, marking it as fixed accordingly.

[Robie Basak (racb)]

Doesn't this just invert the problem? Before, the user couldn't set the max to below the default min. But setting the min to above the default max would have worked. After this patch, what if the user tries to set the min to above the default max? Won't that now fail?

I don't see any discussion of this upstream. Surely both min and max need to be considered together for a proper fix? Maybe it's not a problem in practice for some reason I'm missing?

Separately, another possible regression is that a user has bad tunables set that aren't currently having any effect, and by issuing a fix those bad tunables will take effect and regress behaviour. That's something for users to look out for, but I don't think we should hold back a fix for a real bug for this reason.

[Heitor Alves de Siqueira (halves)]

Hi Robie,

You're right, the patch does essentially invert the problem. This is still the behavior upstream, and it currently works like you mentioned: if the user tries to set a min above the default max (ramsize/2), it fails.

I'm working on a patch to propose upstream that should fix this. We should be setting min/max values as a pairs else we'll run into a similar issue as the one reported here. I'm also going to double check other tunables to see if they exhibit similar issues, so we can avoid further problems on those too.

For this particular LP bug, do you think we should wait until a "proper" fix upstream? I do understand the point about breaking setups relying on the current min/max behavior, but that will also happen when upgrading to newer releases. My (subjective) opinion is that users trying to reduce ZFS memory footprint are much more common than the alternative, and for high memory systems this is currently not possible due to this bug. What do you think?

[Łukasz Zemczak (sil2100)]

Hey Heitor! Thank you for following up on Robie's questions. As this is a bug in the current upstream versions of zfs-linux + currently LTS upgrades to jammy are enabled, I'm starting to lean towards getting this accepted into focal-proposed. Out of curiosity, what is the situation on bionic btw.? What is the behavior there?

Too bad there's simply no way of knowing how many people would be potentially affected by the inversion of the behavior. Did we see any reports of this being a problem on jammy, after we switched upgrades from focal? The thing is that, potentially, some might want to stay of focal to not have to deal with incompatibilities like that after upgrades. So we need to be considerate of such people. Although I do agree the reducing of ZFS memory footprint being much more common.

[Robie Basak (racb)]

The correct logic shouldn't be hard to write, unless I'm missing something? So can we just do it right, and as soon as is necessary? If it's obviously correct, I don't think we need to wait for upstream acceptance, but equally it shouldn't take them long to review it either. Because the parameters are well-defined, and the correct validation is unambiguous. So diverging from upstream on the precise fix shouldn't matter as long as it is reviewed to be correct, and that shouldn't be difficult.

[Robie Basak (racb)]

As there's been no response, I'm cleaning this up from the queue. This can still be fixed in Focal, but see the comments above on what needs to be resolved first.

[Robie Basak (racb)]

(and please re-upload when ready - the SRU team won't notice to respond otherwise)

[Heitor Alves de Siqueira (halves)]

No worries, thank you for the patience, Robie!

With further testing, we've found out that there's an additional upstream
patch required for Focal, which fixes the "inversion" when setting the ARC
limits.

I'll amend the description and re-spin this SRU to include the proper
fixes. I'll then re-upload, after running the regression tests on the new version.

[Ghadi Rahme (ghadi-rahme)]

Hello, I just wanted to clarify the changes in the two commits that were backported.
For commit 36a6e2335c45, the main features that were backported were the warnings generated when ARC fails to set the max and min value to the user specified values. Although this commit has a fix for the zfs_arc_max size below allmem/32, it does have a major regression where the issue gets essentially flipped and will cause zfs_arc_min not to accept any value above allmem/2.

Commit e945e8d7f4fc although mainly a FreeBSD patch, does fix the regression created by the previous commit and assures that the user cannot set the value of zfs_arc_max below the maximum transaction size of 64MB. All other FreeBSD specific changes were ignored.

[Dimitri John Ledkov (xnox)]

Editing 0.8.3-1ubuntu12.14 changelog seems inappropriate. Whoever is sponsoring this debdiff, should drop that hung, as long as it doesn't have any other mistakes.

[Heitor Alves de Siqueira (halves)]

Thanks, Dimitri! I've tested Ghadi's patches and confirmed they work and don't trigger any regressions in the ZFS test suite.

I've asked Ghadi to rework some portions of his backport in order to be closer to the upstream patches though, so we should have a v2 soon. We'll make sure that previous changelog entries aren't modified, and I'll re-run the ZFS regression tests against the new version as well.
Thank you for the work on this one, Ghadi!

[Heitor Alves de Siqueira (halves)]

Thank you for the v2, Ghadi! I made some minor modifications to your changelog entry (added "d/p/" before the patch files), and included a "backport-notes:" tag in your second patch mentioning the FreeBSD-specific sections were dropped from upstream.

With the v2, I ran the regression tests again and did some basic validation. Both min and max ARC parameters were set as expected, and invalid values are being ignored. I was able to set arc_min below RAM/32 as well as above RAM/2, and the same for arc_max.

Uploaded to focal, thank you for the help!

[Łukasz Zemczak (sil2100)]

Hello Heitor, or anyone else affected,

Accepted zfs-linux into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/zfs-linux/0.8.3-1ubuntu12.15 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Heitor Alves de Siqueira (halves)]

zfs-linux FTBS on riscv64, but that's been the case for the versions from -updates for some time now. It's not related to this upload, but rather to the ZFS version in focal not supporting riscv64:

In file included from ../../lib/libspl/include/sys/types.h:36,
                 from ../../module/avl/avl.c:101:
../../lib/libspl/include/sys/isa_defs.h:200:2: error: #error "Unsupported ISA type"
  200 | #error "Unsupported ISA type"
      | ^~~~~
../../lib/libspl/include/sys/isa_defs.h:216:2: error: #error "Neither _LITTLE_ENDIAN nor _BIG_ENDIAN are defined"
  216 | #error "Neither _LITTLE_ENDIAN nor _BIG_ENDIAN are defined"
      | ^~~~~
make[5]: *** [Makefile:709: avl.lo] Error 1

[Ghadi Rahme (ghadi-rahme)]

Hello everyone,

I was able to confirm the fix for focal using the test case from the description:
root@zfs-test:~$ free -h
              total used free shared buff/cache available
Mem: 31Gi 247Mi 30Gi 1.0Mi 352Mi 30Gi
Swap: 0B 0B 0B

root@zfs-test:~$ cat /etc/modprobe.d/99-zfs-arc.conf
options zfs zfs_arc_min=536870912
options zfs zfs_arc_max=966367641

root@zfs-test:~$ dpkg -l | grep zfs
ii libzfs2linux 0.8.3-1ubuntu12.15 amd64 OpenZFS filesystem library for Linux
ii zfs-dkms 0.8.3-1ubuntu12.15 all OpenZFS filesystem kernel modules for Linux
ii zfs-zed 0.8.3-1ubuntu12.15 amd64 OpenZFS Event Daemon
ii zfsutils-linux 0.8.3-1ubuntu12.15 amd64 command-line tools to manage OpenZFS filesystems

root@zfs-test:~$ sudo update-initramfs -u -k all

root@zfs-test:~$ reboot

root@zfs-test:~$ arc_summary -s arc

------------------------------------------------------------------------
ZFS Subsystem Report Tue Apr 18 10:22:26 2023
Linux 5.4.0-146-generic 0.8.3-1ubuntu12.15
Machine: zfs-test (x86_64) 0.8.3-1ubuntu12.15

ARC status: HEALTHY
        Memory throttle count: 0

ARC size (current): 0.0 % 0 Bytes
        Target size (adaptive): 100.0 % 921.6 MiB
        Min size (hard limit): 55.6 % 512.0 MiB
        Max size (high water): 1:1 921.6 MiB
        Most Frequently Used (MFU) cache size: n/a 0 Bytes
        Most Recently Used (MRU) cache size: n/a 0 Bytes
        Metadata cache size (hard limit): 75.0 % 691.2 MiB
        Metadata cache size (current): 0.0 % 0 Bytes
        Dnode cache size (hard limit): 10.0 % 69.1 MiB
        Dnode cache size (current): 0.0 % 0 Bytes

ARC hash breakdown:
        Elements max: 0
        Elements current: n/a 0
        Collisions: 0
        Chain max: 0
        Chains: 0

ARC misc:
        Deleted: 0
        Mutex misses: 0
        Eviction skips: 0

[Launchpad Janitor (janitor)]

This bug was fixed in the package zfs-linux - 0.8.3-1ubuntu12.15

---------------
zfs-linux (0.8.3-1ubuntu12.15) focal; urgency=medium

  * Fix zfs_arc_max getting ignored when value below allmem/32 (LP: #1964992)
   - d/p/4930-Dont-ignore-zfs_arc_max-below-allmem-32.patch
   - d/p/4931-Restore-processing-for-arc-min-and-arc-max.patch

 -- Ghadi Elie Rahme <email address hidden> Wed, 30 Nov 2022 15:46:58 +0000

[Chris Halse Rogers (raof)]

The verification of the Stable Release Update for zfs-linux has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.