Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should we check the kernel version? #55

Closed
kakra opened this issue Jan 21, 2018 · 6 comments
Closed

Should we check the kernel version? #55

kakra opened this issue Jan 21, 2018 · 6 comments

Comments

@kakra
Copy link
Contributor

kakra commented Jan 21, 2018

Should bees refuse to run with known bad kernel interactions? And maybe print a warning about known minor problems with some kernels?

@Zygo
Copy link
Owner

Zygo commented Jan 22, 2018

Refuse? No, then bees would never run. I'm pulling patches from v4.16 integration branches (two releases ahead of the current release as of this writing), and if bees refused to run due to any known bug, nobody who wasn't building their own kernels off git.kernel.org would be able to run bees at all.

I'm hesitant to add warnings because there is a lot of room for error.

It would be hard to know if the kernels had been patched based on the uts version strings alone. Old stable kernels get bug fixes sometimes years later, and I've used a v4.1 kernel for some time with a lot of patches backported as far as from v4.6). Even the LTS kernels are a roiling cauldron of random btrfs patches.

The extra testing required to keep the warning versions list accurate (with neither false positives nor negatives) would be a large time sink for me. It might not be a time sink for a contributor with access to a larger pool of test users/machines to collect data from, but I'd have to rely on such a contributor entirely. A distro maintainer could easily maintain a kernel version blacklist and put it in the bees startup scripts--they know what patches are in their kernels, and I have a (mostly) complete list of kernel bugs to look for.

There's diminishing value and increasing cost in maintaining support for older kernels. For kernels between v4.4 and v4.10, bees avoided kernel bugs with a delicate balance between the scan/dedup algorithm and thread behavior that is difficult to preserve without extensive testing feedback (master is probably OK today with -c1 but it would take a few machine-weeks of testing to know).

These days I've already stopped testing on kernels before v4.11 (sadly that includes the LTS v4.9 as it is missing some performance bug fixes that don't meet -stable criteria). I am currently considering whether and when to drop support for kernels prior to v4.14 simply because v4.14 doesn't need the older-kernel workarounds any more. The fact that v4.14 also introduces LOGICAL_INO_V2 makes it even more attractive as a minimum kernel version.

I guess another thing I'm considering is adopting Linux kernel version numbers for bees as btrfs-tools does. That way I could say something like "if you're using kernel v4.7, use bees v4.7" and not have to worry as much about breaking older kernel versions in newer bees versions. (The obvious plan with this problem is that it hasn't really worked that way for btrfs-tools either).

@kakra
Copy link
Contributor Author

kakra commented Jan 22, 2018

Okay, so I'd put a warning into the Gentoo ebuild script to either use the latest kernel or carefully read the README about known bugs.

But my request was more about grave bugs, like the README says "don't try to use with kernel older than 4.xy - it won't end well".

@Zygo
Copy link
Owner

Zygo commented Jan 23, 2018

