Bug 1469726 - Review Request: zram - Enable compressed swap in memory
Summary: Review Request: zram - Enable compressed swap in memory
Keywords:
Status: CLOSED DUPLICATE of bug 1602846
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 1169926 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-07-11 18:12 UTC by Raphael Groner
Modified: 2018-07-22 06:32 UTC (History)
9 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2018-07-18 15:47:09 UTC


Attachments (Terms of Use)

Description Raphael Groner 2017-07-11 18:12:18 UTC
Spec URL: http://raphgro.fedorapeople.org//zram.spec
SRPM URL: http://raphgro.fedorapeople.org//zram-0-0.1.20170206git3a7a278.fc26.src.rpm

Description:
zram compresses swap partitions into RAM for performance.

Comment 1 Raphael Groner 2017-07-11 18:12:22 UTC
This package built on koji:  https://koji.fedoraproject.org/koji/taskinfo?taskID=20463801

Comment 2 Raphael Groner 2017-07-11 18:13:46 UTC
May replace the (stalled?) review in bug #1169926.

Comment 3 Raphael Groner 2017-07-11 18:29:20 UTC
In case of any relevance, I explicitly decided to use the package name zram instead of the upstream name systemd-zram because this project is not part of the systemd core and so to avoid any confusion, though guidelines [1] miss a special part for systemd addons as we have for other base packages.
[1] https://fedoraproject.org/wiki/Packaging:Naming?rd=Packaging:NamingGuidelines#Addon_Packages

Comment 4 František Zatloukal 2017-07-27 13:50:43 UTC
Packaging looks good to me.

I see upstream recommends disabling swap after installation but I don't see the package doing it. Is it an issue if swap remains enabled? It would make sense to at least print warning to the terminal during installation (I'd be against disabling swap in a completely automatic manner).

The other potential issue - I see documented incompatibility with encrypted /home . Does it break anything after installing this package to such system?

Comment 5 Zbigniew Jędrzejewski-Szmek 2017-08-04 19:25:08 UTC
*** Bug 1169926 has been marked as a duplicate of this bug. ***

Comment 6 Zbigniew Jędrzejewski-Szmek 2017-08-04 19:44:52 UTC
> # need Linux kernel version 2.6.37.1 or better to use zram
> Requires: kernel >= 2.6.37.1

First, nowadays kernel has been split into kernel-core and other subpackages, so you can easily boot a machine without kernel.rpm (which also pulls in kernel-modules). Second, the fact that you have a specific kernel.rpm installed does not mean that you're running with that version so this is not effective. And third, there's no way to install a kernel rpm for 2.6.x in any Fedora. So you should just drop this line.

> BuildRequires: systemd-units

> Requires(post): systemd-sysv
> Requires(post): systemd-units
> Requires(preun): systemd-units
> Requires(postun): systemd-units
> Requires: systemd
> Requires: initscripts

