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
Unknown Object (File)
Fri, Nov 15, 2:18 PM
Unknown Object (File)
Wed, Nov 13, 3:23 AM
Unknown Object (File)
Tue, Nov 5, 5:30 AM
Unknown Object (File)
Sun, Oct 27, 6:14 PM
Unknown Object (File)
Oct 20 2024, 3:37 PM
Unknown Object (File)
Oct 20 2024, 3:09 PM
Unknown Object (File)
Oct 18 2024, 4:57 AM
Unknown Object (File)
Oct 4 2024, 11:36 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

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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
36 ↗(On Diff #76303)

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 ↗(On Diff #76306)

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 ↗(On Diff #76306)

Is this order important or can the zio_compress.h include just be pulled out of the if and avoid the else?

84 ↗(On Diff #76306)

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 ↗(On Diff #76306)

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 ↗(On Diff #76306)

Unsure. I'll try it.

84 ↗(On Diff #76306)

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 ↗(On Diff #77553)

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 ↗(On Diff #77553)

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
18 ↗(On Diff #78139)

Clever.

stand/libsa/zfs/zfsimpl.c
134 ↗(On Diff #78139)

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
18 ↗(On Diff #78139)

thanks.

stand/libsa/zfs/zfsimpl.c
134 ↗(On Diff #78139)

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
head/
stand/
efi/
boot1/
1 line
libsa/
zfs/
26 lines
2 lines
40 lines
sys/
cddl/
boot/
zfs/
54 lines
7 lines
contrib/
opensolaris/
uts/
common/
os/
243 lines
contrib/
openzfs/
include/
os/
freebsd/
linux/
2 lines
spl/
rpc/
4 lines
sys/
11 lines
10 lines
12 lines
9 lines
1 line
10 lines
3 lines
12 lines
3 lines
6 lines
5 lines
7 lines
9 lines
4 lines
sys/
8 lines
14 lines
lib/
libspl/
include/
os/
freebsd/
sys/
4 lines
module/
os/
freebsd/
spl/
2 lines

Diff 78147

View Options

head/stand/efi/boot1/Makefile

Loading...
View Options

head/stand/libsa/zfs/Makefile.inc

Loading...
View Options

head/stand/libsa/zfs/zfsimpl.c

Loading...
View Options

head/stand/libsa/zfs/zstd_shim.c

Loading...
View Options

head/sys/cddl/boot/zfs/zfsimpl.h

Loading...
View Options

head/sys/cddl/boot/zfs/zfssubr.c

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

head/sys/contrib/openzfs/include/os/freebsd/spl/sys/kmem_cache.h

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

head/sys/contrib/openzfs/include/os/freebsd/spl/sys/procfs_list.h

Loading...
View Options

head/sys/contrib/openzfs/include/os/freebsd/spl/sys/sig.h

Loading...
View Options

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

Loading...
View Options

head/sys/contrib/openzfs/include/os/freebsd/spl/sys/taskq.h

Loading...
View Options

head/sys/contrib/openzfs/include/os/freebsd/spl/sys/uio.h

Loading...
View Options

head/sys/contrib/openzfs/include/sys/nvpair.h

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...