Bug#929557: linux: restore __kernel_fpu needed for zfs for AES-NI/AVX support [mainline not in debian yet]

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Bug#929557: linux: restore __kernel_fpu needed for zfs for AES-NI/AVX support [mainline not in debian yet]

Chris Zubrzycki-3
Source: linux
Severity: wishlist
Tags: upstream patch

Dear Maintainer,

GKH has been purging __kernel_fpu_{begin,end}() from all kernels
including LTS (4.19/5) and it's needed for AES-NI/AVX support in the zfs
package. If this commit makes it to debian it will break all older zfs
releases and will cause very high cpu and terrible speed with encrypted
zfs pools. Arch Linux has already reverted the offending commit in their
distro. Patch available at
https://raw.githubusercontent.com/Mic92/nixpkgs/7b77c27caa8617c82df5c5af6b9ce6ae010d7f9a/pkgs/os-specific/linux/kernel/export_kernel_fpu_functions.patch

Thank you!

-- System Information:
Debian Release: 9.9
  APT prefers stable-updates
  APT policy: (500, 'stable-updates'), (500, 'stable')
Architecture: amd64 (x86_64)

Kernel: Linux 4.19.0-0.bpo.4-amd64 (SMP w/32 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8) (ignored: LC_ALL set to en_US.UTF-8), LANGUAGE=en_US.UTF-8 (charmap=UTF-8) (ignored: LC_ALL set to en_US.UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: sysvinit (via /sbin/init)




-chris zubrzycki
- --
PGP ID: 0xA2ABC070
Fingerprint: 26B0 BA6B A409 FA83 42B3  1688 FBF9 8232 A2AB C070
========================================================

  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot

Reply | Threaded
Open this post in threaded view
|

Bug#929557: linux: restore __kernel_fpu needed for zfs for AES-NI/AVX support [mainline not in debian yet]

Bastian Blank-5
Hi Chris

On Sat, May 25, 2019 at 10:03:07PM -0400, Chris Zubrzycki wrote:
> GKH has been purging __kernel_fpu_{begin,end}() from all kernels
> including LTS (4.19/5) and it's needed for AES-NI/AVX support in the zfs
> package.

The commit also tells you why this was done.  Please bring this up to
upstream and Greg, not to us.

>            Arch Linux has already reverted the offending commit in their
> distro. Patch available at
> https://raw.githubusercontent.com/Mic92/nixpkgs/7b77c27caa8617c82df5c5af6b9ce6ae010d7f9a/pkgs/os-specific/linux/kernel/export_kernel_fpu_functions.patch

How is this change not a copyright violation?

Regards,
Bastian

--
Punishment becomes ineffective after a certain point.  Men become insensitive.
                -- Eneg, "Patterns of Force", stardate 2534.7

Reply | Threaded
Open this post in threaded view
|

Bug#929557: linux: restore __kernel_fpu needed for zfs for AES-NI/AVX support [mainline not in debian yet]

Ben Hutchings-3
Control: tag -1 wontfix

On Sun, 2019-05-26 at 14:00 +0200, Bastian Blank wrote:

> Hi Chris
>
> On Sat, May 25, 2019 at 10:03:07PM -0400, Chris Zubrzycki wrote:
> > GKH has been purging __kernel_fpu_{begin,end}() from all kernels
> > including LTS (4.19/5) and it's needed for AES-NI/AVX support in the zfs
> > package.
>
> The commit also tells you why this was done.  Please bring this up to
> upstream and Greg, not to us.
>
> >            Arch Linux has already reverted the offending commit in their
> > distro. Patch available at
> > https://raw.githubusercontent.com/Mic92/nixpkgs/7b77c27caa8617c82df5c5af6b9ce6ae010d7f9a/pkgs/os-specific/linux/kernel/export_kernel_fpu_functions.patch
>
> How is this change not a copyright violation?
It's reverting an upstream commit that doesn't mention anything about
copyright, but I can see that there was an odd inconsistency of GPL-
only status with the preempt-disabling versions.

If the upstream developers consider a feature to be so low-level that
modules shouldn't need it at all, it implies that "the function is
considered an internal implementation issue", which is the main reason
documented for using EXPORT_SYMBOL_GPL.

Debian tries to avoid breaking changes during a stable release or pre-
release freeze, and I would certainly considering restoring exports in
order to do that.  We even have long-standing patches that add exports
to support modular builds of aufs and some Android drivers.  However,
we defer to the upstream developers by only using EXPORT_SYMBOL_GPL
when we do that - which would not help ZoL.

So I agree with Bastian that you will need to persuade upstream to
change this.

Ben.

--
Ben Hutchings
73.46% of all statistics are made up.


signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Bug#929557: Please revert LTS kernel change that will break ZFS for Buster point releases

Mo Zhou
In reply to this post by Chris Zubrzycki-3
control: severity -1 grave

Dear kernel maintainers,

Buster will be released with 4.19.37 kernel. That's fine
and it doesn't break ZFS. However, the changes introduced
in 4.19.38 and linux 5.0 break ZFS. That means the current
0.7.12-2 will fail to build everywhere after the first
Buster point release (with kernel version bump).
A foreseeable stable RC is grave enough.

Upstream has made a compromise to disable SIMD for kernels
that received the breaking change:
 
https://github.com/zfsonlinux/zfs/commit/e22bfd814960295029ca41c8e116e8d516d3e730

Based on these, we Debian ZoL maintainers have several
possible choices:

1. unblock 0.7.13-1 (see rmadison -S zfs-linux)
   (it contains the above patch)
2. revert debdiff(0.7.12-2,0.7.12-5), apply the upstream
   patch and upload 0.7.12-6 through t-p-u.
3. ask kernel maintainers to revert problematic commits

Freeze policy makes it difficult for the release team to
accept solution 1. Solution 2 is able to eliminate bugs
but I doubt how useful a SIMD-less ZFS is. Compared to
the others, solution 3 is the best solution because there
won't be any future breakage or significant performance loss.

My position is solution 3, as it's the ***LTS KERNEL UPDATE***
that introduced breaking change breaks ZFS 0.7.12-2.
It's not a bug of ZFS 0.7.12-2 at all.

Debian ZFS users are sensitive because I and Aron often
receive private user reports and prodding mails. That means
reverting the problematic kernel commit is beneficial to
our users. Our priorities are our users and free software,
NOT to stick to the problematic breaking LTS update.

I believe this is a kernel bug. Instead of submitting
a grave RC for the 10.1 release, we'd better sort it out
right now before the Buster release.

Thanks,
Mo

Reply | Threaded
Open this post in threaded view
|

Bug#929557: Please revert LTS kernel change that will break ZFS for Buster point releases

Bastian Blank
Control: severity -1 wishlist

On Mon, Jun 03, 2019 at 06:39:39PM -0700, Mo Zhou wrote:
> I believe this is a kernel bug. Instead of submitting
> a grave RC for the 10.1 release, we'd better sort it out
> right now before the Buster release.

We already stated that we wont change it by marking this bug as wontfix.
We also told youd that it is your turn to pursuade upstream.

The export of this low-level functions was revoked because it was often
used in a wrong way.  The safe alternative is still exported, however
marked as gpl-only.  It is up to you to pursuade upstream to change the
still existing export, which should be possible, but this needs do be
done by you.  After upstream applied it we can cherry pick this chane.

Regards,
Bastian

--
I have never understood the female capacity to avoid a direct answer to
any question.
                -- Spock, "This Side of Paradise", stardate 3417.3

Reply | Threaded
Open this post in threaded view
|

Bug#929557: additional point of view

Geert Stappers
In reply to this post by Chris Zubrzycki-3
An additional point of view. Prespective is Linux Kernel developer.

OOT means  Out of Tree.
And the tree is the Linux source code directory tree.


----- Forwarded message from "Enrico Weigelt, metux IT consult" <[hidden email]> -----

Date: Wed, 29 May 2019 13:01:09 +0200
From: "Enrico Weigelt, metux IT consult" <[hidden email]>
To: Dan <[hidden email]>, [hidden email], [hidden email]
Subject: Re: ZFS in Buster
User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1

On 28.05.19 18:43, Dan wrote:

> Sadly the Linux Kernel has introduced a commit in kernel 4.19 and 5.0> that prevents ZFS from using SIMD. The result is that ZFS won't be>
usable in Buster. See the following issue>
https://github.com/zfsonlinux/zfs/issues/8793
We recently had this discussion on lkml - yet another case of 3rdparty
folks that just don't follow the license rules.

It's not the kernel who broke zfs, it's zfs that broke itself. The
kernel is GPL, and they just have to follow the rules or go away.

OOT modules are conceptionally messy in the first place. It's sometimes
okay as an temporary workaround, until things get mainlined. But
intentionally keeping things oot for long time is just silly and creates
lots of more problems than it creates.

And they're even using now *deeply* arch-internal functions directly.

> NixOS reverted that particular commit:>
https://www.phoronix.com/scan.php?page=news_item&px=NixOS-Linux-5.0-ZFS-FPU-Drop
Intentional license violation. Not funny.

> Debian is the "Universal Operating System" and gives the user the> option to choose. It provides "vim and emacs", "Gnome and KDE",
If you wanna have something new included, you'll have to sit down and do
the actual work. In the end of the day, it's that simple.

> Would it be possible to provide an alternative patched linux kernel
> that works with ZFS?

You mean patching against the license ?

> The ZFS developers proposed the Linux developers to rewrite the whole
> ZFS code and use GPL, but surprisingly the linux developers didn't
> accept. See below:
> https://github.com/zfsonlinux/zfs/issues/8314

Wait, no. It's not that we refused anything (actually, I don't even
recall any decent discussion on that @lkml). There even wasn't anything
to accept or refuse - except the existing code, that is nowhere near
a quality where any maintainer likes to even have a closer look at.

The major problem is that ZoL always has been oot on purpose, which is
the wrong approach to begin with. That also leads to bad code quality
(eg. lots of useless wrappers, horrible maintenance, ...)

What ZoL folks could do is step by step rewrite it to use mainline
functionality where ever technically feasible and work closely with
upstream to introduce missing functionality. Obviously, their current
proprietary userland interface can't be accepted for mainline - it
has to be reworked to be conformant w/ standard uapi (eg. we already
have it for things like snapshots, deduplication, quotas, ...)

But it's up to ZoL developers to do the actual work and post patches
to lkml. There won't be anybody else doing that.


--mtx

--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[hidden email] -- +49-151-27565287


----- End forwarded message -----

Groeten
Geert Stappers
--
Leven en laten leven

Reply | Threaded
Open this post in threaded view
|

Bug#929557: Some Thoughts

Sam Hartman-3
In reply to this post by Chris Zubrzycki-3

The linux Kernel introduced an upstream commit designed to remove an
interface that was being misused.

That does not meet the kind of requirements for changes that we (Debian)
make in stable releases.
If I filed an unblock for krb5 to remove an interface at this point in
the release process it would be outright refused.

Even if krb5 had multiple levels of visibility and even if I filed an
unblock to change interface visibility it would be refused.

Even if I filed an unblock to change interface visibility to prevent
other people from violating the krb5 license it would be refused.

We are more permissive in what changes we accept from the kernel team.
There are a lot of reasons for that.  But I think it's more that we're
changing the default for changes from the kernel team, than that we
necessarily want the kernel team to be more able to break other software
in stable than other packages.

That is, if we had the resources to review the changes adequately and do
adequate testing, I actually suspect we would hold the kernel to the
same standards we hold other packages to.

I do not think this particular change would meet those standards for
buster.
I think this change clearly meets those standards for unstable.
And yet, as I said above, we do hold the kernel to different standards
for stable releases for a variety of reasons.
However, even if the default is that we are more permissive with the
kernel, we can review the complex cases on a case by case basis.  And we
should review that based on what is best for Debian, not what is best
for the kernel team.

I'm not saying the kernel team should revert the commit.
I'm saying that the issue is far more complex than has been outlined in
this bug so far.

I think that the kernel team and the ZOL maintainers should work with
the stable release team for buster to figure out which changes are
permissible.
Ultimately I'd expect that the stable release team will get to decide
which changes they want in buster and I hope that the kernel team and
the ZOL maintainers will work with that.

Reply | Threaded
Open this post in threaded view
|

Bug#929557: Some Thoughts

Chris Zubrzycki-3
In reply to this post by Chris Zubrzycki-3
On Thu, 06 Jun 2019 12:19:03 -0400 Sam Hartman <[hidden email]> wrote:
>
> The linux Kernel introduced an upstream commit designed to remove an
> interface that was being misused.

No, it was removed as part of code cleanup. There was no “misuse”. It was added for KVM and existed for 7 years. KVM hasn’t needed it for a while, so patch was submitted to remove the exported function *only* leaving the GPL exported version. GKH went ahead and violated the kernel procedures and removed the symbols from LTS kernels as well. LTS is not where code cleanup should be done and symbols removed.


> That does not meet the kind of requirements for changes that we (Debian)
> make in stable releases.
> If I filed an unblock for krb5 to remove an interface at this point in
> the release process it would be outright refused.

Except this is to *not* remove an interface from LTS kernels. See above.

 
> We are more permissive in what changes we accept from the kernel team.

And yet you are trying to stick to the rules unlike the kernel team…


> That is, if we had the resources to review the changes adequately and do
> adequate testing, I actually suspect we would hold the kernel to the
> same standards we hold other packages to.

The symbol was exported without issue for 7 years. It’s been tested.
 
> I do not think this particular change would meet those standards for
> buster.

I thought LTS was supposed to be stable and not have symbols removed?

> I'm not saying the kernel team should revert the commit.
> I'm saying that the issue is far more complex than has been outlined in
> this bug so far.
>
> I think that the kernel team and the ZOL maintainers should work with
> the stable release team for buster to figure out which changes are
> permissible.
> Ultimately I'd expect that the stable release team will get to decide
> which changes they want in buster and I hope that the kernel team and
> the ZOL maintainers will work with that.

Unfortunately GKH has put his foot down with this change and insists the symbols be exported GPL only for 5.x and above. He doesn’t care much about other open source projects:
"My tolerance for ZFS is pretty non-existant. “ https://marc.info/?l=linux-kernel&m=154714516832389 except he is doing extra work to cause problems with ZFS by removing exported symbols from LTS kernels against policy.

See the pattern? No ZFS on linux user is happy about the 5.x change, but there is no valid reason to remove it from 4.x.

Also. there is no copyright violation reversing a commit, as it’s still released GPL code. GPL code is still GPL code even if it’s not in a current release. That would be interesting if code lost it’s copyright once it wasn’t being shipped.


-chris zubrzycki
- --
PGP ID: 0xA2ABC070
Fingerprint: 26B0 BA6B A409 FA83 42B3  1688 FBF9 8232 A2AB C070
========================================================

"Twice blessed is help unlooked for." --Tolkien