Details

Reviewers

tsoome
mmacy

Commits

rS366657: Add zstd support to the boot loader.

Summary

Integrate zstd support into the boot loader, from earlier work by Matt Macey.

Test Plan

Make sure the last minutes shuffling I did didn't break the kernel and/or userland
builds.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 33218
Build 30561: arc lint + arc unit

Event Timeline

imp created this revision.Aug 28 2020, 11:46 AM

Herald added a subscriber: delphij. · View Herald TranscriptAug 28 2020, 11:46 AM

imp requested review of this revision.Aug 28 2020, 11:46 AM

Harbormaster completed remote builds in B33207: Diff 76302.Aug 28 2020, 11:46 AM

imp mentioned this in D26207: Add zfs zstd support to loader.Aug 28 2020, 11:52 AM

imp retitled this revision from Make this compile to Integrate zstd into boot loader.Aug 28 2020, 12:00 PM

Last second tweaks that I missed the need for before a full build.

Harbormaster completed remote builds in B33208: Diff 76303.Aug 28 2020, 12:01 PM

this isn't needed, remove it.

Harbormaster completed remote builds in B33209: Diff 76304.Aug 28 2020, 12:06 PM

this isn't needed either...

Harbormaster completed remote builds in B33210: Diff 76305.Aug 28 2020, 12:09 PM

Ah, just use the standard definition of offsetof.

Harbormaster completed remote builds in B33211: Diff 76306.Aug 28 2020, 12:13 PM

imp added inline comments.Aug 28 2020, 3:05 PM

