systemctl hibernate does not hibernate on swapfile on Btrfs after kernel 5.0

[helloworld1]

systemd version the issue has been seen with
241.7

Used distribution
Archlinux

Expected behaviour you didn't see
systemctl hibernate should hibernate on swap file on Btrfs after kernel 5.0

Unexpected behaviour you saw
Btrfs supports swapfile after kernel 5.0. echo disk > /sys/power/state works fine to hibernate but systemctl hibernate failed.
Since Btrfs is using virtual physical offset of the file. The current logic in sleep.c does not get the correct physical offset of the file. The resume_offset kernel parameter is also ignored and overriden by systemd so it is unable to get the correct offset.

systemd/src/sleep/sleep.c

Line 80 in c5e6f73

offset = fiemap->fm_extents[0].fe_physical / page_size();

Steps to reproduce the problem

1. Create swapfile on btrfs according to man 5 btrfs
2. Activate swap file in fstab
3. Calculate the physical offset of swapfile and add kernel parameter
   resume=/dev/nvme1n1p2 resume_offset=52761856
4. echo disk > /sys/power/state works fine to hibernate
5. systemctl hibernate failed.
   [ 26.828436] PM: Cannot find swap device, try swapon -a
   [ 26.828441] PM: Cannot get swap writer

[helloworld1]

The related discussion to get the physical offset of Btrfs swapfile is in https://bugzilla.kernel.org/show_bug.cgi?id=202803.

[poettering]

Hmm, so why does btrfs not follow the same APIs as other file systems on this? Isn#t this a btrfs interface issue?

[helloworld1]

I think this is by design for Btrfs since it can handle multiple storage devices. virtual physical offsets are provided to have a consistent view of file addressing. Seems like hibernation on swapfile requires real physical offset to work.
Feel free to comment on the https://bugzilla.kernel.org/show_bug.cgi?id=202803 with Btrfs developer so we can have a consensus on how to address the issue.
Maybe systemd should also read kernel parameter for the manually set resume_offset instead of calculating.

[osandov]

Yes, as @helloworld1 said, Btrfs gives you a virtual device number for stat and a virtual offset for fiemap by design. Even in the single device case, we make use of the virtual offsets to simplify block allocation.

In my opinion, the issue is that the hibernate interface is stupid. We should have an interface (system call? ioctl?) that just takes an fd and sets that swap device or file as the resume device. @poettering, would systemd make use of that if we were to implement it in the kernel? Or do you have any better ideas?

[poettering]

In my opinion, the issue is that the hibernate interface is stupid. We should have an interface (system call? ioctl?) that just takes an fd and sets that swap device or file as the resume device. @poettering, would systemd make use of that if we were to implement it in the kernel? Or do you have any better ideas?

Yes, please add that. Would love to simply remove the previous code then and just use the new API. The ioctl mucking is really terrible for this purpose.

[Snoop05]

May i ask how would people (with rights to merge pull requests) like to have this one solved?
a) Wait for btrfs to change till this magically starts working?
b) Implement btrfs swapfile support on systemd side?
c) Rewrite this component from scratch?
d) Honor resume and resume_offset parameters set in cmdline instead of wizardry?

Current status: neither resume, nor resume_offset gets detected properly, yet systemd will override both

[poettering]

anything specified on the kernel cmdline should take precedence over any automatic discovery. Hence, yes, I'd be happy to merge a patch that makes sure we always use the kernel parameters if they are specified.

That said, given that btrfs hibernation is a pretty new feature I think we'd also be fine to wait until the kernel learns a new hibernation API that allows us to just pass an fd in, and only support it when that's done.

The sleep code could generally use some love though. For example the s2h code uses RTC ioctls, where it probably should just use CLOCK_BOOTTIME_ALARM. Or we currently parse sleep.conf multiple times, which sucks too.

[zachsmith]

I was able to reproduce this bug on a virtual machine running Arch using the steps @helloworld1 provided in the initial report. I did have to use the utility @osandov provided in the issue reported in bugzilla to calculate the offset for btrfs correctly (in case anybody else tries it).

I submitted #12760 which solves the general issue of ignoring the resume_offset by implementing d) in @Snoop05's list above.

The route I took is fairly simple but I think it serves the purpose. Rather than inspecting the kernel cmdline for resume= and resume_offset= (which was how I originally started), it simply checks for any value already present in /sys/power/resume_offset. If it finds a value > 0 will assume that was set previously and return without any offset calculation or resume device override. I've confirmed that this works in the VM I setup and confirmed that the calculation continues to take place in absence of a resume_offset being set.

It does not check for /sys/power/resume or inspect resume= kernel cmdline. If /sys/power/resume_offset has a previously set value, it assumes that value provided is for the swapfile detected in find_hibernation_location which reads from /proc/swaps.

systemd/src/shared/sleep-config.c

Line 200 in c5e6f73

f = fopen("/proc/swaps", "re");

If anybody using btrfs with a swapfile is game, please give the PR a spin.