This should be replaced by
%{?systemd_requires}
BuildRequires: systemd
[https://fedoraproject.org/wiki/Packaging:Scriptlets#Systemd]

> %systemd_postun_with_restart mkzram.service
Hmm, that could be dangerous. Disabling swap during an upgrade could bring the machine down. %systemd_postun seems more reasonable.

> After=multi-user.target
That's certainly the wrong place to order this. Not only it causes a dependency loop with swap.target which is ordered much earlier, it might also disable swap at the beginning of shutdown, crashing the machine.

> sleep 1
Hmm. swapoff is synchronous so this is either masking a bug in the kernel or just plain unnecessary.

I'm not particularly impressed by how this is all put together. The problem starts with the stupid kernel interface which requires ~50 lines of shell to bring up swap. That's just bizarre. But anyway, zram is useful, so we should make it easy to use, so I think it's worth to package this, so it'd be great to finally get this package into Fedora.

Comment 7 Raphael Groner 2017-08-14 07:03:04 UTC
(In reply to Zbigniew Jędrzejewski-Szmek from comment #6)
> > # need Linux kernel version 2.6.37.1 or better to use zram
> > Requires: kernel >= 2.6.37.1
> 
> First, nowadays kernel has been split into kernel-core and other
> subpackages, so you can easily boot a machine without kernel.rpm (which also
> pulls in kernel-modules). Second, the fact that you have a specific
> kernel.rpm installed does not mean that you're running with that version so
> this is not effective. And third, there's no way to install a kernel rpm for
> 2.6.x in any Fedora. So you should just drop this line.

OK. Good point about several kernels available to boot on a system.

> > BuildRequires: systemd-units
> 
> > Requires(post): systemd-sysv
> > Requires(post): systemd-units
> > Requires(preun): systemd-units
> > Requires(postun): systemd-units
> > Requires: systemd
> > Requires: initscripts
> 
> This should be replaced by
> %{?systemd_requires}
> BuildRequires: systemd
> [https://fedoraproject.org/wiki/Packaging:Scriptlets#Systemd]

OK

> > %systemd_postun_with_restart mkzram.service
> Hmm, that could be dangerous. Disabling swap during an upgrade could bring
> the machine down. %systemd_postun seems more reasonable.

OK

> > After=multi-user.target
> That's certainly the wrong place to order this. Not only it causes a
> dependency loop with swap.target which is ordered much earlier, it might
> also disable swap at the beginning of shutdown, crashing the machine.

Drop it?

> > sleep 1
> Hmm. swapoff is synchronous so this is either masking a bug in the kernel or
> just plain unnecessary.

Drop it?

> I'm not particularly impressed by how this is all put together. The problem
> starts with the stupid kernel interface which requires ~50 lines of shell to
> bring up swap. That's just bizarre. But anyway, zram is useful, so we should
> make it easy to use, so I think it's worth to package this, so it'd be great
> to finally get this package into Fedora.

How can they talk on reddit about available systemd integration without an official package? Is this review still needed?
https://www.reddit.com/r/Fedora/comments/4y6n5t/how_to_enable_zram_on_fedora_24/

What about storaged-zram? Is it stilled developed? I see it's part of general storaged, merged into systemd or somehow actual plans to merge.

Comment 8 Raphael Groner 2017-08-14 07:05:10 UTC
(In reply to František Zatloukal from comment #4)
> Packaging looks good to me.
> 
> I see upstream recommends disabling swap after installation but I don't see
> the package doing it. Is it an issue if swap remains enabled? It would make
> sense to at least print warning to the terminal during installation (I'd be
> against disabling swap in a completely automatic manner).

OK

> The other potential issue - I see documented incompatibility with encrypted
> /home . Does it break anything after installing this package to such system?

Please provide any link for more information and reproducibility.

Comment 9 Zbigniew Jędrzejewski-Szmek 2017-08-14 12:22:01 UTC
Oh, /usr/lib/systemd/system/zram.service is part of anaconda. It seems to be tailored for anaconda (see the part at the end about configuring /tmp).
It also has no support for enabling (hence the reddit thread).
So it seems that this package is still useful, and in the long run anaconda should switch to depending on it.


>>> After=multi-user.target
> Drop it?

Yes. I think the following should work:
[Unit]
DefaultDependencies=no
Before=swap.target
Conflicts=umount.target

[Install]
WantedBy=swap.target

>>> sleep 1
>> Hmm. swapoff is synchronous so this is either masking a bug in the kernel or
>> just plain unnecessary.
> Drop it?
Yeah. If anything breaks, there's a bug somewhere.

Comment 10 Zbigniew Jędrzejewski-Szmek 2017-08-14 12:25:31 UTC
Hmm, it seems that the anaconda service is smarter than the one being packaged here. Instead of making one swap device per cpu, it just does "echo $num_cpus > /sys/block/zram0/max_comp_streams". If this is updated, not much is left of the original, I don't know what you want to do with that.

Comment 11 Raphael Groner 2017-09-21 23:08:16 UTC
Thanks for your comments. Will update soonish, please be still patient.

Comment 12 Pavel Alexeev 2017-11-30 20:56:56 UTC
Raphael do you plan to continue?

Comment 13 Raphael Groner 2017-12-01 21:05:30 UTC
Less time available for development currently and so I can't promise anything, sorry. Do you have interest to take over for this request? We could close here and please feel free to take this as a base for your new request. You can add me as co-maintainer after imported the package.

Comment 14 Teppo Turtiainen 2018-07-13 18:29:48 UTC
Just fyi, seems like several people are currently interested in zram on Fedora:
https://wiki.gnome.org/Hackfests/Performance2018/Agenda
https://github.com/systemd/systemd/issues/8990
https://fedoraproject.org/wiki/Changes/ZRAMforARMimages

Comment 15 Peter Robinson 2018-07-18 15:47:09 UTC
I'm going to drive this, feel free to add feedback to the new review.

> https://wiki.gnome.org/Hackfests/Performance2018/Agenda
> https://github.com/systemd/systemd/issues/8990
> https://fedoraproject.org/wiki/Changes/ZRAMforARMimages

I was involved in all of the above.

*** This bug has been marked as a duplicate of bug 1602846 ***

Comment 16 Raphael Groner 2018-07-22 06:32:47 UTC
(In reply to Teppo Turtiainen from comment #14)
> Just fyi, seems like several people are currently interested in zram on
> Fedora:
> https://wiki.gnome.org/Hackfests/Performance2018/Agenda
> https://github.com/systemd/systemd/issues/8990
> https://fedoraproject.org/wiki/Changes/ZRAMforARMimages

See also bug #1469767.


Note You need to log in before you can comment on or make changes to this bug.