Before v4.2 there are missing ioctls or the ioctls have restrictions that have since been removed. Bees will fail to do anything useful at all on kernels before v3.16, but it should be mostly inert. Bees will operate inefficiently (it won't be able to handle the last extent of files with non-4K-aligned EOF) until v4.2.

Data corruption and hangs happen from v4.2 to v4.11, except for backported fixes to v4.4 and v4.9. Some of these are related to dedup ioctls, but others are general btrfs problems (i.e. you should not run btrfs on these kernels with some combinations of features, e.g. compression and inline extents).

v4.11 to v4.13 have a crippling performance issue--a risk of temporary kernel CPU lockups for 20 minutes at a time every few days or weeks. Earlier kernels had this too but it was even worse.

v4.14 has thousands of WARN_ON(ref->count < 0) stack traces per day. The fix for those won't land in Linux until at least v4.16, so v4.15 will have the same problem.

Which kernel should be the cutoff? I'd say I'm still waiting for that kernel to appear.

@kakra
Copy link
Contributor Author

kakra commented Jan 23, 2018

Okay, understood. That makes sense.

kakra added a commit to kakra/bees that referenced this issue Feb 3, 2018
Add checking the kernel versions and write some info and/or warnings
before building and installing the package. Running bees on older
kernels may have some serious performance and stability impacts, let's
tell the user about it.

Closes Zygo#55

Signed-off-by: Kai Krakow <kai@kaishome.de>
kakra added a commit to kakra/bees that referenced this issue Feb 3, 2018
Add checking the kernel versions and write some info and/or warnings
before building and installing the package. Running bees on older
kernels may have some serious performance and stability impacts, let's
tell the user about it.

Closes Zygo#55

Signed-off-by: Kai Krakow <kai@kaishome.de>
kakra added a commit to kakra/bees that referenced this issue Feb 3, 2018
Add checking the kernel versions and write some info and/or warnings
before building and installing the package. Running bees on older
kernels may have some serious performance and stability impacts, let's
tell the user about it.

Closes Zygo#55

Signed-off-by: Kai Krakow <kai@kaishome.de>
kakra added a commit to kakra/bees that referenced this issue Feb 3, 2018
Add checking the kernel versions and write some info and/or warnings
before building and installing the package. Running bees on older
kernels may have some serious performance and stability impacts, let's
tell the user about it.

Closes Zygo#55

Signed-off-by: Kai Krakow <kai@kaishome.de>
kakra added a commit to kakra/bees that referenced this issue Feb 4, 2018
Add checking the kernel versions and write some info and/or warnings
before building and installing the package. Running bees on older
kernels may have some serious performance and stability impacts, let's
tell the user about it.

Closes Zygo#55

Signed-off-by: Kai Krakow <kai@kaishome.de>
kakra added a commit to kakra/bees that referenced this issue Feb 19, 2018
Add checking the kernel versions and write some info and/or warnings
before building and installing the package. Running bees on older
kernels may have some serious performance and stability impacts, let's
tell the user about it.

Closes Zygo#55

Signed-off-by: Kai Krakow <kai@kaishome.de>
kakra added a commit to kakra/bees that referenced this issue Mar 7, 2018
Add checking the kernel versions and write some info and/or warnings
before building and installing the package. Running bees on older
kernels may have some serious performance and stability impacts, let's
tell the user about it.

Closes Zygo#55

Signed-off-by: Kai Krakow <kai@kaishome.de>
@Zygo
Copy link
Owner

Zygo commented May 30, 2018

4.14.29 has all but one known bug fixed. The last bug is data corruption on compressed data read near a hole (a new one, different from a similar bug that I fixed last year). bees can trigger it because bees punches a lot of holes, especially in MS Windows disk image dumps.

There are still crippling performance problems with many reflinks to the same extent even in recent kernels; however, the workarounds currently in place in bees seem to mitigate the problem adequately.

@kakra
Copy link
Contributor Author

kakra commented May 30, 2018

Okay, let me see that I can add additional info to the Gentoo ebuild soon.

kakra added a commit to kakra/bees that referenced this issue Jul 16, 2018
Add checking the kernel versions and write some info and/or warnings
before building and installing the package. Running bees on older
kernels may have some serious performance and stability impacts, let's
tell the user about it.

Closes Zygo#55

Signed-off-by: Kai Krakow <kai@kaishome.de>
kakra added a commit to kakra/bees that referenced this issue Sep 8, 2018
This commit squashes all the little changes from the previous
integration branch into one, adjusts to the new Makefile changes, and
introduces an overlay layout so that the contrib/gentoo-bees subtree
can be directly added as a Portage overlay to the system.

The following list contains the previous commit descriptions:

sys-fs/bees: Keyword tested architecture ~amd64

    Bees was tested on this platform.

sys-fs/bees: Add kernel version checks

    Add checking the kernel versions and write some info and/or warnings
    before building and installing the package. Running bees on older
    kernels may have some serious performance and stability impacts, let's
    tell the user about it.

    Closes Zygo#55

sys-fs/bees: Add metadata.xml

sys-fs/bees: There's no configure script

    So, there's no point in calling "default".

sys-fs/bees: Simplify src_configure()

sys-fs/bees: Don't depend on markdown

    It makes no sense to install both README.md and README.html, and we can
    get rid of one dependency.

Dependencies: btrfs-progs is no longer a buildtime-only dep

    It is actually needed by the bees service wrapper script, as pointed out
    by Gentoo QA review.

sys-fs/bees: DOCS is not needed

    "COPYING" is already covered by the licensing. The ebuild defaults
    already include README*

sys-fs/bees: Make warnings exclusive

    It was recommended by Gentoo QA to show only either one or another
    warning, and change the texts accordingly.

sys-fs/bees: RDEPEND is not implicit

    RDEPEND does not implicitly default to DEPEND. Let's explicitly set the
    variable.

sys-fs/bees: IUSE=test is only needed for explicit dependencies

    Thus, remove it.

Signed-off-by: Kai Krakow <kai@kaishome.de>
kakra added a commit to kakra/bees that referenced this issue Sep 8, 2018
This commit squashes all the little changes from the previous
integration branch into one, adjusts to the new Makefile changes, and
introduces an overlay layout so that the contrib/gentoo-bees subtree
can be directly added as a Portage overlay to the system.

The following list contains the previous commit descriptions:

sys-fs/bees: Keyword tested architecture ~amd64

    Bees was tested on this platform.

sys-fs/bees: Add kernel version checks

    Add checking the kernel versions and write some info and/or warnings
    before building and installing the package. Running bees on older
    kernels may have some serious performance and stability impacts, let's
    tell the user about it.

    Closes Zygo#55

sys-fs/bees: Add metadata.xml

sys-fs/bees: There's no configure script

    So, there's no point in calling "default".

sys-fs/bees: Simplify src_configure()

sys-fs/bees: Don't depend on markdown

    It makes no sense to install both README.md and README.html, and we can
    get rid of one dependency.

Dependencies: btrfs-progs is no longer a buildtime-only dep

    It is actually needed by the bees service wrapper script, as pointed out
    by Gentoo QA review.

sys-fs/bees: DOCS is not needed

    "COPYING" is already covered by the licensing. The ebuild defaults
    already include README*

sys-fs/bees: Make warnings exclusive

    It was recommended by Gentoo QA to show only either one or another
    warning, and change the texts accordingly.

sys-fs/bees: RDEPEND is not implicit

    RDEPEND does not implicitly default to DEPEND. Let's explicitly set the
    variable.

sys-fs/bees: IUSE=test is only needed for explicit dependencies

    Thus, remove it.

Signed-off-by: Kai Krakow <kai@kaishome.de>
@Zygo Zygo closed this as completed in 7887747 Sep 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants