Allow Kernel to link in both legacy libkern/zlib and new sys/contrib/zlib.

[delphij]

Summary

The 1st large review have been separated into multiple pieces based on modules and functionalities.

This change-set updates the kernel to link both old and new ZLIB versions so that we can migrate one component at a time.

I created sys/dev/zlib directory to have some std compatible header files, also place kernel module C file, and kernel versions memory allocation functions like zcalloc, zcfree and few others.

Test Plan

% make buildkernel

[ota_j.email.ne.jp]

Added sys/contrib/zlib/crc32.c to zlib kernel module.

[delphij]

sys/conf/kern.pre.mk

178 ↗

(On Diff #59136)

The two additional -D's are only specified for zutil.c for sys/modules/zlib (kmod.mk version of ZLIB_CFLAGS don't have these). Since the sys/conf/files already have them, should they be removed here?

sys/dev/zlib/errno.h

1 ↗

(On Diff #59136)

I still prefer a way to not use these "fake" headers (I can't really work on this until mid-July but will take a look as soon as I can).

sys/opencrypto/cryptodeflate.c

108 ↗

(On Diff #59136)

I think the probes needs to match their definitions? (Also, is there some specific reason not to convert this to use new zlib?)

[ota_j.email.ne.jp]

sys/conf/kern.pre.mk

178 ↗

(On Diff #59136)

The extra flags will be changed for Z_SOLO.

sys/dev/zlib/errno.h

1 ↗

(On Diff #59136)

We can delay this action by using -DZ_SOLO. ZFS needs non Z_SOLO zlib and we can revisit that time.

sys/opencrypto/cryptodeflate.c

108 ↗

(On Diff #59136)

The probes need to be matching ABI, the binary symbol.
I can put cryptodefate changes together.
One of suggestion was break one large changeset into smaller ones; this one looks better be a part of this one to avoid changing probe names.

[ota_j.email.ne.jp]

Use -DZ_SOLO to avoid standard C header files.
ZFS requires Non-Z_SOLO ZLIB and we will revisit when we work on ZFS.
Combined opencrypto conversion to avoid dtrace probe name changes.

[ota_j.email.ne.jp]

Drop -DMY_ZCALLOC as no use under -DZ_SOLO.

[ota_j.email.ne.jp]

Drop -DMY_ZCALLOC from kmod.mk but put zcfree() back.

[ota_j.email.ne.jp]

Missed zcallc.h/c files from last update.

[ota_j.email.ne.jp]

I had missed zcalloc.c in sys/modules/Makefile, too.

[ota_j.email.ne.jp]

Drop NO_GZIP compile option changes in zlib as these aren't necessary at this stage.

[delphij]

Thanks! This looks mostly good to me. I intend to commit this version with some minor tweaks after some testing in a day or two, so please yell if you don't agree with some change proposals (commented inline).

sys/contrib/zlib/zconf.h

442 ↗

(On Diff #59524)

This seems to be my bad (rS206708 derived from vendor 7147f24cd7b27dd95f6e841851a111cb311a9c07).

Initially, in rS205306, we #if 0'ed Z_HAVE_UNISTD_H, which used to determine if we would include sys/types.h. Later, the upstream have decided to include it for all platforms, and sys/types.h was included again, so when upgrading zlib I have decided to just take it and removed the include below, right before our hardcoded configuration.

May I ask a favor that we revert the change here to vendor version, and instead, add a block of:

#ifdef Z_SOLO
#  include <sys/types.h>      /* for off_t */
#endif

under "This is hard-configured for FreeBSD." so we keep the local changes together? Potentially, that would make it easier for future upgrades.

sys/opencrypto/cryptodeflate.c

63 ↗

(On Diff #59524)

the new line before crypto_zalloc() is not necessary.

64 ↗

(On Diff #59524)

[OPTIONAL] Personally I'd remove the variable names in prototype definition. Alternatively, you can move these functions' bodies here because these are static functions.

sys/sys/zlib.h

99 ↗

(On Diff #59524)

[OPTIONAL] I wonder if we can make deflate_copyright and inflate_copyright in sys/libkern/zlib.c static instead?

[ota_j.email.ne.jp]

sys/contrib/zlib/zconf.h

442 ↗

(On Diff #59524)

Ok.

sys/opencrypto/cryptodeflate.c

64 ↗

(On Diff #59524)

I will move implementations here.

sys/sys/zlib.h

99 ↗

(On Diff #59524)

It appeared that these 2 variables are not used at all. Converting to static variable resulted in error.

[ota_j.email.ne.jp]

Adjusted sys/type.h include to FreeBSD section and also moved crypto_z* function implementations to top of file.

[ota_j.email.ne.jp]

Disable broken gzipped aout executable support and rename zlib in ZFS to avoid filename conflicts.

gzipped aout supports has been broken for more than few years now. Disable it to avoid inflate() ABI conflict. I'm interested in reimplementing it with zlib but it will be one of the last zlib update processes.

Add opensolaris_ prefix to zlib files in ZFS to avoid object filename conflicts. Zlib in ZFS is compiled with different options and they are not ABI compatible.
This will also address https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=205822

These changes address link errors found by LINT:
% make buildLINT
% make buildkernel KERNCONF=LINT

[ota_j.email.ne.jp]

Actually, this file rename doesn't fix https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=205822.
I don't think it is worth addressing this bug report here as removing the zlib in zfs also fixes this issue and we are aiming for this serious of changes.

[ota_j.email.ne.jp]

D21072, "Remove support for kernel.tramp and kernel.tramp.gz", precedes this revision. This removes obsolete use of legacy zlib version.

[ota_j.email.ne.jp]

D21099 removed gzip'ed a.out support. Rebased and adjusted.

[delphij]

Rebase.

[delphij]

Add license plate for sys/dev/zlib/zlib_mod.c.

[delphij]

+@markm (Please confirm if the license is Okay for sys/dev/zlib/zlib_mod.c, the file is largely based on your change rS130799 in 2004 for sys/net/zlib.c).

sys/dev/zlib/zcalloc.c

1 ↗

(On Diff #60316)

A license plate should be added.

sys/dev/zlib/zcalloc.h

1 ↗

(On Diff #60316)

A license plate should be added.

[delphij]

+@mav @avg @mmacy for ZFS portion of change as a FYI (sys/cddl/contrib/opensolaris/uts/common/zmod). TL;DR: the bundled zlib (1.2.3) was removed in favor of new zlib; alloc/free functions merged from zmod_subr.c into zmod.c.

[markm]

LGTM otherwise.

sys/dev/zlib/zlib_mod.c

2 ↗

(On Diff #60318)

This file is 99.999% boilerplate; I'm happy for the copyright to revert to FreeBSD Foundation without mentioning me.

[delphij]

Modified zlib_mod.c to use standard FreeBSD Foundation license.

[ota_j.email.ne.jp]

It looks ZFS changes, dropping zlib files in zfs, are melded into this one.
I recall needing to add -DGZIP and/or -DGUNZIP option for zfs compression=gzip to properly uncompress gzipped zfs devices with older version; there was gzip header regression...

I'd like to test to ensure I can read zfs with compresssion=gzip from 12-RELEASE.

sys/dev/zlib/zcalloc.c

1 ↗

(On Diff #60316)

Please use
/* This file is in the public domain. */

It is a clone of multiple copies of same functions.

sys/dev/zlib/zcalloc.h

1 ↗

(On Diff #60316)

Please use
/* This file is in the public domain. */

It is a clone of multiple copies of same functions.

[ota_j.email.ne.jp]

I tested reads and writes on gzipped ZFS between 12.0-RELEASE and 13-CURRENT with this changes.
The test result was good.

Do we need to bump kernel version for this change like we did for gzip'ed a.out support removal?

[delphij]

Update license for zcalloc;

Fix tinderbox build (needs upstream).

[delphij]

Submitted as rS350496.