include/stddef.h
75 ↗	(On Diff #76302)	Hmmm, this one might not be right... I'll have to look later today...
stand/libsa/zfs/zstd_shim.c
37	If we get the other issues with boolean_t resolved, this can be eliminated.

Generally happy about the cleanup and the added flexibility in the build infrastructure.

Changes to the ifdefs/if defined could use a pass to check for consistency of the comments on the closing endif.

sys/contrib/openzfs/module/os/freebsd/spl/list.c
30	This is normally included on the command line. I don't think we should start being inconsistent about that here.
sys/contrib/openzfs/module/zstd/zfs_zstd.c
46	Is this order important or can the zio_compress.h include just be pulled out of the `if` and avoid the `else`?
84	This seems like a weird place to do this.

imp added inline comments.Aug 28 2020, 6:23 PM

sys/contrib/openzfs/module/os/freebsd/spl/list.c
30	It's quite unusual wrt rest of FreeBSD and the boot loader to do this for anything but the global #define options in the kernel...
sys/contrib/openzfs/module/zstd/zfs_zstd.c
46	Unsure. I'll try it.
84	Yea, I didn't want to get into the tangled mess of where it's defined, but that may be a fair bit of feedback.

feedback...

Herald added a subscriber: bcran. · View Herald TranscriptAug 28 2020, 7:01 PM

Harbormaster completed remote builds in B33218: Diff 76317.Aug 28 2020, 7:01 PM

rebase

Harbormaster completed remote builds in B33219: Diff 76318.Aug 28 2020, 7:01 PM

Rebase

Harbormaster completed remote builds in B33789: Diff 77468.Sep 24 2020, 6:12 AM

Rebase (again!)

Harbormaster completed remote builds in B33790: Diff 77469.Sep 24 2020, 6:16 AM

rebase

Harbormaster completed remote builds in B33833: Diff 77553.Sep 25 2020, 8:52 PM

imp edited the summary of this revision. (Show Details)Sep 25 2020, 8:55 PM

imp added reviewers: tsoome, mmacy.

freqlabs added inline comments.Sep 25 2020, 9:09 PM

sys/contrib/openzfs/lib/libspl/include/os/freebsd/sys/param.h
48–50	This change from ifndef to ifdef is concerning. What's going on here?

imp added inline comments.Sep 25 2020, 9:10 PM

sys/contrib/openzfs/lib/libspl/include/os/freebsd/sys/param.h
48–50	That's likely a mistake. I'll bet it can be removed entirely.

Update, per reviews from the PR

Harbormaster completed remote builds in B33868: Diff 77619.Sep 29 2020, 3:47 AM

Rebase and rework after comments from the OpenZFS upstreaming
to reduce diffs in upstream files. I was able to flesh out
the _STANDALONE support in the spl for FreeBSD instead of
some surgery in upstream files.

Harbormaster completed remote builds in B34106: Diff 78121.Oct 12 2020, 6:02 AM

update atomic.h to fully neutered for STANDALONE version, save for
atomic_add_uint64

Harbormaster completed remote builds in B34107: Diff 78122.Oct 12 2020, 6:05 AM

rebase, and some last second refinement.

Harbormaster completed remote builds in B34117: Diff 78139.Oct 12 2020, 5:06 PM

Once you can get ZTS to pass it LGTM.

stand/libsa/zfs/Makefile.inc
32	Clever.
stand/libsa/zfs/zfsimpl.c
132	Remove this one if you're going to move it up.

update with matt's suggestion

Harbormaster completed remote builds in B34119: Diff 78143.Oct 12 2020, 7:55 PM

One more nit before upstreaming. Down to 2 defines in the shim.

Harbormaster completed remote builds in B34120: Diff 78144.Oct 12 2020, 8:11 PM

imp added inline comments.Oct 12 2020, 8:15 PM

stand/libsa/zfs/Makefile.inc
32	thanks.
stand/libsa/zfs/zfsimpl.c
132	nah, it was added independently of me, so I'll just delete the one I'm adding.

This revision was not accepted when it landed; it landed in state Needs Review.Oct 12 2020, 10:19 PM

Closed by commit rS366657: Add zstd support to the boot loader. (authored by imp). · Explain Why

This revision was automatically updated to reflect the committed changes.

imp added a commit: rS366657: Add zstd support to the boot loader..

Integrate zstd into boot loader
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 76317

stand/defs.mk

stand/efi/boot1/Makefile

stand/libsa/zfs/Makefile.inc

stand/libsa/zfs/zfsimpl.c

stand/libsa/zfs/zstd_shim.c

sys/cddl/boot/zfs/zfsimpl.h

sys/cddl/boot/zfs/zfssubr.c

sys/cddl/contrib/opensolaris/uts/common/os/list.c

sys/contrib/openzfs/include/os/freebsd/linux/compiler.h

sys/contrib/openzfs/include/os/freebsd/spl/rpc/xdr.h

sys/contrib/openzfs/include/os/freebsd/spl/sys/atomic.h

sys/contrib/openzfs/include/os/freebsd/spl/sys/byteorder.h

sys/contrib/openzfs/include/os/freebsd/spl/sys/ccompile.h

sys/contrib/openzfs/include/os/freebsd/spl/sys/cmn_err.h

sys/contrib/openzfs/include/os/freebsd/spl/sys/condvar.h

sys/contrib/openzfs/include/os/freebsd/spl/sys/kmem.h

sys/contrib/openzfs/include/os/freebsd/spl/sys/kstat.h

sys/contrib/openzfs/include/os/freebsd/spl/sys/proc.h

sys/contrib/openzfs/include/os/freebsd/spl/sys/sysmacros.h

sys/contrib/openzfs/include/sys/zfs_context.h

sys/contrib/openzfs/include/sys/zio_compress.h

sys/contrib/openzfs/lib/libspl/include/os/freebsd/sys/param.h

sys/contrib/openzfs/module/os/freebsd/spl/list.c

sys/contrib/openzfs/module/zstd/zfs_zstd.c

sys/sys/systm.h

sys/sys/time.h

sys/sys/types.h

Integrate zstd into boot loaderClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 76317

stand/defs.mk

stand/efi/boot1/Makefile

stand/libsa/zfs/Makefile.inc

stand/libsa/zfs/zfsimpl.c

stand/libsa/zfs/zstd_shim.c

sys/cddl/boot/zfs/zfsimpl.h

sys/cddl/boot/zfs/zfssubr.c

sys/cddl/contrib/opensolaris/uts/common/os/list.c

sys/contrib/openzfs/include/os/freebsd/linux/compiler.h

sys/contrib/openzfs/include/os/freebsd/spl/rpc/xdr.h

sys/contrib/openzfs/include/os/freebsd/spl/sys/atomic.h

sys/contrib/openzfs/include/os/freebsd/spl/sys/byteorder.h

sys/contrib/openzfs/include/os/freebsd/spl/sys/ccompile.h

sys/contrib/openzfs/include/os/freebsd/spl/sys/cmn_err.h

sys/contrib/openzfs/include/os/freebsd/spl/sys/condvar.h

sys/contrib/openzfs/include/os/freebsd/spl/sys/kmem.h

sys/contrib/openzfs/include/os/freebsd/spl/sys/kstat.h

sys/contrib/openzfs/include/os/freebsd/spl/sys/proc.h

sys/contrib/openzfs/include/os/freebsd/spl/sys/sysmacros.h

sys/contrib/openzfs/include/sys/zfs_context.h

sys/contrib/openzfs/include/sys/zio_compress.h

sys/contrib/openzfs/lib/libspl/include/os/freebsd/sys/param.h

sys/contrib/openzfs/module/os/freebsd/spl/list.c

sys/contrib/openzfs/module/zstd/zfs_zstd.c

sys/sys/systm.h

sys/sys/time.h

sys/sys/types.h

Integrate zstd into boot loader
ClosedPublic
Actions

Revision Contents
Changeset List