Page MenuHomeFreeBSD
Differential D26218

Integrate zstd into boot loader
ClosedPublic
Actions

Authored by imp on Aug 28 2020, 11:46 AM.
Tags
None
Referenced Files
F81886171: D26218.id78139.diff
Wed, Nov 27, 10:51 PM
Unknown Object (File)
Mon, Nov 25, 5:46 AM
Unknown Object (File)
Mon, Nov 25, 4:54 AM
Unknown Object (File)
Mon, Nov 25, 4:38 AM
Unknown Object (File)
Thu, Nov 14, 6:43 AM
Unknown Object (File)
Apr 8 2024, 1:34 AM
Unknown Object (File)
Apr 7 2024, 8:57 PM
Unknown Object (File)
Mar 29 2024, 3:51 PM
View All Files
Subscribers
bcran
delphij
freqlabs

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 33219
Build 30562: 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
imp updated this revision to Diff 76303.Aug 28 2020, 12:01 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
imp updated this revision to Diff 76304.Aug 28 2020, 12:05 PM

this isn't needed, remove it.

Harbormaster completed remote builds in B33209: Diff 76304.Aug 28 2020, 12:06 PM
imp updated this revision to Diff 76305.Aug 28 2020, 12:09 PM

this isn't needed either...

Harbormaster completed remote builds in B33210: Diff 76305.Aug 28 2020, 12:09 PM
imp updated this revision to Diff 76306.Aug 28 2020, 12:13 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.

freqlabs added a subscriber: freqlabs.Aug 28 2020, 5:06 PM

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.

imp updated this revision to Diff 76317.Aug 28 2020, 7:01 PM

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
imp updated this revision to Diff 76318.Aug 28 2020, 7:01 PM

rebase

Harbormaster completed remote builds in B33219: Diff 76318.Aug 28 2020, 7:01 PM
imp updated this revision to Diff 77468.Sep 24 2020, 6:12 AM

Rebase

Harbormaster completed remote builds in B33789: Diff 77468.Sep 24 2020, 6:12 AM
imp updated this revision to Diff 77469.Sep 24 2020, 6:16 AM

Rebase (again!)

Harbormaster completed remote builds in B33790: Diff 77469.Sep 24 2020, 6:16 AM
imp updated this revision to Diff 77553.Sep 25 2020, 8:52 PM

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.

imp updated this revision to Diff 77619.Sep 29 2020, 3:47 AM
imp edited the summary of this revision. (Show Details)

Update, per reviews from the PR

Harbormaster completed remote builds in B33868: Diff 77619.Sep 29 2020, 3:47 AM
imp updated this revision to Diff 78121.Oct 12 2020, 6:02 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
imp updated this revision to Diff 78122.Oct 12 2020, 6:05 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
imp updated this revision to Diff 78139.Oct 12 2020, 5:06 PM

rebase, and some last second refinement.

Harbormaster completed remote builds in B34117: Diff 78139.Oct 12 2020, 5:06 PM
mmacy added a comment.Oct 12 2020, 6:28 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.

imp updated this revision to Diff 78143.Oct 12 2020, 7:55 PM

update with matt's suggestion

Harbormaster completed remote builds in B34119: Diff 78143.Oct 12 2020, 7:55 PM
imp updated this revision to Diff 78144.Oct 12 2020, 8:11 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..

Revision Contents
Changeset List

PathSize
stand/
3 lines
efi/
boot1/
1 line
libsa/
zfs/
22 lines
3 lines
54 lines
sys/
cddl/
boot/
zfs/
54 lines
7 lines
contrib/
opensolaris/
uts/
common/
os/
contrib/
openzfs/
include/
os/
freebsd/
linux/
2 lines
spl/
rpc/
4 lines
sys/
2 lines
2 lines
4 lines
9 lines
1 line
3 lines
1 line
3 lines
2 lines
sys/
2 lines
7 lines
lib/
libspl/
include/
os/
freebsd/
sys/
6 lines
module/
os/
freebsd/
spl/
4 lines
zstd/
14 lines
sys/
28 lines
11 lines

Diff 76318

View Options

stand/defs.mk

Loading...
View Options

stand/efi/boot1/Makefile

Loading...
View Options

stand/libsa/zfs/Makefile.inc

Loading...
View Options

stand/libsa/zfs/zfsimpl.c

Loading...
View Options

stand/libsa/zfs/zstd_shim.c

Loading...
View Options

sys/cddl/boot/zfs/zfsimpl.h

Loading...
View Options

sys/cddl/boot/zfs/zfssubr.c

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

sys/sys/systm.h

Loading...
View Options

sys/sys/types.h

Loading...